feat(v2): Ultra mode toggle — proactive delegation for every model/effort - #1209
feat(v2): Ultra mode toggle — proactive delegation for every model/effort#1209dbc-hbin wants to merge 9 commits into
Conversation
|
✅ Deterministic PR hygiene checks passed. |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe change adds Ultra Mode hint-text configuration across Codex persistence, the ChangesUltra Mode delegation settings
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⏳ DRAFT
What to do
Review readiness checklist
3/4 boxes ticked. This PR stays in draft until every box above is ticked. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/components/subagents-workspace/SubagentDelegationSection.tsx`:
- Around line 127-145: Update SubagentDelegationSection’s Ultra Mode editor to
maintain a local draft of ultraMode.hintText instead of calling onUltraModeSave
from the textarea onChange. Add an explicit Save action or serialized debounced
commit using the draft, pass ultraSaving from Subagents.tsx, and disable the
textarea and preset/save controls while the commit is active; keep the draft
synchronized with management API responses after successful reloads.
In `@gui/src/pages/Subagents.tsx`:
- Around line 31-57: Consolidate the duplicated `/api/v2` fetch logic in
`loadUltraMode` and the mount `useEffect` into one shared loader. Ensure
initial-load failures set `status` to `t("sub.ultraModeLoadFail")`, while
refresh failures invoked by saving propagate to `saveUltraMode` instead of being
swallowed; preserve the existing state updates on successful loads.
In `@src/cli/v2.ts`:
- Around line 131-148: Update the mode-hint argument handling around the verb
=== "mode-hint" branch to preserve the raw supplied text for
setMultiAgentModeHintText. Treat only a missing argument as equivalent to
--clear, reject a present whitespace-only value, and retain leading/trailing
whitespace in nonblank hints so CLI behavior matches the API contract.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: ec5671af-c0fb-4a89-842c-42e92cdabde3
📒 Files selected for processing (14)
gui/src/components/subagents-workspace/SubagentDelegationSection.tsxgui/src/components/subagents-workspace/SubagentsWorkspace.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/Subagents.tsxgui/src/pages/use-subagent-delegation.tssrc/cli/v2.tssrc/codex/features.tssrc/server/management/agent-settings-routes.tstests/codex-v2-gate.test.ts
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a914681d3d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/codex/features.ts (1)
650-667: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winHandle quoted keys before editing a dedicated table.
A config can contain
"multi_agent_mode_hint_text" = "old value"or'multi_agent_mode_hint_text' = "old value"in[features.multi_agent_v2].editScalarInTableonly matches the bare key, and the multiline guard at Line 661 also only matches the bare form. A set operation then inserts a bare duplicate key instead of replacing the existing key. TOML treats these keys as identical, so Codex can no longer parseconfig.toml.Match and decode bare and quoted keys consistently in the dedicated-table reader, multiline guard, and writer. Add a regression test for updating and clearing a quoted key.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/codex/features.ts` around lines 650 - 667, Update setV2StringField and the dedicated-table helpers to recognize bare, double-quoted, and single-quoted forms of the same key, decoding them consistently for reads, multiline detection, and edits so existing quoted entries are replaced rather than duplicated. Preserve support for unquoted keys and null clearing, and add regression coverage for updating and clearing quoted multi_agent_mode_hint_text entries.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/pages/Subagents.tsx`:
- Around line 34-42: Update loadUltraMode to return the parsed UltraModeState
without calling setUltraMode, and have its callers apply the result only when
the request is still current. Add cancellation, request-generation, or
current-apiBase guarding that covers both mount loads and save-triggered
refreshes, ensuring stale /api/v2 responses never overwrite state for a newer
API server.
---
Outside diff comments:
In `@src/codex/features.ts`:
- Around line 650-667: Update setV2StringField and the dedicated-table helpers
to recognize bare, double-quoted, and single-quoted forms of the same key,
decoding them consistently for reads, multiline detection, and edits so existing
quoted entries are replaced rather than duplicated. Preserve support for
unquoted keys and null clearing, and add regression coverage for updating and
clearing quoted multi_agent_mode_hint_text entries.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: f09bb947-49b9-44e0-b92e-b8202a9af59c
⛔ Files ignored due to path filters (1)
docs-site/public/assets/ultra-mode-subagents.pngis excluded by!**/*.png
📒 Files selected for processing (8)
docs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/agents.mdgui/src/components/subagents-workspace/SubagentDelegationSection.tsxgui/src/components/subagents-workspace/SubagentsWorkspace.tsxgui/src/pages/Subagents.tsxsrc/cli/v2.tssrc/codex/features.tstests/codex-v2-gate.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b85b95cb45
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb8a391591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const probe = probeCodexSupportsModeHint(); | ||
| if (probe === false) { |
There was a problem hiding this comment.
When a user downgrades Codex or shares a CODEX_HOME containing a hint written by a newer build, the old runtime rejects the existing key and cannot start, but this unconditional probe also makes ocx v2 mode-hint --clear and the dashboard's off action refuse to remove it. The newly added capability gate should apply only to non-null writes; null removal is backward-compatible and must remain available as the recovery path.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| // Multi-line basic string: `"""..."""` with escapes. | ||
| if (token.startsWith('"""')) { | ||
| if (!token.endsWith('"""') || token.length < 6) return null; | ||
| return decodeBasicStringBody(token.slice(3, -3)); |
There was a problem hiding this comment.
For a valid multiline hint using TOML's opening-newline elision or a line-ending backslash continuation, this new branch delegates to the single-line escape decoder: it retains the normally discarded first newline, and a continuation reaches the default escape case and returns null. Consequently getMultiAgentModeHintText can report text different from what Codex uses—or report an active hint as absent—while the writer then refuses to edit that multiline value. Decode the multiline-only newline and continuation rules before returning the hint.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| if (value.trim().length === 0 || value.startsWith("-")) { | ||
| log.error("v2 mode-hint: pass the hint text, or --clear to unset it."); | ||
| return 1; |
There was a problem hiding this comment.
When the intended prose begins with a hyphen, such as ocx v2 mode-hint "- Delegate independent work early", this condition rejects it even though --clear is the only reserved argument and the documented contract otherwise accepts arbitrary nonblank text. This makes a valid hint impossible to set through the CLI; distinguish the exact reserved flag, or support a -- terminator, instead of rejecting every leading hyphen.
AGENTS.md reference: AGENTS.md:L231-L232
Useful? React with 👍 / 👎.
| ultraSaving, | ||
| onUltraModeSave: patch => { void saveUltraMode(patch); }, | ||
| ultraLoadFailed, | ||
| onUltraModeRetry: () => { void loadUltraMode().catch(() => { setOk(false); setUltraLoadFailed(true); setStatus(t("sub.ultraModeLoadFail")); }); }, |
There was a problem hiding this comment.
After the initial /api/v2 request fails, status is set to the Ultra-mode load error and rendered as the page-level red notice; the newly added retry callback only handles rejection, while a successful loadUltraMode() clears ultraLoadFailed but never clears that status. The controls therefore recover while the page continues to claim loading failed until some unrelated save changes the notice. Clear the stale status, and update its tone if needed, when the retry succeeds.
AGENTS.md reference: gui/AGENTS.md:L33-L33
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/codex/features.ts`:
- Around line 832-892: Deduplicate the platform package definitions in
codexNativeBinaryCandidates by extracting one shared array of package name,
target triple, and executable name tuples, including all Windows variants.
Replace the three repeated candidate-building lists in the wrapper-relative,
CODEX_MANAGED_PACKAGE_ROOT, and resolved-command branches with loops over that
shared array, preserving their existing root paths.
- Around line 785-830: Cache the result of probeCodexSupportsModeHint per
resolved runtime.command so repeated setMultiAgentModeHintText requests do not
reread and scan native binaries on the request thread. Reuse the cached boolean
or null result for the same command, while invalidating or distinguishing
entries when the resolved command changes; preserve the existing false rejection
and null fallback behavior.
- Around line 375-396: Update decodeTomlStringToken so multi-line literal and
basic string bodies remove the TOML-required leading newline after stripping
their triple delimiters. Apply the trim directly to the triple-single body and
before calling decodeBasicStringBody for triple-double strings, while preserving
all other decoding and validation behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: a10a9305-5c3a-4d56-bf27-2498bca681b5
📒 Files selected for processing (12)
gui/src/components/subagents-workspace/SubagentDelegationSection.tsxgui/src/components/subagents-workspace/SubagentsWorkspace.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/Subagents.tsxsrc/cli/help.tssrc/codex/features.tstests/codex-v2-gate.test.ts
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb8a391591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const table = tomlTableBody(content, "features.multi_agent_v2"); | ||
| if (table !== null) { | ||
| const m = table.match(/^\s*subagent_developer_instructions\s*=\s*/m); | ||
| const keyRe = new RegExp(`^\\s*${escapeRegExp(key)}\\s*=\\s*`, "m"); |
There was a problem hiding this comment.
When a valid dedicated table uses the quoted key form "multi_agent_mode_hint_text" = "old", this bare-key regex reports the hint as absent. A subsequent non-null save reaches editScalarInTable, which also misses the quoted assignment and inserts an equivalent bare key, producing a duplicate TOML key that Codex cannot parse; clearing similarly becomes a no-op. Recognize and decode quoted keys before reading, replacing, or removing this field.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| // saveUltraMode can report them against the save action. | ||
| const loadUltraMode = useCallback(async (signal?: AbortSignal) => { | ||
| const res = await fetch(`${apiBase}/api/v2`, { signal }); | ||
| const data = await readJsonOrThrow<{ enabled?: boolean; multiAgentModeHintText?: string | null }>(res, t("sub.ultraModeLoadFail")); |
There was a problem hiding this comment.
The /api/v2 response also provides multiAgentMode, but this loader ignores it and treats the native feature flag alone as sufficient. In the normal default mode, applyMultiAgentMode preserves upstream V1 pins such as Luna, and after mode v1 followed by ocx v2 on, every catalog entry remains forced to V1; those sessions do not receive this V2-only mode hint even though the dashboard enables the switch and claims every model is covered. Consume the effective surface mode and either require/offer v2 mode or clearly restrict Ultra mode to V2 models.
AGENTS.md reference: gui/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| out.push(join(pkgRoot, "node_modules", "@openai", "codex-darwin-arm64", "vendor", "aarch64-apple-darwin", "bin", "codex")); | ||
| out.push(join(pkgRoot, "node_modules", "@openai", "codex-darwin-x64", "vendor", "x86_64-apple-darwin", "bin", "codex")); | ||
| out.push(join(pkgRoot, "node_modules", "@openai", "codex-linux-x64", "vendor", "x86_64-unknown-linux-musl", "bin", "codex")); | ||
| out.push(join(pkgRoot, "node_modules", "@openai", "codex-linux-arm64", "vendor", "aarch64-unknown-linux-musl", "bin", "codex")); |
There was a problem hiding this comment.
On a normal Windows npm installation, the resolved runtime is a codex.cmd or codex.opencodex-real.cmd wrapper. The probe reads that wrapper and sets sawBinary = true, but this wrapper-discovery block adds only Darwin and Linux native packages; the later realpathSync(command) fallback also derives the package root from the .cmd file's directory rather than from the referenced JS package. It therefore finds no native executable, returns false, and rejects every attempt to enable Ultra mode with the misleading instruction to update Codex even when the installed version supports the key. Add the Windows platform-package candidates using the wrapper's actual npm package location, and do not count script wrappers as inspected native binaries.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fb8a391591
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const pathDirs = (process.env.PATH ?? "").split(process.platform === "win32" ? ";" : ":"); | ||
| for (const dir of pathDirs) { | ||
| const wrapper = join(dir, process.platform === "win32" ? "codex.opencodex-real.cmd" : "codex.opencodex-real"); |
There was a problem hiding this comment.
When CODEX_CLI_PATH or the persisted runtime selects an older Codex while a newer codex.opencodex-real is also on PATH, this unconditional PATH scan adds the newer installation's native binary to the candidates. The probe returns true as soon as that unrelated binary contains the key, so Ultra mode writes multi_agent_mode_hint_text for the selected older runtime, which then rejects the unknown field and cannot start. Derive candidates exclusively from runtime.command (and its own shim metadata/package root) rather than mixing every PATH installation into the capability result.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| const tableText = `[features.multi_agent_v2]${eol}${key} = ${encoded}${eol}`; | ||
| atomicWriteFile(path, `${content}${suffix}${separator}${tableText}`); |
There was a problem hiding this comment.
When a valid Codex config expresses this object with root dotted keys, such as features.multi_agent_v2.enabled = true, none of the preceding dedicated-table, [features] inline-table, or bare-boolean checks recognize it. Enabling Ultra mode therefore appends [features.multi_agent_v2], redefining the table created by the dotted key and making config.toml invalid, so Codex fails at startup. Detect and edit the dotted-key representation or refuse the mutation without writing.
AGENTS.md reference: src/AGENTS.md:L10-L10
Useful? React with 👍 / 👎.
| /** Canonical Proactive delegation text mirrored from codex-rs (multi_agent_mode_instructions.rs). */ | ||
| export const ULTRA_MODE_PRESET = | ||
| "Proactive multi-agent delegation is active. Any earlier instruction requiring an explicit user request before spawning sub-agents no longer applies. Use sub-agents when parallel work would materially improve speed or quality. This mode remains active until a later multi-agent mode developer message changes it."; |
There was a problem hiding this comment.
This string is labeled as the canonical codex-rs preset, but it differs materially from the repository's existing PROACTIVE_MULTI_AGENT_MODE_TEXT in src/server/responses/collaboration.ts: it omits the instructions not to serialize independent work and to prefer specialist sub-agents with their own tool-capable contexts. Because multi_agent_mode_hint_text replaces the effort-derived message rather than augmenting it, enabling or restoring Ultra mode installs this weaker prompt instead of the native proactive behavior the UI and documentation promise. Keep this preset byte-aligned with the canonical text, ideally from a single shared source.
Useful? React with 👍 / 👎.
Ingwannu
left a comment
Ingwannu
left a comment
There was a problem hiding this comment.
The upstream capability is technically real, but this head is not an integration candidate yet.
- The branch is currently 526 commits behind
dev; the readiness checklist correctly leaves the latest-dev box unchecked. Please rebase and resolve the accumulated config/GUI/test overlap before further review. - Exact-head CI is red (
test 2/4, macOS, gates, and the aggregatecicheck). Do not treat the local focused pass as sufficient until the rebased head is green. - The current
decodeTomlStringTokenimplementation returns the mandatory first newline inside TOML multiline basic/literal strings. TOML trims the newline immediately following the opening triple quote, so reading a validmulti_agent_mode_hint_text = """ Proactive..."""currently adds a spurious leading newline. Fix both triple-quoted forms and keep a regression that matches TOML semantics.
The product naming, default preset text, GUI switch, and whether OCX should expose this unstable upstream override at all require @lidge-jun's explicit product/UI decision. This review is limited to the stale integration and parser/CI blockers; it is not an approval of the UI direction.
fb8a391 to
d0affeb
Compare
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@docs-site/src/content/docs/reference/cli/agents.md`:
- Around line 51-57: Clarify the mode-hint documentation to state that the CLI
writes the configuration key regardless of whether multi_agent_v2 is enabled,
while only the Subagents dashboard Ultra mode toggle requires that feature and
ocx v2 on. Avoid wording that makes the feature requirement appear to apply to
the CLI command.
In `@gui/src/i18n/ja.ts`:
- Line 547: Update the Japanese translation value for sub.ultraModeHint to
reference the fully qualified configuration key
multi_agent_v2.multi_agent_mode_hint_text instead of the unscoped key,
preserving the rest of the message.
In `@src/codex/features.ts`:
- Around line 1000-1038: Update probeCodexSupportsModeHint so cached results
cannot remain stale after an in-place binary replacement: either include the
inspected binary’s mtimeMs and size in cacheKey or re-probe instead of caching
false/null, and replace the non-null assertion on modeHintCapabilityCache.get
with a sentinel lookup that preserves null. Update the existing rewritten-file
test to expect re-probing after the binary changes.
In `@tests/codex-v2-gate.test.ts`:
- Around line 368-384: Make the mode-hint writer tests independent of the
installed Codex binary by extracting the existing native() and selectRuntime()
helpers to file scope and using them to install an ELF fixture runtime. In
tests/codex-v2-gate.test.ts lines 368-384, add beforeEach/afterEach setup and
teardown via
setCodexRuntimeResolveCacheForTests/resetCodexRuntimeResolveCacheForTests; in
lines 1263-1288, install the same runtime alongside CODEX_HOME/OPENCODEX_HOME
and restore it in the existing finally; in lines 1495-1536, apply and reset it
in both CLI tests alongside CODEX_HOME, with no direct changes needed elsewhere.
- Around line 567-587: Replace hard-coded ":" PATH separators in the affected
fixtures, including the test shown and the cases around selected PATH setup,
with node:path’s delimiter value. Ensure all four PATH constructions used by
these tests match resolveSelectedCommandPath across platforms without changing
the production behavior or assertions.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2827d30a-c113-4eac-a7b3-09730ee05bd1
⛔ Files ignored due to path filters (1)
docs-site/public/assets/ultra-mode-subagents.pngis excluded by!**/*.png
📒 Files selected for processing (19)
docs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/agents.mdgui/src/components/subagents-workspace/SubagentDelegationSection.tsxgui/src/components/subagents-workspace/SubagentsWorkspace.tsxgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/zh.tsgui/src/pages/Subagents.tsxgui/src/pages/use-subagent-delegation.tsgui/tests/multi-agent-guidance.test.tsxgui/tests/subagents-ultra-mode.test.tsxsrc/cli/help.tssrc/cli/v2.tssrc/codex/features.tssrc/server/management/agent-settings-routes.tstests/codex-v2-gate.test.ts
| "sub.workspace.selectModelDesc": "一覧からモデルを選んで詳細を確認し、spawn_agent のおすすめに設定します。", | ||
| "sub.workspace.selector": "公開セレクター", | ||
| "sub.ultraMode": "ウルトラモード", | ||
| "sub.ultraModeHint": "すべてのモデルと reasoning effort で Proactive マルチエージェント委任ポリシーを有効にします（reasoning effort 自体は変更しません）。config.toml に multi_agent_mode_hint_text を書き込みます。", |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Include the full V2 configuration path.
The text names multi_agent_mode_hint_text without the required multi_agent_v2 namespace. This can mislead users about the TOML structure and conflicts with the API and CLI configuration contract.
Use multi_agent_v2.multi_agent_mode_hint_text in the Japanese message.
Proposed fix
- "sub.ultraModeHint": "すべてのモデルと reasoning effort で Proactive マルチエージェント委任ポリシーを有効にします（reasoning effort 自体は変更しません）。config.toml に multi_agent_mode_hint_text を書き込みます。",
+ "sub.ultraModeHint": "すべてのモデルと reasoning effort で Proactive マルチエージェント委任ポリシーを有効にします（reasoning effort 自体は変更しません）。config.toml の multi_agent_v2.multi_agent_mode_hint_text を設定します。",📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "sub.ultraModeHint": "すべてのモデルと reasoning effort で Proactive マルチエージェント委任ポリシーを有効にします（reasoning effort 自体は変更しません）。config.toml に multi_agent_mode_hint_text を書き込みます。", | |
| "sub.ultraModeHint": "すべてのモデルと reasoning effort で Proactive マルチエージェント委任ポリシーを有効にします（reasoning effort 自体は変更しません）。config.toml の multi_agent_v2.multi_agent_mode_hint_text を設定します。", |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@gui/src/i18n/ja.ts` at line 547, Update the Japanese translation value for
sub.ultraModeHint to reference the fully qualified configuration key
multi_agent_v2.multi_agent_mode_hint_text instead of the unscoped key,
preserving the rest of the message.
|
@dbc-hbin In your pr description you got boxes which you check. Since there are still open rabbit bug-findings you will have to fix these first and then check all 4 boxes! Thanks! |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Requesting changes on current head d0affeb89c8083192b458621ea76d3b84c79b604.
I reviewed this for runtime/data bugs, leaks, edge cases, security boundaries, and the currently open CodeRabbit/Codex findings. I would not merge this yet.
Blocking findings
- P2: capability cache can become stale when the selected Codex binary is replaced in place
probeCodexSupportsModeHint() can keep a cached result even though the executable at the same path has changed. Both directions matter, but the stale-positive case is the dangerous one: a previously supported binary can be replaced/downgraded with an unsupported one while the cached true still permits writing multi_agent_mode_hint_text.
Please invalidate capability cache entries using executable identity, e.g. a stat fingerprint (size, mtimeMs, ctimeMs, and inode where available), and add regression coverage for both false -> binary replaced -> true and true -> binary replaced -> false.
- P2:
PUT /api/v2can partially mutate configuration and then return an error
A combined request can persist earlier changes such as multiAgentMode / enabling V2, then fail the later mode-hint capability probe and return 502. The caller sees a failed request while part of the requested state has already been committed.
Please preflight the non-null multiAgentModeHintText capability check before any mutation, or make the endpoint transactional. Add a regression test proving that a capability failure on a combined request leaves the feature flag, multiAgentMode, and TOML unchanged.
- Current
devintegration still needs to be completed before merge
This branch is materially behind current dev. After rebasing, make sure the newly added sub.ultraMode* translation keys satisfy the current typed locale contract, including Turkish, and re-run exact-head CI. The current PR description itself still marks “pushed to latest dev” unchecked.
Other valid findings / edge cases
- Capability-sensitive tests should not depend on whichever Codex binary happens to be installed on the developer/CI machine. Pin a fake runtime for deterministic tests.
- Use
path.delimiterinstead of hard-coding:in PATH-related tests so Windows behavior is deterministic. - The Japanese copy should name the full
features.multi_agent_v2.multi_agent_mode_hint_textpath consistently. - Docs should distinguish CLI config writes from dashboard/API prerequisite behavior more precisely.
- The multiline-value rejection in
setV2StringField()appears to scan by key name globally rather than being scoped to[features.multi_agent_v2]. An unrelated table containing a multilinemulti_agent_mode_hint_textcan therefore block an otherwise safe Ultra Mode edit. Please scope this guard to the actual target table/assignment.
Open Rabbit threads
Several unresolved threads are stale and already fixed in substance on this head, including the earlier multiline TOML handling, quoted keys, --clear, stale GUI errors, Windows runtime discovery, effective V2 gating, unrelated PATH probing, and proactive preset completeness. Those can be resolved after validating against the rebased head rather than reworking already-correct code.
Security / leak pass
I did not find a current credential/API-key leak, obvious XSS path, or terminal escape injection in this feature. The main security-adjacent concern is correctness of the capability gate and avoiding configuration state that claims success/failure inconsistently.
Please fix the two P2 issues first, rebase onto current dev, resolve the remaining valid deterministic/docs/i18n items, run exact-head CI, then re-check all unresolved Rabbit/Codex threads.
d0affeb to
7f88b13
Compare
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Re-review on current head 7f88b13153a310e2695800bfb17362cf059f445b.
The previous blockers are fixed: capability-cache invalidation now tracks in-place binary replacement, /api/v2 preflights mode-hint support before combined mutations, clear remains available on unsupported runtimes, capability-sensitive tests are hermetic, PATH fixtures use path.delimiter, selected-runtime probing/Windows wrapper resolution are corrected, dotted V2 forms fail closed, multiline handling is much safer, docs/i18n are updated, and this head is rebased onto current dev.
One remaining P2 data-integrity blocker:
Escaped quoted TOML keys can be duplicated and corrupt config.toml
Valid TOML can spell the key semantically as:
[features.multi_agent_v2]
"multi_agent_mode_hint_\u0074ext" = "old"That quoted key decodes to multi_agent_mode_hint_text.
The new code is inconsistent about semantic key equality:
findTomlAssignment()decodes quoted keys, so it can recognize the escaped spelling as the target key.getV2StringField()and especiallyeditScalarInTable()still rediscover the key withtomlKeyPattern(), which only matches the literal bare/quoted spellings and does not match escaped quoted keys.
A save can therefore append:
multi_agent_mode_hint_text = "new"next to the escaped quoted assignment. TOML sees both as the same key, so the document now contains a duplicate key and Codex can fail parsing config.toml. The reader can likewise report the existing escaped assignment as absent.
Please use semantic assignment discovery consistently for dedicated-table reads and writes. The editor should replace/remove the assignment found by the TOML-aware key decoder rather than rediscovering it with a narrower regex.
Add regression coverage for at least:
[features.multi_agent_v2]
"multi_agent_mode_hint_\u0074ext" = "old"and verify:
- reader returns
old - update leaves exactly one semantic key
- clear removes it
- resulting TOML still parses
Because setV2StringField() is shared, please cover the same representation for subagent_developer_instructions too.
CI note: exact-head Cross-platform CI and React Doctor currently show action_required, so there is still no executed exact-head CI signal to rely on.
Once this semantic quoted-key case is fixed and exact-head CI actually runs green, this looks close.
…el/effort Expose codex-rs features.multi_agent_v2.multi_agent_mode_hint_text through the OCX config editor, /api/v2, the ocx v2 CLI, and the Subagents GUI. The upstream field overrides the effort-derived multi-agent policy (ultra -> Proactive, else ExplicitRequestOnly) with custom <multi_agent_mode> text, so any model and any reasoning effort can run the Proactive delegation prompt. - features.ts: generalize the v2 string-field reader/writer and add get/setMultiAgentModeHintText (dedicated table, inline table, bare-boolean upgrade, CRLF/EOL preservation). - agent-settings-routes.ts: GET /api/v2 reports multiAgentModeHintText; PUT accepts string|null and rejects empty/whitespace strings (a present empty override would suppress even the ultra-derived Proactive message). - cli/v2.ts: ocx v2 mode-hint <text|--clear> and status line. - GUI: Ultra mode switch + editable text + preset restore on the Subagents page, disabled until multi_agent_v2 is enabled. - tests: reader/writer encodings, clear semantics, inline-table tricky values, API GET/PUT/400 validation, CLI mode-hint round-trip.
…aft editor, docs CodeRabbit findings: - CLI: preserve raw leading/trailing whitespace in mode-hint text; a missing argument is a usage error (never a destructive clear); only --clear unsets. Reject a present whitespace-only value, matching the API 400 contract. - features.ts: refuse to edit an existing multi-line TOML string for the hint key. scanTomlValueEnd stops at the second quote, so rewriting/removing a """...""" value would corrupt the document; convert to single line first. - GUI: the Ultra mode textarea now keeps a local draft and commits via an explicit Save button (remounted on server-value change), so keystrokes are never dropped while a PUT is in flight. /api/v2 fetch consolidated into one shared loader; initial-load failures surface sub.ultraModeLoadFail instead of masquerading as v2-off. V2 requirement is visible text, not only a title attr. - docs: document ocx v2 mode-hint and the multi_agent_mode_hint_text config key (override semantics, --clear, whitespace rejection, v2 requirement).
loadUltraMode now accepts an AbortSignal; the mount effect aborts it on cleanup so a pending fetch cannot apply state after unmount or after a refresh superseded it.
…e, GUI retry/clear
Codex + CodeRabbit findings:
- features.ts: scanTomlValueEnd/decodeTomlStringToken now read complete
multi-line TOML strings ("""...""" and '''...''') so the getter returns
the real hint instead of an empty string. probeCodexSupportsModeHint checks
the installed native codex binary for the multi_agent_mode_hint_text key
(following shim -> npm wrapper -> vendor binary) and refuses writes on older
builds that reject the unknown config member.
- cli/help.ts: advertise mode-hint in the v2 usage and global synopsis.
- GUI: empty/whitespace hint renders as OFF; clearing stays available when v2
is disabled (stale hint would otherwise re-activate); load failures surface
a retry; a successful save clears stale error status and confirms.
- tests: multi-line decode for basic and literal strings.
7f88b13 to
6649e5b
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@gui/src/i18n/en.ts`:
- Around line 591-598: Update the sub.ultraModeHint translation in
gui/src/i18n/en.ts (591-598), gui/src/i18n/de.ts (570-577), gui/src/i18n/ko.ts
(587-594), gui/src/i18n/ru.ts (585-592), and gui/src/i18n/zh.ts (580-587) to
omit the bare multi_agent_mode_hint_text reference and identify it under
features.multi_agent_v2. In gui/src/i18n/ja.ts (553-560), translate
features.multi_agent_v2.multi_agent_mode_hint_text without the extra features.
qualifier, preserving the intended managed V2 configuration path in every
locale.
In `@src/server/management/agent-settings-routes.ts`:
- Around line 303-314: Export the unsupported-runtime message from
setMultiAgentModeHintText in features.ts, then reuse that shared symbol in the
preflight check and scalarWrites error construction in the route instead of
duplicating the literal. Preserve the existing 502 response prefixes and atomic
preflight behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 2720b686-c7c1-436b-9570-0a1023db74d2
📒 Files selected for processing (13)
docs-site/src/content/docs/reference/cli/agents.mddocs-site/src/content/docs/reference/configuration/agents.mdgui/src/i18n/de.tsgui/src/i18n/en.tsgui/src/i18n/ja.tsgui/src/i18n/ko.tsgui/src/i18n/ru.tsgui/src/i18n/tr.tsgui/src/i18n/zh.tssrc/cli/help.tssrc/codex/features.tssrc/server/management/agent-settings-routes.tstests/codex-v2-gate.test.ts
💤 Files with no reviewable changes (1)
- gui/src/i18n/tr.ts
3 participants
Summary
Adds an Ultra mode control that enables codex-rs's Proactive multi-agent delegation policy for every model and every reasoning effort, not just native
ultra-effort turns. It exposes the upstreamfeatures.multi_agent_v2.multi_agent_mode_hint_textconfig key through OCX's existing v2 config editor, management API, CLI, and the Subagents GUI.How it works
Upstream codex-rs derives the multi-agent policy from effort:
ultra→MultiAgentMode::Proactive, everything else →ExplicitRequestOnly(core/src/session/multi_agents.rs). A configuredmulti_agent_mode_hint_textoverrides that derivation entirely and injects the configured text as the<multi_agent_mode>developer message. Verified live: with the hint set,max/highsessions receive the Proactive prompt andcollaboration__spawn_agentactually spawns sub-agents.Note: the toggle does not change reasoning effort — it changes the delegation prompt policy. The GUI sublabel says so explicitly.
Changes
getMultiAgentModeHintText/setMultiAgentModeHintText. Handles all three TOML encodings (dedicated table, inline table, bare-boolean upgrade) with EOL preservation, matchingsubagent_developer_instructions.GET /api/v2returnsmultiAgentModeHintText;PUTacceptsstring | null. Empty/whitespace strings are rejected (a present empty override would suppress even the ultra-derived Proactive message upstream).ocx v2 mode-hint <text|--clear>plus a status line.multi_agent_v2is enabled.}, quotes), bare-boolean upgrade, API GET/PUT/400 validation, CLI round-trip.Validation
bun x tsc --noEmit(server + gui) passesbun run lint(gui) passesbun test tests/codex-v2-gate.test.ts tests/management-client-config-route.test.ts— 102 passUI Screenshot
Ultra mode toggle on the Subagents page (switch + description + editor):
Summary by CodeRabbit
UI Screenshot
Ultra mode toggle on the Subagents page (switch + description + editor):
Review readiness checklist