fix(acp): restore omp effort after its model, exact model ids, omp advisor

- omp lists thinking levels per model: the saved effort ("high") is not
  offered by the default model, so it was skipped and the "off" the agent
  reported was saved as the default; every new chat started Off. A restored
  effort the catalog lacks now waits for the restored model's switch
  (m_pendingEffort), is set once offered, and the reset value is never
  saved meanwhile.
- Config-option model restore matches the saved id exactly before falling
  back to Codex's "model/effort" base, and compares with the option's own
  value: pi-acp/omp ids are "provider/model", so a saved model sharing the
  default's provider was taken for it and never restored.
- /advisor follows the advertised hint: omp's "[on|off|status|...]" gets
  "/advisor on|off" instead of "/advisor opus", and is not re-sent on a
  model change; typed status/dump/configure no longer tick the toggle.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

Author: nullmastermind

src/AcpProtocol.cpp

@@ -978,15 +978,45 @@ bool commandsIncludeGoal(const QList<AcpCommandInfo> &commands)
     return false;
 }
 
-bool commandsIncludeAdvisor(const QList<AcpCommandInfo> &commands)
+namespace {
+
+const AcpCommandInfo *findAdvisorCommand(const QList<AcpCommandInfo> &commands)
 {
     for (const AcpCommandInfo &cmd : commands) {
         QStringView v{cmd.name};
         v = v.trimmed();
         while (!v.isEmpty() && v.front() == QLatin1Char('/'))
             v = v.mid(1);
         if (v.compare(QLatin1String("advisor"), Qt::CaseInsensitive) == 0)
+            return &cmd;
+    }
+    return nullptr;
+}
+
+} // namespace
+
+bool commandsIncludeAdvisor(const QList<AcpCommandInfo> &commands)
+{
+    return findAdvisorCommand(commands) != nullptr;
+}
+
+bool advisorIsToggle(const QList<AcpCommandInfo> &commands)
+{
+    const AcpCommandInfo *cmd = findAdvisorCommand(commands);
+    if (!cmd) return false;
+    // "[on|off|status|dump [raw]|configure]": alternatives split by | [ ] and spaces.
+    const QStringView hint{cmd->inputHint};
+    qsizetype start = -1;
+    for (qsizetype i = 0; i <= hint.size(); ++i) {
+        const bool sep = i == hint.size() || hint[i] == QLatin1Char('|') || hint[i] == QLatin1Char('[')
+            || hint[i] == QLatin1Char(']') || hint[i].isSpace();
+        if (!sep) {
+            if (start < 0) start = i;
+            continue;
+        }
+        if (start >= 0 && hint.mid(start, i - start).compare(QLatin1String("on"), Qt::CaseInsensitive) == 0)
             return true;
+        start = -1;
     }
     return false;
 }
@@ -1016,6 +1046,14 @@ QString advisorCommandText(bool on, const QString &modelValue, const QString &mo
               : QStringLiteral("/advisor off");
 }
 
+QString advisorCommandText(bool on, const QList<AcpCommandInfo> &commands,
+                           const QString &modelValue, const QString &modelLabel)
+{
+    if (advisorIsToggle(commands))
+        return on ? QStringLiteral("/advisor on") : QStringLiteral("/advisor off");
+    return advisorCommandText(on, modelValue, modelLabel);
+}
+
 std::optional<bool> parseAdvisorCommand(const QString &text)
 {
     QStringView v = QStringView{text}.trimmed();
@@ -1029,6 +1067,19 @@ std::optional<bool> parseAdvisorCommand(const QString &text)
     v = v.trimmed();
     if (v.isEmpty())
         return std::nullopt;
+    // omp's queries and setup leave the advisor as it is.
+    QStringView verb = v;
+    for (qsizetype i = 0; i < v.size(); ++i) {
+        if (v[i].isSpace()) {
+            verb = v.left(i);
+            break;
+        }
+    }
+    for (const QLatin1String query : {QLatin1String("status"), QLatin1String("dump"),
+                                      QLatin1String("configure")}) {
+        if (verb.compare(query, Qt::CaseInsensitive) == 0)
+            return std::nullopt;
+    }
     return v.compare(QLatin1String("off"), Qt::CaseInsensitive) != 0;
 }
 

