feat(codex-auth): add default_mode_request_user_input feature toggle - #911
Conversation
Adds a Codex Auth page toggle for Codex's own default_mode_request_user_input feature flag. Enabling it adds [features] default_mode_request_user_input = true to \/config.toml via the official codex features CLI (format-preserving, removed when disabled), letting Codex pause a Default-mode session and ask the user questions with request_user_input. - features.ts: reads the flag from config.toml ([features] boolean form) - cli/v2.ts: codexFeaturesInvocation now takes any feature key - management API: GET/PUT /api/codex-auth/features/default-mode-request-user-input with deps injection and fail-loud 502 when the installed Codex build does not know the flag - GUI: DefaultModeRequestUserInputSetting card on the Codex Auth page, i18n in en/de/ja/ko/zh/ru - tests: config reader, API round-trip/validation/postcondition, GUI toggle tests - docs: reference configuration note
|
Warning Review limit reached
Next review available in: 18 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (12)
📝 WalkthroughWalkthroughAdded end-to-end support for Codex’s ChangesCodex default-mode request-user-input toggle
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant CodexAuthSetting
participant AgentSettingsRoutes
participant CodexFeatures
participant CodexCLI
CodexAuthSetting->>AgentSettingsRoutes: GET feature state
AgentSettingsRoutes->>CodexFeatures: Read config.toml state
CodexFeatures-->>AgentSettingsRoutes: Return enabled state and feature key
CodexAuthSetting->>AgentSettingsRoutes: PUT enabled boolean
AgentSettingsRoutes->>CodexCLI: Enable or disable feature
CodexCLI-->>AgentSettingsRoutes: Return command result
AgentSettingsRoutes->>CodexFeatures: Verify persisted state
AgentSettingsRoutes-->>CodexAuthSetting: Return state or HTTP 502
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![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: 03f4b557db
ℹ️ 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".
| } | ||
| if (url.pathname === "/api/codex-auth/features/default-mode-request-user-input" && req.method === "PUT") { | ||
| let body: { enabled?: unknown }; | ||
| try { body = await req.json(); } catch { return jsonResponse({ error: "invalid JSON body" }, 400); } |
There was a problem hiding this comment.
When this endpoint receives a chunked request without Content-Length, req.json() buffers the entire body and bypasses the management API's 4 MiB decompressed-body limit, allowing an oversized request to exhaust memory. Parse it with readManagementJsonBody(req) and rethrow DecompressedBodyTooLargeError, as the other management routes do, so the dispatcher can return 413.
AGENTS.md reference: src/AGENTS.md:L20-L20
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[shipping-github] Fixed in 3a324231c: the PUT now parses through readManagementJsonBody (4 MiB bound, size sentinel rethrown so the dispatcher returns 413) and rejects null/array/non-object bodies with 400 before touching enabled. Regression tests added for null, array, and oversized chunked bodies.
| const inv = codexFeaturesInvocation(enabled ? "enable" : "disable", DEFAULT_MODE_REQUEST_USER_INPUT_FEATURE_KEY); | ||
| execFileSync(inv.file, inv.args, | ||
| { stdio: ["ignore", "pipe", "pipe"], timeout: 15_000, windowsHide: true, ...inv.options }); |
There was a problem hiding this comment.
When opencodex is running in WSL without an explicit CODEX_HOME and auto-detects the sole Windows-mounted .codex directory, the reader checks that Windows config, but this child inherits no CODEX_HOME, so Codex uses the Linux ~/.codex instead. The toggle can therefore mutate the wrong config and then return 502 because its postcondition checks the unchanged Windows file; pass the resolved home in the child environment.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[shipping-github] Fixed in 3a324231c: runCodexFeaturesCommand now passes the reader's resolved home (dirname(activeCodexConfigPath())) as CODEX_HOME in the child environment, so the CLI toggles the same config the postcondition re-reads — including the WSL Windows-home detection path. No-op when the reader and CLI already agree.
| enabledRef.current = payload.enabled === true; | ||
| setEnabled(enabledRef.current); |
There was a problem hiding this comment.
If the 30-second GET starts just before the user toggles and its stale response resolves during or after the PUT, these assignments overwrite enabledRef and the rendered state with the pre-toggle value. The next click can then submit the opposite of the persisted state; track a request revision or defer/ignore reads that overlap a save, following the existing auto-switch controller's reconciliation pattern.
AGENTS.md reference: gui/AGENTS.md:L9-L10
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[shipping-github] Fixed in 3a324231c: load() now tracks a generation ref; GETs that were in flight when a save started are ignored after await, so a stale poll can no longer revert the optimistic state. New GUI test covers the overlap window (poll resolves mid-PUT).
| "codexAuth.autoSwitchUpdateFailed": "The usage-based switching update could not be confirmed. The last confirmed value is shown.", | ||
| "codexAuth.requestUserInput": "Ask for input in Default mode", | ||
| "codexAuth.requestUserInputDesc": "Lets Codex pause a Default-mode session and ask you questions with the request_user_input tool.", | ||
| "codexAuth.requestUserInputConfig": "config.toml: [features] default_mode_request_user_input = true", |
There was a problem hiding this comment.
This value presents [features] default_mode_request_user_input = true as one copyable line, but TOML requires the table header and assignment on separate lines, so pasting the displayed snippet makes config.toml invalid. Render a literal <pre>/<code> snippet containing [features]\ndefault_mode_request_user_input = true instead of duplicating this machine text across locale files.
AGENTS.md reference: gui/AGENTS.md:L22-L25
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[shipping-github] Fixed in 3a324231c: the card renders a literal two-line <code> snippet ([features]\ndefault_mode_request_user_input = true), and the duplicated machine text was removed from all six locale files per gui/AGENTS.md (technical/machine text does not belong in locale files).
| if (url.pathname === "/api/codex-auth/features/default-mode-request-user-input" && req.method === "PUT") { | ||
| let body: { enabled?: unknown }; | ||
| try { body = await req.json(); } catch { return jsonResponse({ error: "invalid JSON body" }, 400); } | ||
| if (typeof body.enabled !== "boolean") return jsonResponse({ error: "body.enabled must be a boolean" }, 400); |
There was a problem hiding this comment.
When a client sends the valid JSON body null, parsing succeeds and body.enabled then throws a TypeError; the management dispatcher only converts its specific bounded-body and busy errors, so this malformed request escapes as an internal server failure instead of a 400. Validate that the parsed value is a non-null plain object before reading enabled.
AGENTS.md reference: src/AGENTS.md:L17-L17
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
[shipping-github] Fixed in 3a324231c: covered by the same bounded-parser change as the body-size finding — a JSON null body now returns 400 before any property dereference. Regression test added.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 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/DefaultModeRequestUserInputSetting.tsx`:
- Around line 23-44: Guard the successful response handling in load so it does
not overwrite the optimistic toggle state while savingRef.current is true. In
the load callback, only update enabledRef.current and call setEnabled when no
save is in progress, while preserving hydration and existing error behavior.
- Around line 56-66: Update the PUT response handling in
DefaultModeRequestUserInputSetting to parse an optional warnings string array,
preserve the localized success message, and render any non-empty warnings
returned by the server. Extend the relevant component test to cover a response
containing at least one warning and verify it is displayed.
In `@src/server/management/agent-settings-routes.ts`:
- Around line 312-315: Update the PUT handler for
/api/codex-auth/features/default-mode-request-user-input to parse the request
through readManagementJsonBody() as unknown, preserving and rethrowing its size
sentinel. Validate that the parsed value is a non-null, non-array object before
accessing enabled, returning 400 for invalid bodies, and retain the boolean
validation for body.enabled. Add regression coverage for null and oversized
chunked bodies.
In `@tests/codex-v2-gate.test.ts`:
- Around line 893-898: Remove the duplicate deps object from both
handleManagementAPI calls in tests/codex-v2-gate.test.ts at lines 893-898 and
906-911, leaving each call with exactly the request, URL, config, and single
deps arguments.
🪄 Autofix (Beta)
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: 606bac8a-c953-4737-a8df-45c092ddc90e
📒 Files selected for processing (16)
docs-site/src/content/docs/reference/configuration/agents.mdgui/src/components/DefaultModeRequestUserInputSetting.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/CodexAuth.tsxgui/src/styles.cssgui/tests/codex-auth-request-user-input.test.tsxsrc/cli/v2.tssrc/codex/features.tssrc/server/management/agent-settings-routes.tssrc/server/management/context.tstests/codex-v2-gate.test.ts
The PUT response body was parsed before res.ok was checked, which react-doctor flags as no-fetch-response-used-without-status-check. Use the repo's readJsonOrThrow helper so non-OK responses are handled before the body is consumed (error copy still surfaces from the server payload).
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/DefaultModeRequestUserInputSetting.tsx`:
- Around line 62-63: Update the request-user-input update catch block around
readJsonOrThrow to preserve the caught error’s message, displaying it when
available and falling back to the localized requestUserInputUpdateFailed string
otherwise. Apply the same handling to both affected error paths while keeping
the existing failure flow intact.
🪄 Autofix (Beta)
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: 4087a4de-8fae-4793-ab63-9c6cae3786bb
📒 Files selected for processing (1)
gui/src/components/DefaultModeRequestUserInputSetting.tsx
…UI poll Extract runCodexFeaturesCommand so the /api/v2 and default_mode_request_user_input PUT handlers share one execFileSync fallback (same invocation builder, timeout, and stdio options). The GUI load poll now skips while a PUT is in flight so the 30s refresh cannot revert the optimistic toggle. Adds a POSIX invocation assertion for the new feature key.
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 (2)
gui/src/components/DefaultModeRequestUserInputSetting.tsx (1)
71-74: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve the server error detail in failure feedback.
readJsonOrThrowat Line 65 can contain the management API's502reason, but the empty catch discards it. An unsupported Codex build therefore produces only the generic localized message.Use the caught error message when available and fall back to
codexAuth.requestUserInputUpdateFailedwhen it is not. Add a test for the502response 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 `@gui/src/components/DefaultModeRequestUserInputSetting.tsx` around lines 71 - 74, The catch handling around readJsonOrThrow in DefaultModeRequestUserInputSetting must preserve server error details in the feedback message. Capture the caught error, use its message when available, and otherwise fall back to t("codexAuth.requestUserInputUpdateFailed"); retain the rollback behavior for enabledRef and setEnabled, and add coverage for the 502 response path.src/server/management/agent-settings-routes.ts (1)
319-325: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winPreserve Codex CLI stderr on toggle failures.
runCodexFeaturesCommanddoes not set an encoding, so Bun 1.3.14 exposes pipederror.stderras aBuffer. DecodeBuffer/Uint8Arrayvalues or setencoding: "utf8", then add a regression test that asserts the 502 response includes the CLI diagnostic.🤖 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/server/management/agent-settings-routes.ts` around lines 319 - 325, Update the error handling around runCodexFeaturesCommand in the toggle flow to preserve CLI diagnostics when error.stderr is a Buffer or Uint8Array by decoding it as UTF-8, or configure the command with encoding: "utf8". Keep the existing trimmed stderr/message fallback behavior, and add a regression test asserting the 502 response contains the CLI diagnostic.
♻️ Duplicate comments (3)
gui/src/components/DefaultModeRequestUserInputSetting.tsx (2)
65-70: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick winShow the new-session warning through localized UI state.
src/server/management/agent-settings-routes.ts, Lines 333-337, returnschangedand a non-emptywarningsarray when the setting changes. These lines omit both fields and always show only the success message. The user therefore receives no notice that the setting applies only to new sessions.Use
payload.changedor a stable warning code to select a new localized message. Do not render raw server warning text. Add a component test for the changed response.As per path instructions, GUI state changes must stay consistent with management API responses, and user-visible strings must go through the i18n locale files.
🤖 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/components/DefaultModeRequestUserInputSetting.tsx` around lines 65 - 70, Update the response handling in DefaultModeRequestUserInputSetting to read the management API’s changed or stable warning indicator and select a localized new-session warning when the setting changed, while retaining the existing success message otherwise. Do not render raw warning text; add the corresponding translation to the locale files and cover the changed-response behavior with a component test.Source: Path instructions
24-39: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winInvalidate in-flight loads when a save starts.
Line 27 checks
savingRef.currentonly beforefetch(). A GET that started before the click can resolve after the click and execute Lines 32-33 with stale server data. If it resolves after the PUT, it can overwrite the final state until the next poll.Track a load/save generation or abort the request. Check the generation after
await res.json()and before updating either state or load-error values.Proposed guard
+ const loadGenerationRef = useRef(0); + const load = useCallback(async () => { if (savingRef.current) return; + const generation = ++loadGenerationRef.current; try { const res = await fetch(`${apiBase}${FEATURE_ENDPOINT}`); if (!res.ok) throw new Error("load"); const payload = await res.json() as { enabled?: unknown }; + if (savingRef.current || generation !== loadGenerationRef.current) return; enabledRef.current = payload.enabled === true; setEnabled(enabledRef.current); setHydrated(true); setLoadError(false); } catch { - if (!savingRef.current) setLoadError(true); + if (!savingRef.current && generation === loadGenerationRef.current) setLoadError(true); } }, [apiBase]); const toggle = useCallback(async () => { if (savingRef.current || !hydrated || loadError) return; + loadGenerationRef.current++;As per path instructions, GUI state changes must stay consistent with management API responses.
🤖 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/components/DefaultModeRequestUserInputSetting.tsx` around lines 24 - 39, Invalidate in-flight loads when saving begins in the load callback and save flow of DefaultModeRequestUserInputSetting. Track a load/save generation or abort the GET, then verify it after await res.json() and before updating enabledRef, enabled, hydrated, or load-error state; preserve updates only for the current generation and keep state changes aligned with the management API response.Source: Path instructions
src/server/management/agent-settings-routes.ts (1)
307-310: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick winUse the bounded management parser for this PUT body.
Line 309 calls
req.json()directly. Chunked requests can therefore bypass the management body-size limit. A JSONnullbody also makes Line 310 throw instead of returning400.Parse as
unknownwithreadManagementJsonBody(), rethrow its size sentinel, reject null, arrays, and non-objects, then validateenabled. Add regression tests for null and oversized chunked bodies.Proposed fix
- let body: { enabled?: unknown }; - try { body = await req.json(); } catch { return jsonResponse({ error: "invalid JSON body" }, 400); } + let parsedBody: unknown; + try { + parsedBody = await readManagementJsonBody(req); + } catch (error) { + rethrowManagementBodyTooLarge(error); + return jsonResponse({ error: "invalid JSON body" }, 400); + } + if (!parsedBody || typeof parsedBody !== "object" || Array.isArray(parsedBody)) { + return jsonResponse({ error: "body must be a JSON object" }, 400); + } + const body = parsedBody as { enabled?: unknown };🤖 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/server/management/agent-settings-routes.ts` around lines 307 - 310, Update the PUT handler for /api/codex-auth/features/default-mode-request-user-input to parse the request through readManagementJsonBody() as unknown, preserving and rethrowing its body-size sentinel. Reject null, arrays, and non-object bodies with a 400 response before validating body.enabled as a boolean, and add regression coverage for null and oversized chunked payloads.
🤖 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/cli/v2.ts`:
- Around line 57-69: Update runCodexFeaturesCommand to execute the Codex process
asynchronously without blocking the Bun event loop, preserving the existing
invocation options and 15-second timeout. Propagate async/await through
transitionMultiAgentV2 and its callers, and perform feature-state verification
only after the bounded process completes.
---
Outside diff comments:
In `@gui/src/components/DefaultModeRequestUserInputSetting.tsx`:
- Around line 71-74: The catch handling around readJsonOrThrow in
DefaultModeRequestUserInputSetting must preserve server error details in the
feedback message. Capture the caught error, use its message when available, and
otherwise fall back to t("codexAuth.requestUserInputUpdateFailed"); retain the
rollback behavior for enabledRef and setEnabled, and add coverage for the 502
response path.
In `@src/server/management/agent-settings-routes.ts`:
- Around line 319-325: Update the error handling around runCodexFeaturesCommand
in the toggle flow to preserve CLI diagnostics when error.stderr is a Buffer or
Uint8Array by decoding it as UTF-8, or configure the command with encoding:
"utf8". Keep the existing trimmed stderr/message fallback behavior, and add a
regression test asserting the 502 response contains the CLI diagnostic.
---
Duplicate comments:
In `@gui/src/components/DefaultModeRequestUserInputSetting.tsx`:
- Around line 65-70: Update the response handling in
DefaultModeRequestUserInputSetting to read the management API’s changed or
stable warning indicator and select a localized new-session warning when the
setting changed, while retaining the existing success message otherwise. Do not
render raw warning text; add the corresponding translation to the locale files
and cover the changed-response behavior with a component test.
- Around line 24-39: Invalidate in-flight loads when saving begins in the load
callback and save flow of DefaultModeRequestUserInputSetting. Track a load/save
generation or abort the GET, then verify it after await res.json() and before
updating enabledRef, enabled, hydrated, or load-error state; preserve updates
only for the current generation and keep state changes aligned with the
management API response.
In `@src/server/management/agent-settings-routes.ts`:
- Around line 307-310: Update the PUT handler for
/api/codex-auth/features/default-mode-request-user-input to parse the request
through readManagementJsonBody() as unknown, preserving and rethrowing its
body-size sentinel. Reject null, arrays, and non-object bodies with a 400
response before validating body.enabled as a boolean, and add regression
coverage for null and oversized chunked payloads.
🪄 Autofix (Beta)
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: e3b06fb5-3148-4a72-a252-5a7147685ba0
📒 Files selected for processing (4)
gui/src/components/DefaultModeRequestUserInputSetting.tsxsrc/cli/v2.tssrc/server/management/agent-settings-routes.tstests/codex-v2-gate.test.ts
| /** | ||
| * Run `codex features <action> <feature>` synchronously - the management API | ||
| * fallback when no deps toggle is injected. Shares the invocation builder and | ||
| * the bounded timeout/stdio options so every production toggle path behaves | ||
| * identically. | ||
| */ | ||
| export function runCodexFeaturesCommand( | ||
| action: "enable" | "disable", | ||
| feature: string = "multi_agent_v2", | ||
| ): void { | ||
| const inv = codexFeaturesInvocation(action, feature); | ||
| execFileSync(inv.file, inv.args, | ||
| { stdio: ["ignore", "pipe", "pipe"], timeout: 15_000, windowsHide: true, ...inv.options }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- file inventory ---'
git ls-files 'src/cli/v2.ts' 'src/**' | sed -n '1,120p'
printf '%s\n' '--- outline ---'
ast-grep outline src/cli/v2.ts --view compact || true
printf '%s\n' '--- target source ---'
sed -n '1,180p' src/cli/v2.ts
printf '%s\n' '--- helper and call sites ---'
rg -n -C 4 'runCodexFeaturesCommand|codexFeaturesInvocation|execFileSync|features' srcRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- management route ---'
sed -n '205,305p' src/server/management/agent-settings-routes.ts
printf '%s\n' '--- feature transition implementation ---'
rg -n -A 100 -B 15 'export function transitionMultiAgentV2|function transitionMultiAgentV2' src/codex/features.ts
printf '%s\n' '--- route registration and handler shape ---'
rg -n -A 8 -B 8 'agent-settings-routes|handle.*Request|serve\\(|fetch\\(|agentSettings' src/server src
printf '%s\n' '--- relevant tests ---'
rg -n -C 5 'runCodexFeaturesCommand|toggleCodexMultiAgentV2|transitionMultiAgentV2|/api/v2' test tests src --glob '*test*' --glob '*spec*' || true
printf '%s\n' '--- read-only call-chain verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
v2 = Path("src/cli/v2.ts").read_text()
route = Path("src/server/management/agent-settings-routes.ts").read_text()
features = Path("src/codex/features.ts").read_text()
checks = {
"v2 imports execFileSync": r'import\s+\{\s*execFileSync\s*\}\s+from\s+"node:child_process"',
"management fallback imports runCodexFeaturesCommand": r'const\s+\{\s*runCodexFeaturesCommand\s*\}\s*=\s*await import\("\.\./\.\./cli/v2"\)',
"fallback invokes synchronously": r'toggle\s*=\s*\(enabled:\s*boolean\)\s*=>\s*runCodexFeaturesCommand',
"runCodexFeaturesCommand invokes execFileSync": r'export function runCodexFeaturesCommand[\s\S]{0,500}?execFileSync\(',
"sync timeout is 15 seconds": r'runCodexFeaturesCommand[\s\S]{0,600}?timeout:\s*15_000',
"transition accepts synchronous toggle": r'export function transitionMultiAgentV2[\s\S]{0,500}?toggle',
}
for name, pattern in checks.items():
source = route if "management" in name or "fallback" in name else v2 if "v2" in name or "runCodex" in name or "timeout" in name else features
print(f"{name}: {'YES' if re.search(pattern, source) else 'NO'}")
m = re.search(r'if \(requestedFlag !== undefined \|\| wantsThreads\) \{([\s\S]{0,2500})', route)
if m:
body = m.group(1)
print("route transition block contains await before transition:",
"YES" if re.search(r'\bawait\b', body[:body.find("transitionMultiAgentV2")] if "transitionMultiAgentV2" in body else "") else "NO")
PYRepository: lidge-jun/opencodex
Length of output: 12261
🏁 Script executed:
#!/bin/bash
set -eu
printf '%s\n' '--- route function and request dispatch ---'
sed -n '1,90p' src/server/management/agent-settings-routes.ts
sed -n '300,345p' src/server/management/agent-settings-routes.ts
rg -n -F -C 6 'agent-settings-routes' src/server src
rg -n -F -C 6 'handleManagement' src/server src
printf '%s\n' '--- exact source line numbers ---'
nl -ba src/cli/v2.ts | sed -n '50,90p'
nl -ba src/server/management/agent-settings-routes.ts | sed -n '224,290p'
nl -ba src/codex/features.ts | sed -n '911,987p'
printf '%s\n' '--- corrected read-only call-chain verifier ---'
python3 - <<'PY'
from pathlib import Path
import re
v2 = Path("src/cli/v2.ts").read_text()
route = Path("src/server/management/agent-settings-routes.ts").read_text()
features = Path("src/codex/features.ts").read_text()
checks = [
("v2 imports execFileSync", v2, r'import\s+\{\s*execFileSync\s*\}\s+from\s+"node:child_process"'),
("management fallback imports runCodexFeaturesCommand", route, r'runCodexFeaturesCommand\s*\}\s*=\s*await import\("\.\./\.\./cli/v2"\)'),
("management fallback invokes runCodexFeaturesCommand", route, r'toggle\s*=\s*\(enabled:\s*boolean\)\s*=>\s*runCodexFeaturesCommand'),
("runCodexFeaturesCommand invokes execFileSync", v2, r'export function runCodexFeaturesCommand[\s\S]{0,600}?execFileSync\('),
("runCodexFeaturesCommand has 15 second timeout", v2, r'export function runCodexFeaturesCommand[\s\S]{0,700}?timeout:\s*15_000'),
("transition toggle parameter is synchronous", features, r'toggleFeature:\s*\(enabled:\s*boolean\)\s*=>\s*void'),
("transition calls toggle before postcondition", features, r'toggleFeature\(true\)[\s\S]{0,160}?isMultiAgentV2Enabled\(path\)'),
]
for name, source, pattern in checks:
print(f"{name}: {'YES' if re.search(pattern, source) else 'NO'}")
transition_start = features.index("export function transitionMultiAgentV2")
transition = features[transition_start:]
print("transition contains await:", "YES" if re.search(r"\bawait\b", transition[:transition.find("\n}") if "\n}" in transition else len(transition)]) else "NO")
PYRepository: lidge-jun/opencodex
Length of output: 22265
Move Codex feature execution off the Bun event loop.
src/server/management/agent-settings-routes.ts:235-240 calls runCodexFeaturesCommand through synchronous transitionMultiAgentV2. That helper calls execFileSync with a 15-second timeout, blocking unrelated requests while Codex runs. Make the process execution asynchronous, or move it to a worker. Propagate await through transitionMultiAgentV2 and perform state verification only after the bounded process result is available.
🤖 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/cli/v2.ts` around lines 57 - 69, Update runCodexFeaturesCommand to
execute the Codex process asynchronously without blocking the Bun event loop,
preserving the existing invocation options and 15-second timeout. Propagate
async/await through transitionMultiAgentV2 and its callers, and perform
feature-state verification only after the bounded process completes.
Source: Path instructions
There was a problem hiding this comment.
[shipping-github] Declined with rationale: the synchronous execFileSync (15s bound) is the pre-existing contract shared with the /api/v2 toggle through transitionMultiAgentV2; making it async would ripple through the CLI command path and the existing route. This PR's new endpoint deliberately matches the established subsystem pattern. Tracked as a follow-up recommendation in the full review rather than an in-PR refactor.
There was a problem hiding this comment.
@Wibias Understood. The new endpoint follows the existing synchronous feature-toggle contract, including the 15-second bound.
The event-loop blocking concern remains a valid follow-up item, but it is outside the scope of this PR. No change is required here.
🐇
You are interacting with an AI system.
[shipping-github] Verdict: approve-commentPR: Semantic propagation
Linked: none UsefulnessReal value: exposes an otherwise unmanageable upstream Codex flag through the Codex Auth page, keeps the TOML edit upstream-owned via Bugs / correctness
Security
Spec / standards
Reviews
Base / CI
Gatenone - not draft/WIP. Merge still awaits one maintainer approval per repository policy. Bottom lineUseful, well-tested, and now hardened: all confirmed findings fixed and verified, CI green on the current head, base synchronized. Ready for the required maintainer review. |
1 participant
Adds a Codex Auth page toggle for Codex's own
default_mode_request_user_inputfeature flag. Enabling it adds[features] default_mode_request_user_input = trueto$CODEX_HOME/config.tomlvia the officialcodex features enable|disableCLI (format-preserving, removed again when disabled), which lets Codex pause a Default-mode session and ask the user questions with therequest_user_inputtool.What changed
src/codex/features.ts— newisDefaultModeRequestUserInputEnabled()reader for the[features]boolean form plus a sharedDEFAULT_MODE_REQUEST_USER_INPUT_FEATURE_KEYconstant. Missing file/key reads as false, matching the upstream default.src/cli/v2.ts—codexFeaturesInvocationnow accepts any feature key (multi_agent_v2remains the default), so the same win-exec/.cmdshim handling and runtime resolution serve both flags.src/server/management/agent-settings-routes.ts— newGET/PUT /api/codex-auth/features/default-mode-request-user-input. PUT flips the flag throughcodex features(deps-injectable for tests) and returns 502 with the CLI's stderr when the installed Codex build does not know the flag yet, instead of silently writing an unknown key.src/server/management/context.ts—toggleDefaultModeRequestUserInputdeps slot.DefaultModeRequestUserInputSettingcard on the Codex Auth page showing the exactconfig.tomlline it manages, with optimistic toggle, rollback on failure, load/retry state, and 30s polling. Copy is localized in en/de/ja/ko/zh/ru; new CSS follows the existing auto-switch card pattern.Why
default_mode_request_user_inputis an under-development upstream codex-rs feature (default off) that is not exposed anywhere in opencodex. The Codex Auth page is the natural surface: one switch, and the flag line is added to or removed fromconfig.tomlby the official CLI so the TOML edit stays upstream-owned and format-preserving.Testing
bun run typecheck,bun run lint:gui,bun run lint:i18n,bun run privacy:scan,bun run build:guiall green; GUI suite green.[features] default_mode_request_user_input = true, disable removes it.Notes
Summary by CodeRabbit
New Features
Documentation
Bug Fixes