feat(v2): Ultra mode toggle — proactive delegation for every model/effort - #1209
feat(v2): Ultra mode toggle — proactive delegation for every model/effort#1209hanbinnoh wants to merge 5 commits into
Conversation
…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.
|
✅ Deterministic PR hygiene checks passed. |
📝 WalkthroughWalkthroughThe change adds Ultra Mode hint-text configuration across Codex persistence, the ChangesUltra Mode delegation settings
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested labels: 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".
…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).
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".
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.
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
| function decodeTomlStringToken(token: string): string | null { | ||
| if (token.length < 2) return null; | ||
| // Multi-line literal string: `'''...'''` verbatim (backslashes not special). | ||
| if (token.startsWith("'''")) { | ||
| return token.endsWith("'''") && token.length >= 6 | ||
| ? token.slice(3, -3) | ||
| : null; | ||
| } | ||
| // Multi-line basic string: `"""..."""` with escapes. | ||
| if (token.startsWith('"""')) { | ||
| if (!token.endsWith('"""') || token.length < 6) return null; | ||
| return decodeBasicStringBody(token.slice(3, -3)); | ||
| } | ||
| if (token.startsWith("'")) { | ||
| return token.endsWith("'") ? token.slice(1, -1) : null; | ||
| } | ||
| if (!token.startsWith('"') || !token.endsWith('"')) return null; | ||
| const body = token.slice(1, -1); | ||
| return decodeBasicStringBody(token.slice(1, -1)); | ||
| } | ||
|
|
||
| /** Unescape the body of a TOML basic string (single- or multi-line). */ | ||
| function decodeBasicStringBody(body: string): string | null { |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the omitted reader logic that builds the raw token passed to decodeTomlStringToken.
sed -n '540,700p' src/codex/features.tsRepository: lidge-jun/opencodex
Length of output: 8199
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== decoder slice =="
sed -n '360,430p' src/codex/features.ts
echo "== tests =="
sed -n '310,335p' tests/codex-v2-gate.test.ts
echo "== scan definition =="
rg -n "function scanTomlValueEnd|scanTomlValueEnd" src/codex/features.ts -A 25 -B 5
echo "== deterministic decoder probe =="
node - <<'JS'
function sliceMultiLineBody(token) {
if (token.startsWith('"""') && token.endsWith('"""') && token.length >= 6) return token.slice(3, -3);
if (token.startsWith("'''") && token.endsWith("'''") && token.length >= 6) return token.slice(3, -3);
return null;
}
const token = '"""\\nProactive\\nmulti-line\\n"""';
console.log("token:", JSON.stringify(token));
console.log("result:", JSON.stringify(sliceMultiLineBody(token)));
JSRepository: lidge-jun/opencodex
Length of output: 13205
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== toml table body and multiline scan =="
sed -n '300,370p' src/codex/features.ts
echo "== scan continuation =="
sed -n '430,520p' src/codex/features.ts
echo "== run only the affected tests =="
npm test -- tests/codex-v2-gate.test.ts -t "reader decodes multi-line basic and literal TOML strings"Repository: lidge-jun/opencodex
Length of output: 7306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== static simulation of getV2StringField on multi-line text =="
python3 - <<'PY'
def scan_toml_value_end(text, start):
i = start
while i < len(text) and text[i] in " \t":
i += 1
first = text[i]
if first == '"':
if text[i+1:i+3] == '""':
i += 3
while i < len(text):
if text[i] == '\\':
i += 2
continue
if text[i:i+3] == '"""':
return i + 3
i += 1
return len(text)
i += 1
while i < len(text):
if text[i] == '\\':
i += 2
continue
if text[i] == '"':
return i + 1
i += 1
return len(text)
if first == "'":
if text[i+1:i+3] == "''":
i += 3
close = text.find("'''", i)
return text.length if close == -1 else close + 3
return i
text = '[features.multi_agent_v2]\nmulti_agent_mode_hint_text = """\nProactive\nmulti-line\n"""\n'
value_start = text.index('"""')
value_end = scan_toml_value_end(text, value_start)
token = text[value_start:value_end]
print("token:", repr(token))
print("slice 3:-3:", repr(token[3:-3]))
print("contains leading newline:", token[3:-3].startswith("\n"))
print("contains trailing newline:", token[3:-3].endswith("\n"))
PYRepository: lidge-jun/opencodex
Length of output: 364
Trim the leading newline for multi-line TOML string bodies.
src/codex/features.ts:433-460 scans "'\nProactive\nmulti-line\n"' through both closing quotes, and getV2StringField passes that token to decodeTomlStringToken. src/codex/features.ts:378-388 then returns the body with .slice(3, -3), leaving the TOML-required leading newline as "\nProactive\nmulti-line\n". For the shown fixture, getMultiAgentModeHintText will exceed tests/codex-v2-gate.test.ts:322-325. Apply the newline trim before decodeBasicStringBody for triple-double strings and directly after slicing for triple-single strings.
🤖 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 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.
| export function setMultiAgentModeHintText(value: string | null, configPath?: string): ConfigEditResult { | ||
| // The upstream `multi_agent_mode_hint_text` key is newer than the v2 config | ||
| // surface opencodex already manages; an older Codex build rejects the unknown | ||
| // member (`#[serde(deny_unknown_fields)]`) and fails to start. Probe the | ||
| // installed runtime binary for the key string and refuse the write when the | ||
| // binary provably lacks it. A probe that cannot run (missing binary, | ||
| // unreadable file) does not block: that is the test/hermetic path and the | ||
| // headless runtime fallback. | ||
| const probe = probeCodexSupportsModeHint(); | ||
| if (probe === false) { | ||
| return { | ||
| ok: false, | ||
| error: "installed Codex does not support multi_agent_mode_hint_text; update Codex first", | ||
| }; | ||
| } | ||
| return setV2StringField("multi_agent_mode_hint_text", value, configPath); | ||
| } | ||
|
|
||
| /** | ||
| * True when the installed Codex runtime binary contains the | ||
| * `multi_agent_mode_hint_text` config key, false when it provably does not, and | ||
| * null when the probe could not run (missing binary, unreadable file). | ||
| */ | ||
| export function probeCodexSupportsModeHint(): boolean | null { | ||
| try { | ||
| const runtime = resolveAndPersistCodexRuntime({ env: process.env }).runtime; | ||
| const candidates = codexNativeBinaryCandidates(runtime.command); | ||
| let sawBinary = false; | ||
| for (const candidate of candidates) { | ||
| try { | ||
| if (!existsSync(candidate)) continue; | ||
| const buf = readFileSync(candidate); | ||
| sawBinary = true; | ||
| if (buf.includes(Buffer.from("multi_agent_mode_hint_text", "utf8"))) { | ||
| return true; | ||
| } | ||
| } catch { | ||
| continue; | ||
| } | ||
| } | ||
| // At least one real binary was inspected and none contained the key. | ||
| return sawBinary ? false : null; | ||
| } catch { | ||
| return null; | ||
| } | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
fd -t f 'agent-settings-routes.ts' src
rg -n -C6 'setMultiAgentModeHintText' src/server/management/agent-settings-routes.tsRepository: lidge-jun/opencodex
Length of output: 2275
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== features.ts relevant slice =="
sed -n '770,835p' src/codex/features.ts
echo
echo "== resolveAndPersistCodexRuntime and codexNativeBinaryCandidates definitions =="
rg -n -C8 'function resolveAndPersistCodexRuntime|const codexNativeBinaryCandidates|resolveAndPersistCodexRuntime|codexNativeBinaryCandidates' src/codex src/server/management
echo
echo "== agent-settings routes relevant slice =="
sed -n '280,350p' src/server/management/agent-settings-routes.tsRepository: lidge-jun/opencodex
Length of output: 18272
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== candidate implementation =="
sed -n '838,895p' src/codex/features.ts
echo
echo "== all call sites for probeCodexSupportsModeHint and setMultiAgentModeHintText =="
rg -n -C3 'probeCodexSupportsModeHint|setMultiAgentModeHintText' src tests
echo
echo "== route context around the async import and call loop =="
sed -n '290,355p' src/server/management/agent-settings-routes.tsRepository: lidge-jun/opencodex
Length of output: 19330
Cache or move the probeCodexSupportsModeHint() capability check.
src/server/management/agent-settings-routes.ts:333 executes setMultiAgentModeHintText inline in the PUT /api/v2 handler path, so every mode-hint update runs probeCodexSupportsModeHint() on the request thread. That probe calls readFileSync(candidate) and scans the entire candidate binary with Buffer.includes; native Codex executables can be tens of megabytes, so each write blocks the event loop and can stall other traffic while the full file is read and scanned. Cache the result per resolved runtime.command, or perform the probe outside a per-request code path.
🤖 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 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.
| /** | ||
| * Candidate native codex binaries to probe. The resolved `command` may be the | ||
| * opencodex shim or the npm JS wrapper (`codex.opencodex-real`), neither of | ||
| * which embeds the Rust config schema. The real binary ships under the | ||
| * platform package's `vendor/<triple>/bin/codex`; also try adjacent wrappers. | ||
| */ | ||
| function codexNativeBinaryCandidates(command: string): string[] { | ||
| const out: string[] = [command]; | ||
| // The opencodex autostart shim (`codex`) forwards to `codex.opencodex-real` | ||
| // (the npm JS wrapper). Add the real wrapper's resolved native binary when it | ||
| // is discoverable on PATH, so probing the shim still reaches the Rust binary. | ||
| 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"); | ||
| if (existsSync(wrapper)) { | ||
| out.push(wrapper); | ||
| try { | ||
| const real = realpathSync(wrapper); | ||
| const pkgRoot = resolve(real, "..", ".."); | ||
| 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")); | ||
| } catch { | ||
| // keep the wrapper as a candidate | ||
| } | ||
| } | ||
| } | ||
| const managedRoot = process.env.CODEX_MANAGED_PACKAGE_ROOT; | ||
| if (managedRoot) { | ||
| out.push( | ||
| join(managedRoot, "node_modules", "@openai", "codex-darwin-arm64", "vendor", "aarch64-apple-darwin", "bin", "codex"), | ||
| join(managedRoot, "node_modules", "@openai", "codex-darwin-x64", "vendor", "x86_64-apple-darwin", "bin", "codex"), | ||
| join(managedRoot, "node_modules", "@openai", "codex-linux-x64", "vendor", "x86_64-unknown-linux-musl", "bin", "codex"), | ||
| join(managedRoot, "node_modules", "@openai", "codex-linux-arm64", "vendor", "aarch64-unknown-linux-musl", "bin", "codex"), | ||
| join(managedRoot, "node_modules", "@openai", "codex-win32-x64", "vendor", "x86_64-pc-windows-msvc", "bin", "codex.exe"), | ||
| join(managedRoot, "node_modules", "@openai", "codex-win32-arm64", "vendor", "aarch64-pc-windows-msvc", "bin", "codex.exe"), | ||
| ); | ||
| } | ||
| // Follow the resolved command to its real location. The opencodex shim and | ||
| // the npm JS wrapper resolve to `@openai/codex/bin/codex.js`; the native | ||
| // binary lives in the sibling platform package's vendor directory. | ||
| try { | ||
| const real = realpathSync(command); | ||
| const pkgRoot = resolve(real, "..", ".."); | ||
| for (const [pkg, triple, exe] of [ | ||
| ["codex-darwin-arm64", "aarch64-apple-darwin", "codex"], | ||
| ["codex-darwin-x64", "x86_64-apple-darwin", "codex"], | ||
| ["codex-linux-x64", "x86_64-unknown-linux-musl", "codex"], | ||
| ["codex-linux-arm64", "aarch64-unknown-linux-musl", "codex"], | ||
| ["codex-win32-x64", "x86_64-pc-windows-msvc", "codex.exe"], | ||
| ["codex-win32-arm64", "aarch64-pc-windows-msvc", "codex.exe"], | ||
| ] as const) { | ||
| out.push(join(pkgRoot, "node_modules", "@openai", pkg, "vendor", triple, "bin", exe)); | ||
| } | ||
| } catch { | ||
| // leave the raw command as the only candidate | ||
| } | ||
| return out; | ||
| } | ||
|
|
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick win
Consider deduplicating the repeated platform-package candidate list.
codexNativeBinaryCandidates builds the same set of codex-<platform>-<arch> package/vendor path segments three separate times: once for the wrapper-relative lookup (Line 851-854, missing the two win32 variants present elsewhere), once for CODEX_MANAGED_PACKAGE_ROOT (Line 862-869), and once for the resolved-command fallback (Line 877-884). Extract a single const PLATFORM_PACKAGES = [...] array (package name, target triple, executable name) and reuse it in all three loops. This removes the risk of the lists drifting out of sync — for example, the wrapper-relative branch already omits the Windows entries that the other two branches include.
♻️ Proposed refactor sketch
+const CODEX_PLATFORM_PACKAGES = [
+ ["codex-darwin-arm64", "aarch64-apple-darwin", "codex"],
+ ["codex-darwin-x64", "x86_64-apple-darwin", "codex"],
+ ["codex-linux-x64", "x86_64-unknown-linux-musl", "codex"],
+ ["codex-linux-arm64", "aarch64-unknown-linux-musl", "codex"],
+ ["codex-win32-x64", "x86_64-pc-windows-msvc", "codex.exe"],
+ ["codex-win32-arm64", "aarch64-pc-windows-msvc", "codex.exe"],
+] as const;Then reuse CODEX_PLATFORM_PACKAGES in all three places that currently spell out these tuples.
🤖 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 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.
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 👍 / 👎.
1 participant
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