src/AcpProtocol.h

@@ -419,8 +419,15 @@ bool commandsIncludeAdvisor(const QList<AcpCommandInfo> &commands);
 QString advisorModelFor(const QString &modelValue, const QString &modelLabel = {});
 // "/advisor <advisor model>" when on, "/advisor off" when off.
 QString advisorCommandText(bool on, const QString &modelValue, const QString &modelLabel = {});
-// Reads a typed `/advisor` command: "off" -> false, a model argument -> true.
-// Bare `/advisor` (status query) and any other text -> nullopt.
+// True when the advertised `/advisor` is an on/off switch rather than a model
+// pick: its hint lists "on" (omp: "[on|off|status|dump [raw]|configure]").
+bool advisorIsToggle(const QList<AcpCommandInfo> &commands);
+// The command in the form the agent advertises: "/advisor on" / "/advisor off"
+// for a toggle, else advisorCommandText(on, modelValue, modelLabel).
+QString advisorCommandText(bool on, const QList<AcpCommandInfo> &commands,
+                           const QString &modelValue, const QString &modelLabel = {});
+// Reads a typed `/advisor` command: "off" -> false, "on" or a model -> true.
+// Bare `/advisor`, omp's status/dump/configure and any other text -> nullopt.
 std::optional<bool> parseAdvisorCommand(const QString &text);
 // Prepend `goal` when missing. O(n) over a few dozen commands; no heap if present.
 void ensureHostGoalCommand(QList<AcpCommandInfo> &commands);

src/widgets/AcpSessionView.cpp

@@ -1144,6 +1144,7 @@ void AcpSessionView::rebind(AcpSessionModel *model, AcpConnection *connection)
     m_model = model;
     m_connection = connection;
     m_savedPrefsApplied = false;
+    m_pendingEffort = {};
     // The conversation is gone: a restarted session is a draft again.
     m_draft = true;
     m_carry = {};
@@ -1288,6 +1289,7 @@ void AcpSessionView::rebindConnection(AcpConnection *connection)
         disconnect(m_registry, nullptr, this, nullptr);
     m_connection = connection;
     m_savedPrefsApplied = false;
+    m_pendingEffort = {};
     ++m_scheduledRev;
     m_parked = {};
     m_resumeBlocked = false;
@@ -2653,6 +2655,7 @@ void AcpSessionView::onMetadataChanged()
     // Once the agent's catalogs have arrived, push any saved per-agent
     // selections back into the session (one-shot per session).
     applySavedPreferences();
+    applyPendingEffort();
 
     // After the initial restore, persist any agent-side config changes
     // (e.g. Codex CLI's config_option_update for model/mode/effort) so the
@@ -2674,7 +2677,7 @@ void AcpSessionView::onMetadataChanged()
             if (!curMode.isEmpty())
                 m_registry->setAgentPreference(agentId, QStringLiteral("mode"), curMode);
 
-            if (!m_effortConfigOptionId.isEmpty()) {
+            if (!m_effortConfigOptionId.isEmpty() && m_pendingEffort.effort.isEmpty()) {
                 for (const auto &opt : m_model->configOptions()) {
                     if (opt.id == m_effortConfigOptionId) {
                         const QString val = opt.currentValue.toString();
@@ -2708,7 +2711,8 @@ void AcpSessionView::sendAdvisorCommand(bool on)
     if (!m_connection || !m_model || !m_modelCombo) return;
     const int idx = m_modelCombo->currentIndex();
     submitSlashCommand(AcpProtocol::advisorCommandText(
-        on, m_modelCombo->itemData(idx).toString(), m_modelCombo->itemText(idx)));
+        on, m_model->availableCommands(), m_modelCombo->itemData(idx).toString(),
+        m_modelCombo->itemText(idx)));
 }
 
 // Same routing as typing the command and pressing Send: queued while a turn
@@ -3748,6 +3752,7 @@ void AcpSessionView::onModelComboChanged(int index)
     if (id.isEmpty()) return;
     const QString prefKey = m_modelConfigOptionId.isEmpty()
         ? QStringLiteral("model") : m_modelConfigOptionId;
+    m_pendingEffort = {}; // the user's pick replaces the restore
     if (!m_connection->isSessionReady()) {
         // Agent not started yet (restored session): applied once it is.
         m_carry.model = id;
@@ -3764,8 +3769,10 @@ void AcpSessionView::onModelComboChanged(int index)
     if (m_registry && m_draft) {
         m_registry->setAgentPreference(m_connection->definition().id, prefKey, id);
     }
-    // The advisor tracks the selected model's level: re-point it.
-    if (m_advisorBtn && m_advisorBtn->isVisible() && m_advisorBtn->isChecked())
+    // The advisor tracks the selected model's level: re-point it. An on/off
+    // advisor (omp) names no model, so there is nothing to re-point.
+    if (m_advisorBtn && m_advisorBtn->isVisible() && m_advisorBtn->isChecked() && m_model
+        && !AcpProtocol::advisorIsToggle(m_model->availableCommands()))
         sendAdvisorCommand(true);
 }
 
@@ -3792,6 +3799,7 @@ void AcpSessionView::onEffortComboChanged(int index)
     if (m_updatingSelectors || !m_connection || index < 0) return;
     if (m_effortConfigOptionId.isEmpty()) return;
     const QString val = m_effortCombo->itemData(index).toString();
+    m_pendingEffort = {};
     if (m_connection->isSessionReady()) {
         m_connection->setConfigOption(m_effortConfigOptionId, val);
     } else {
@@ -3846,26 +3854,32 @@ void AcpSessionView::applySavedPreferences()
     const QString savedModel = want.model;
     const QString savedModelBase = baseModelId(savedModel);
     const QString currentModelBase = baseModelId(m_model->currentModelId());
-    if (!savedModel.isEmpty() && savedModelBase != currentModelBase) {
-        if (!m_modelConfigOptionId.isEmpty()) {
-            for (const auto &opt : m_model->configOptions()) {
-                if (opt.id != m_modelConfigOptionId) continue;
-                for (const auto &ch : opt.options) {
-                    if (ch.value == savedModel || ch.value == savedModelBase) {
-                        m_connection->setConfigOption(m_modelConfigOptionId, ch.value);
-                        break;
-                    }
-                }
-                break;
+    QString modelSent;
+    if (!savedModel.isEmpty() && !m_modelConfigOptionId.isEmpty()) {
+        // A choice equal to the saved id wins: omp ids are "provider/model",
+        // where the part before the slash is no model. Only a saved Codex
+        // "model/effort" id falls back to its base model.
+        for (const auto &opt : m_model->configOptions()) {
+            if (opt.id != m_modelConfigOptionId) continue;
+            const auto has = [&opt](const QString &v) {
+                return std::any_of(opt.options.cbegin(), opt.options.cend(),
+                                   [&v](const auto &ch) { return ch.value == v; });
+            };
+            const QString target = has(savedModel) ? savedModel
+                : has(savedModelBase)              ? savedModelBase
+                                                   : QString();
+            if (!target.isEmpty() && target != opt.currentValue.toString()) {
+                modelSent = target;
+                m_connection->setConfigOption(m_modelConfigOptionId, target);
             }
-        } else {
-            for (const auto &m : m_model->availableModels()) {
-                if (m.id == savedModel || baseModelId(m.id) == savedModelBase) {
-                    m_connection->setModel(m.id.contains(QLatin1Char('/'))
-                                               ? baseModelId(m.id)
-                                               : m.id);
-                    break;
-                }
+            break;
+        }
+    } else if (!savedModel.isEmpty() && savedModelBase != currentModelBase) {
+        for (const auto &m : m_model->availableModels()) {
+            if (m.id == savedModel || baseModelId(m.id) == savedModelBase) {
+                modelSent = m.id.contains(QLatin1Char('/')) ? baseModelId(m.id) : m.id;
+                m_connection->setModel(modelSent);
+                break;
             }
         }
     }
@@ -3881,25 +3895,56 @@ void AcpSessionView::applySavedPreferences()
         }
     }
 
-    // Effort / reasoning level: keyed by the matched config-option id.
+    // Effort / reasoning level: keyed by the matched config-option id. A level
+    // the current model lacks may be the restored model's: wait for its switch.
     if (!m_effortConfigOptionId.isEmpty()) {
         const QString savedEffort = want.effort;
         if (!savedEffort.isEmpty()) {
             for (const auto &opt : m_model->configOptions()) {
                 if (opt.id != m_effortConfigOptionId) continue;
                 if (opt.currentValue.toString() == savedEffort) break;
-                for (const auto &ch : opt.options) {
-                    if (ch.value == savedEffort) {
-                        m_connection->setConfigOption(m_effortConfigOptionId, savedEffort);
-                        break;
-                    }
-                }
+                const bool offered = std::any_of(opt.options.cbegin(), opt.options.cend(),
+                                                 [&](const auto &ch) { return ch.value == savedEffort; });
+                if (offered)
+                    m_connection->setConfigOption(m_effortConfigOptionId, savedEffort);
+                else if (!modelSent.isEmpty())
+                    m_pendingEffort = {savedEffort, modelSent, false};
                 break;
             }
         }
     }
 }
 
+void AcpSessionView::applyPendingEffort()
+{
+    if (m_pendingEffort.effort.isEmpty() || !m_connection || !m_model) return;
+    const QString current = m_model->currentModelId();
+    const bool modelLanded = current == m_pendingEffort.model
+        || baseModelId(current) == m_pendingEffort.model;
+    const auto &opts = m_model->configOptions();
+    const auto opt = std::find_if(opts.cbegin(), opts.cend(), [this](const auto &o) {
+        return !m_effortConfigOptionId.isEmpty() && o.id == m_effortConfigOptionId;
+    });
+    if (opt == opts.cend()) {
+        if (modelLanded) m_pendingEffort = {};
+        return;
+    }
+    if (opt->currentValue.toString() == m_pendingEffort.effort) {
+        m_pendingEffort = {}; // landed: what the agent reports is the default again
+        return;
+    }
+    if (m_pendingEffort.sent) return;
+    const bool offered = std::any_of(opt->options.cbegin(), opt->options.cend(), [this](const auto &ch) {
+        return ch.value == m_pendingEffort.effort;
+    });
+    if (offered) {
+        m_connection->setConfigOption(m_effortConfigOptionId, m_pendingEffort.effort);
+        m_pendingEffort.sent = true;
+    } else if (modelLanded) {
+        m_pendingEffort = {}; // the restored model does not offer it either
+    }
+}
+
 void AcpSessionView::onJumpToBottomClicked()
 {
     if (!m_scroll) return;

src/widgets/AcpSessionView.h

@@ -419,6 +419,8 @@ private slots:
     // continues on a new agent process. Called once per session after
     // metadata first populates the available catalogs.
     void applySavedPreferences();
+    // Sets m_pendingEffort once the agent's catalog offers it.
+    void applyPendingEffort();
     // Remember the model's current model / mode / effort for applySavedPreferences.
     void captureSelection();
     // The model and effort the agent reports now.
@@ -638,6 +640,18 @@ private slots:
     // The session's own selection to re-apply on its next agent process
     // (Switch agent, restored session); consumed by applySavedPreferences.
     Selection m_carry;
+    // A restored effort the agent does not offer yet: omp lists thinking levels
+    // per model, so it waits for the restored model's switch to land, then is
+    // set (applyPendingEffort). Meanwhile the effort the agent reports is its
+    // reset, never saved as a default. Dropped when the restored model lacks it
+    // or the user picks a model or effort.
+    struct PendingEffort
+    {
+        QString effort;
+        QString model; // the model id the restore sent
+        bool sent = false;
+    };
+    PendingEffort m_pendingEffort;
     // Restored session: the first prompt waits here while its agent resumes.
     struct ParkedSend
     {

tests/test_acp_protocol_serialization.cpp

@@ -66,6 +66,8 @@ private slots:
     void advisorModelFor_stepsUpOneLevel();
     void advisorModelFor_readsFamilyFromValueOrLabel();
     void advisorCommandText_onAndOff();
+    void advisorIsToggle_readsTheHint();
+    void advisorCommandText_followsTheAdvertisedForm();
     void parseAdvisorCommand_readsTypedToggle();
     void parseAgentCapabilities_specKeysAndLegacyFallback();
 
@@ -603,6 +605,40 @@ void TestAcpProtocolSerialization::advisorCommandText_onAndOff()
              QStringLiteral("/advisor off"));
 }
 
+namespace {
+// `/advisor` as each agent advertises it (captured 2026-11-02).
+AcpProtocol::AcpCommandInfo advisorCommand(const QString &hint)
+{
+    return {QStringLiteral("advisor"), QStringLiteral("advisor"), hint};
+}
+const QString kClaudeAdvisorHint = QStringLiteral("[fable|opus|sonnet|off]"); // claude-agent-acp
+const QString kOmpAdvisorHint = QStringLiteral("[on|off|status|dump [raw]|configure]"); // omp 18.15
+} // namespace
+
+void TestAcpProtocolSerialization::advisorIsToggle_readsTheHint()
+{
+    QVERIFY(AcpProtocol::advisorIsToggle({advisorCommand(kOmpAdvisorHint)}));
+    QVERIFY(!AcpProtocol::advisorIsToggle({advisorCommand(kClaudeAdvisorHint)}));
+    QVERIFY(!AcpProtocol::advisorIsToggle({advisorCommand(QString())}));
+    QVERIFY(!AcpProtocol::advisorIsToggle({}));
+    // "on" must be an alternative of its own, not part of a word.
+    QVERIFY(!AcpProtocol::advisorIsToggle({advisorCommand(QStringLiteral("[sonnet|off]"))}));
+}
+
+void TestAcpProtocolSerialization::advisorCommandText_followsTheAdvertisedForm()
+{
+    const QList<AcpProtocol::AcpCommandInfo> omp{advisorCommand(kOmpAdvisorHint)};
+    const QList<AcpProtocol::AcpCommandInfo> claude{advisorCommand(kClaudeAdvisorHint)};
+    QCOMPARE(AcpProtocol::advisorCommandText(true, omp, QStringLiteral("local-ccr/composer")),
+             QStringLiteral("/advisor on"));
+    QCOMPARE(AcpProtocol::advisorCommandText(false, omp, QStringLiteral("local-ccr/composer")),
+             QStringLiteral("/advisor off"));
+    QCOMPARE(AcpProtocol::advisorCommandText(true, claude, QStringLiteral("sonnet")),
+             QStringLiteral("/advisor opus"));
+    QCOMPARE(AcpProtocol::advisorCommandText(false, claude, QStringLiteral("sonnet")),
+             QStringLiteral("/advisor off"));
+}
+
 void TestAcpProtocolSerialization::parseAdvisorCommand_readsTypedToggle()
 {
     QCOMPARE(AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisor opus")),
@@ -614,7 +650,13 @@ void TestAcpProtocolSerialization::parseAdvisorCommand_readsTypedToggle()
     QCOMPARE(AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisor OFF")),
              std::optional<bool>(false));
     // Status query, lookalikes and ordinary prompts leave the toggle alone.
+    QCOMPARE(AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisor on")),
+             std::optional<bool>(true));
     QVERIFY(!AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisor")).has_value());
+    // omp's queries and setup do not toggle it.
+    QVERIFY(!AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisor status")).has_value());
+    QVERIFY(!AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisor dump raw")).has_value());
+    QVERIFY(!AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisor configure")).has_value());
     QVERIFY(!AcpProtocol::parseAdvisorCommand(QStringLiteral("/advisors opus")).has_value());
     QVERIFY(!AcpProtocol::parseAdvisorCommand(QStringLiteral("use /advisor off")).has_value());
     QVERIFY(!AcpProtocol::parseAdvisorCommand(QString()).has_value());