fix(agents): add opt-in encrypted v2 recovery - #1540
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (4)
📝 WalkthroughWalkthroughAdds experimental, disabled-by-default recovery for unreadable encrypted v2 routed tasks. The change validates configuration and caller credentials, recovers assignments through an authenticated ChatGPT endpoint, caches results, integrates recovery into Responses routing, prevents plaintext persistence, and documents the behavior. ChangesEncrypted agent-task recovery
Estimated code review effort: 4 (Complex) | ~75 minutes Sequence Diagram(s)sequenceDiagram
participant Client as Native ChatGPT client
participant Responses as Responses pipeline
participant Recovery as recoverEncryptedAgentTask
participant Cache as Recovery cache
participant ChatGPT as ChatGPT recovery endpoint
participant Provider as Routed provider
Client->>Responses: Submit encrypted v2 task
Responses->>Responses: Select final noncanonical route
Responses->>Recovery: Attempt recovery
Recovery->>Cache: Resolve scoped ciphertext
Cache->>ChatGPT: Send authenticated recovery request
ChatGPT-->>Cache: Stream assignment
Cache-->>Recovery: Return plaintext assignment
Recovery-->>Responses: Rewrite task as user message
Responses->>Provider: Forward rewritten request
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 |
|
✅ Deterministic PR hygiene checks passed. |
⏳ DRAFT
What to do
Review readiness checklist
2/4 boxes ticked. Automatic draft conversion failed. Please convert this pull request to a draft manually until every box above is ticked. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 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/configuration/agents.md`:
- Line 182: Update the recovery-path description to use the hyphenated compound
modifier “direct-routed children” instead of “direct routed children.”
- Around line 124-145: Update the ChatGPT recovery documentation in
docs-site/src/content/docs/reference/configuration/agents.md lines 124-145 to
state that recovery uses a raw passthrough to the fixed /responses endpoint with
forward-mode authentication, and explicitly list the allowed authorization,
ChatGPT account ID, and approved OpenAI/Codex metadata headers while preserving
the credential-safety restrictions. Add the same transport and safe header
allowlist, or an explicit link to equivalent wording, in
docs-site/src/content/docs/guides/sub-agent-surface.md lines 134-140.
In `@src/server/index.ts`:
- Around line 484-488: Add focused regression coverage near the existing
startServer tests for the startup warning block in startServer: capture
console.warn, verify all three warnings are emitted only when
config.agentTaskRecovery.enabled is exactly true, and assert the captured output
does not contain any credential value.
In `@src/server/responses/agent-task-recovery.ts`:
- Around line 189-192: Update the recovery replacement logic around the
encrypted content assignment to use String.replace with a replacer function,
ensuring recovered plaintext containing `$` patterns is inserted literally. Add
a focused regression test near the existing agent-task recovery tests using an
embedded token and an assignment containing `$&`, and assert the forwarded
provider body preserves the exact payload.
In `@tests/agent-task-recovery.test.ts`:
- Around line 360-387: Replace the single setImmediate barrier in the
“deduplicates concurrent recovery for the same scoped task” test with a
deterministic wait that observes the second request reaching the in-flight
recovery lookup before calling releaseRecovery. Ensure the barrier is tied to
observable request/recovery state, so both post calls join the same recovery
entry before the gate is released, while preserving the existing fetch-count
assertions.
In `@tests/helpers/agent-task-recovery.ts`:
- Around line 160-176: Extend encryptedInput with an optional wrapper
configuration for encrypted_content, applying it around the selected ciphertext
token when provided while preserving the current bare-token default. Use this
fixture option to construct values such as “prefix <token> suffix” so tests can
exercise the embedded-token branch in injectAssignment.
🪄 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: e1202a26-936a-4727-8b45-b5edc928ff50
📒 Files selected for processing (19)
docs-site/src/content/docs/guides/sub-agent-surface.mddocs-site/src/content/docs/ja/guides/sub-agent-surface.mddocs-site/src/content/docs/ko/guides/sub-agent-surface.mddocs-site/src/content/docs/reference/configuration/agents.mddocs-site/src/content/docs/ru/guides/sub-agent-surface.mddocs-site/src/content/docs/zh-cn/guides/sub-agent-surface.mddocs-site/src/content/docs/zh-tw/guides/sub-agent-surface.mdsrc/config.tssrc/server/index.tssrc/server/responses/agent-task-recovery-cache.tssrc/server/responses/agent-task-recovery.tssrc/server/responses/core.tssrc/server/responses/encrypted-payload.tssrc/types.tstests/agent-task-recovery-cache.test.tstests/agent-task-recovery-security.test.tstests/agent-task-recovery.test.tstests/config.test.tstests/helpers/agent-task-recovery.ts
|
@coderabbitai review |
✅ Action performedReview finished.
|
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 `@docs-site/src/content/docs/reference/configuration/agents.md`:
- Around line 146-148: The recovery documentation must describe the
loopback-only proxy boundary and the native credential path using the canonical
openai provider with authMode "forward", without fallback to API keys, another
provider, or another Codex account. Update
docs-site/src/content/docs/reference/configuration/agents.md lines 138-148 and
mirror the same guidance in
docs-site/src/content/docs/guides/sub-agent-surface.md lines 134-143, preserving
only the recovery header allowlist—authorization, matching chatgpt-account-id,
originator, optional openai-beta, and optional user-agent—and stating that
content-type and accept are generated locally rather than documenting the
broader FORWARD_HEADERS set.
In `@src/server/index.ts`:
- Line 486: Update the warning in the startup guidance around the scoped cache
miss message to use conditional wording: state that a miss may send an
additional authenticated ChatGPT request, and note that concurrent callers can
share requests. Keep the existing quota and latency warning while avoiding
claims that every miss creates an upstream request.
🪄 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: d1efe020-5f78-4e00-b39f-9233a10b5674
📒 Files selected for processing (6)
docs-site/src/content/docs/guides/sub-agent-surface.mddocs-site/src/content/docs/reference/configuration/agents.mdsrc/server/index.tssrc/server/responses/agent-task-recovery-cache.tssrc/server/responses/agent-task-recovery.tstests/agent-task-recovery.test.ts
eb7fb65 to
253b8b5
Compare
|
Tip For best results, initiate chat on the files or code changes.
The 🧠 Learnings usedYou are interacting with an AI system. |
|
Tip For best results, initiate chat on the files or code changes.
The 🧠 Learnings usedYou are interacting with an AI system. |
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/server/index.ts`:
- Line 487: Update the console.warn message at the recovery-cache warning to
explicitly identify the retained value as recovered plaintext assignment data
and state that it is kept only in the process-local bounded in-memory cache.
Preserve the existing caveat about fidelity and undocumented backend behavior.
In `@src/server/responses/core.ts`:
- Around line 1734-1735: Remove the redundant buildToolBridgeMaps call in the
recovery path after parsed is replaced by reparsed. Preserve the original
toolBridgeMaps created during the initial parse, since recovery only replaces
the encrypted content and must not charge translatorBudget again.
In `@src/types.ts`:
- Around line 780-789: Restrict agentTaskRecovery.model in the configuration
schema and agentTaskRecoveryConfig normalizer to one shared explicit allowlist
of supported bare ChatGPT models, including gpt-5.6-sol, instead of accepting
arbitrary non-empty strings. Preserve the enabled gate, numeric bounds, and
runtime defaults, and add persisted-configuration tests covering accepted
allowlisted values and rejection or normalization of provider-qualified and
unsupported models.
🪄 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: d3c44934-8906-40a0-a6c8-81bdd1df9e39
📒 Files selected for processing (5)
docs-site/src/content/docs/guides/sub-agent-surface.mddocs-site/src/content/docs/reference/configuration/agents.mdsrc/server/index.tssrc/server/responses/core.tssrc/types.ts
| if (config.agentTaskRecovery?.enabled !== true) return; | ||
| console.warn("⚠️ Experimental encrypted V2 task recovery is enabled."); | ||
| console.warn(" A scoped cache miss may send an additional authenticated request to ChatGPT and may consume quota or add latency; concurrent misses can share one request."); | ||
| console.warn(" Recovered model output is retained only in a bounded in-memory cache; exact fidelity is not guaranteed and the path depends on undocumented backend behavior."); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor | ⚡ Quick win
Name the retained value as plaintext assignment data.
Line 487 calls the retained value “Recovered model output.” The recovery cache is a ciphertext-to-assignment cache, so this wording can make operators underestimate that recovered assignment plaintext remains in memory.
Use precise wording that identifies the data and its process-local scope.
Based on learnings: “injectAssignment replaces the complete encrypted part directly,” so the warning should identify the retained value as recovered plaintext assignment data.
Proposed wording
- console.warn(" Recovered model output is retained only in a bounded in-memory cache; exact fidelity is not guaranteed and the path depends on undocumented backend behavior.");
+ console.warn(" Recovered plaintext assignment data is retained only in a bounded, process-local in-memory cache; exact fidelity is not guaranteed and the path depends on undocumented backend behavior.");📝 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.
| console.warn(" Recovered model output is retained only in a bounded in-memory cache; exact fidelity is not guaranteed and the path depends on undocumented backend behavior."); | |
| console.warn(" Recovered plaintext assignment data is retained only in a bounded, process-local in-memory cache; exact fidelity is not guaranteed and the path depends on undocumented backend behavior."); |
🤖 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/index.ts` at line 487, Update the console.warn message at the
recovery-cache warning to explicitly identify the retained value as recovered
plaintext assignment data and state that it is kept only in the process-local
bounded in-memory cache. Preserve the existing caveat about fidelity and
undocumented backend behavior.
Source: Learnings
| parsed = reparsed; | ||
| toolBridgeMaps = buildToolBridgeMaps(parsed, translatorBudget); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not charge the translator budget twice.
buildToolBridgeMaps() charges retained bytes to translatorBudget. The initial parse already created toolBridgeMaps. Line 1735 recreates the same maps after recovery, although recovery changes only the encrypted content part. A request near the translation limit can then fail closed as unreadable_encrypted_agent_task only because recovery duplicated the retained-budget charge.
Keep the original toolBridgeMaps after reparsing.
Proposed fix
}
parsed = reparsed;
- toolBridgeMaps = buildToolBridgeMaps(parsed, translatorBudget);
} catch {Based on learnings: recovery rejects mixed slots and injectAssignment replaces the complete encrypted part directly.
📝 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.
| parsed = reparsed; | |
| toolBridgeMaps = buildToolBridgeMaps(parsed, translatorBudget); | |
| parsed = reparsed; |
🤖 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/responses/core.ts` around lines 1734 - 1735, Remove the redundant
buildToolBridgeMaps call in the recovery path after parsed is replaced by
reparsed. Preserve the original toolBridgeMaps created during the initial parse,
since recovery only replaces the encrypted content and must not charge
translatorBudget again.
Source: Learnings
| /** Experimental, default-off ChatGPT recovery for encrypted V2 routed tasks. */ | ||
| agentTaskRecovery?: { | ||
| enabled?: boolean; | ||
| /** ChatGPT model used by the recovery request. Default: gpt-5.6-sol. */ | ||
| model?: string; | ||
| /** Recovery request timeout in milliseconds. Default: 45000. */ | ||
| timeoutMs?: number; | ||
| /** Maximum in-memory ciphertext-to-assignment entries. Default: 200. */ | ||
| cacheEntries?: number; | ||
| }; |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline src/config.ts --items all --type function --view expanded || true
printf '%s\n' '--- recovery configuration references ---'
rg -n -C 8 \
'agentTaskRecovery|agentTaskRecoveryConfig|warnDegradedAgentTaskRecovery|recovery.*model|candidate' \
src/config.ts src/server/responses/agent-task-recovery.ts src/server/responses/core.ts tests || trueRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- config declarations and helpers ---'
rg -n -C 12 'agentTaskRecovery|function (getDefaultConfig|validateConfigCandidate|loadConfig)|configSchema' src/config.ts
printf '%s\n' '--- runtime normalization and use ---'
rg -n -C 15 'agentTaskRecoveryConfig|warnDegradedAgentTaskRecovery|agentTaskRecovery' src/server/responses/core.ts src/server/responses/agent-task-recovery.ts
printf '%s\n' '--- focused configuration tests ---'
sed -n '430,500p' tests/config.test.tsRepository: lidge-jun/opencodex
Length of output: 26635
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- recovery implementation ---'
sed -n '1,180p' src/server/responses/agent-task-recovery.ts
sed -n '180,380p' src/server/responses/agent-task-recovery.ts
printf '%s\n' '--- recovery model call sites ---'
rg -n -C 12 'recovery\.model|options\.model|model:' src/server/responses/agent-task-recovery.ts
printf '%s\n' '--- config type and schema imports ---'
rg -n -C 5 'agentTaskRecovery' src/types.ts src/config.ts
printf '%s\n' '--- tests for model restrictions/default normalization ---'
rg -n -C 8 'gpt-5\.6-sol|provider-qualified|qualified.*model|agentTaskRecoveryConfig|timeoutMs|cacheEntries' tests src/serverRepository: lidge-jun/opencodex
Length of output: 50376
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- recovery source with line numbers ---'
nl -ba src/server/responses/agent-task-recovery.ts | sed -n '1,180p'
printf '%s\n' '--- recovery source continuation ---'
nl -ba src/server/responses/agent-task-recovery.ts | sed -n '180,340p'
printf '%s\n' '--- exact recovery-specific tests ---'
rg -n -C 5 'agentTaskRecoveryConfig|agentTaskRecovery|recoverEncryptedAgentTask' tests \
-g '*agent-task*' -g '*config*' -g '*recovery*' | head -n 500Repository: lidge-jun/opencodex
Length of output: 239
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- recovery source ---'
awk '{printf "%6d %s\n", NR, $0}' src/server/responses/agent-task-recovery.ts | sed -n '1,180p'
awk '{printf "%6d %s\n", NR, $0}' src/server/responses/agent-task-recovery.ts | sed -n '180,340p'
printf '%s\n' '--- recovery-specific tests ---'
rg -n -C 5 'agentTaskRecoveryConfig|agentTaskRecovery|recoverEncryptedAgentTask' tests \
-g '*agent-task*' -g '*config*' -g '*recovery*' | head -n 500Repository: lidge-jun/opencodex
Length of output: 22886
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '%s\n' '--- recovery request and model handling ---'
awk '{printf "%6d %s\n", NR, $0}' src/server/responses/agent-task-recovery.ts | sed -n '320,520p'
printf '%s\n' '--- startup warning and recovery tests ---'
awk '{printf "%6d %s\n", NR, $0}' tests/agent-task-recovery.test.ts | sed -n '90,180p'
awk '{printf "%6d %s\n", NR, $0}' tests/agent-task-recovery.test.ts | sed -n '180,360p'
printf '%s\n' '--- read-only contract probe ---'
python3 - <<'PY'
from pathlib import Path
source = Path("src/config.ts").read_text()
recovery = Path("src/server/responses/agent-task-recovery.ts").read_text()
schema = source[source.index("const agentTaskRecoverySchema"):source.index("const configSchema")]
normalizer = recovery[recovery.index("export function agentTaskRecoveryConfig"):recovery.index("interface AgentEnvelope")]
print("schema_model_validator=", "z.string().trim().min(1)" in schema)
print("schema_model_literal_or_enum=", any(x in schema for x in ("z.literal", "z.enum", "z.union")))
print("schema_timeout_bounds=", "min(1_000).max(120_000)" in schema)
print("schema_cache_bounds=", "min(1).max(512)" in schema)
print("normalizer_enabled_true_only=", "raw.enabled !== true" in normalizer)
print("normalizer_default_model=", '"gpt-5.6-sol"' in normalizer)
print("normalizer_default_timeout=", "45_000" in normalizer)
print("normalizer_default_cache=", "200" in normalizer)
print("recovery_payload_uses_configured_model=", "model," in recovery[recovery.index("function recoveryPayload"):recovery.index("function sseDataPayloads")])
PYRepository: lidge-jun/opencodex
Length of output: 18137
Restrict agentTaskRecovery.model to supported bare ChatGPT models.
src/config.ts:1135 accepts any non-empty string, and agentTaskRecoveryConfig passes it to the fixed ChatGPT endpoint at src/server/responses/agent-task-recovery.ts:41-43,396. Provider-qualified or arbitrary values are not routed through configured providers and can cause invalid recovery requests. Use one explicit allowlist in the schema and normalizer, including gpt-5.6-sol, and add persisted-config tests. The enabled === true gate, numeric bounds, and runtime defaults are already enforced.
🤖 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/types.ts` around lines 780 - 789, Restrict agentTaskRecovery.model in the
configuration schema and agentTaskRecoveryConfig normalizer to one shared
explicit allowlist of supported bare ChatGPT models, including gpt-5.6-sol,
instead of accepting arbitrary non-empty strings. Preserve the enabled gate,
numeric bounds, and runtime defaults, and add persisted-configuration tests
covering accepted allowlisted values and rejection or normalization of
provider-qualified and unsupported models.
Source: Path instructions
|
Tip For best results, initiate chat on the files or code changes.
The 🧠 Learnings usedYou are interacting with an AI system. |
Wibias
left a comment
Wibias
left a comment
There was a problem hiding this comment.
Blocking on one verified correctness issue plus branch freshness. The recovery path reparses the request and rebuilds toolBridgeMaps with the same translatorBudget. buildToolBridgeMaps charges retained bytes for the tool maps, so this double-charges the same tools and can make a near-limit request fail only because recovery ran. Recovery changes the encrypted task content, not the tool definitions, so preserve the original toolBridgeMaps after parsed = reparsed. Also update onto current dev (c125b612); this head has diverged since 494ce1d. Minor, non-blocking: the startup warning should call the cached value recovered plaintext assignment data rather than generic model output. I do not consider an explicit recovery-model allowlist necessary as a blocker: arbitrary model IDs still go only to the fixed ChatGPT endpoint and fail closed if unsupported.
| } | ||
| } | ||
| parsed = reparsed; | ||
| toolBridgeMaps = buildToolBridgeMaps(parsed, translatorBudget); |
There was a problem hiding this comment.
This rebuild charges translatorBudget a second time for the same tool definitions. buildToolBridgeMaps() calls chargeRetained() for namespaced, freeform, and tool-search entries. Recovery only replaces the encrypted task content, so keep the original toolBridgeMaps and only assign parsed = reparsed here.
Routed providers cannot consume backend-encrypted native Codex task payloads. Add a disabled-by-default recovery path that uses only the fixed authenticated ChatGPT Codex endpoint and fails closed for unsupported callers. Refs lidge-jun#92
Document the exact credential boundary, cover startup warnings and shared-flight admission, and reject mixed encrypted slots before mutation. Refs lidge-jun#92
Make the loopback and credential constraints explicit in operator docs, and describe shared recovery flights accurately at startup. Refs lidge-jun#92
…nuation cache Recovery mutates the request input in place, so the recovered plaintext becomes _rawBody. The non-streaming and streaming paths then hand _rawBody to rememberResponseState, which stores the input and schedules a snapshot write to responses-state.json. Decrypted task text therefore reached disk with no TTL, contradicting the in-memory 15-minute cache the recovery path documents. Bar the body from the continuation cache instead. The marker is a WeakSet keyed on the body object rather than a field, because _rawBody is serialized verbatim by the native passthrough and a field would be sent upstream. The check lives in rememberResponseState so every recording path inherits it.
253b8b5 to
68354a6
Compare
Review: LAND (one High fixed on top)Threat-model reviewed against the current tree with an independent reviewer, then verified on a Linux CI host. The three contributor commits are preserved and rebased onto current The default-off claim holdsAn untouched config genuinely cannot reach this code. The field is optional in the schema ( The credential boundary holds, with one wording correctionAPI-key callers and proxy admission bearers are rejected, the JWT is checked for structure, issuer, audience, Codex OAuth client, expiry/not-before and account-header match, the forwarded header set is a freshly constructed allowlist, and the destination is a compiled-in constant. Two things worth stating precisely rather than as written: The JWT signature is not verified locally. The code validates untrusted claims and leans on the pinned ChatGPT endpoint as the actual signature authority. That is a defensible design — a forged token gets nothing back — but "accepts only a native Codex credential" describes the endpoint's behavior, not this process's. The loopback determination is derived from the configured bind policy via On redirects: this path uses The High I fixed: recovered plaintext reached diskThe recovery cache is carefully built — 15-minute TTL, byte and entry caps, single-flight dedup, no logging, no plaintext in thrown errors. All of that is undone one layer up.
rememberResponseState(parsed._rawBody, json, continuationStateForResponse(providerState), ...)which stores Worth being clear that The fix bars the body from the continuation cache instead of patching call sites. Two decisions I want on the record: The marker is a The check lives inside VerificationLinux CI host, Bun 1.3.14, at the pushed head
Red-before was proven by ablation: with the new tests kept and only the one-line guard removed, the suite goes to 2 fail — the non-persistence assertion and the sibling-isolation assertion — and back to 100/100 with it restored. The third test in that group deliberately asserts an unmarked body of identical shape still IS stored, so the guard cannot pass by simply disabling the cache. |
![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: 68354a6b4a
ℹ️ 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".
| (reparsed as unknown as Record<string, unknown>)[key] = parsed[key]; | ||
| } | ||
| } | ||
| parsed = reparsed; |
There was a problem hiding this comment.
When the routed primary is cooling down or otherwise marked unavailable and the configured fallback is another routed model, the earlier applySubagentModelFallback call skips every non-native candidate because unreadableEncryptedAgentTask is still true. After recovery makes the task readable, this block reparses it but never re-runs fallback selection, so the request is dispatched to the unavailable primary rather than the healthy routed fallback. Reapply fallback without the native-only restriction after successful recovery, and update route to match the newly selected model.
Useful? React with 👍 / 👎.
3 participants
Summary
NEW_TASKmessages whose payload is backend-encrypted and cannot be routed to a third-party provider.unreadable_encrypted_agent_taskgate. Native passthrough, v1 plaintext tasks, combos, non-Codex callers, and ordinary routed requests keep their existing paths.This is an experimental compatibility path, not a wire-protocol fix. It does not claim byte-for-byte fidelity and does not close the underlying Codex limitation tracked in openai/codex#36376. The plaintext policy remains tracked separately in openai/codex#37197.
Refs #92
Maintainer discussion accepted this shape for a draft: #92 (comment)
Security boundary
https://chatgpt.com/backend-api/codex/responsesendpoint; redirects and retries are disabled.Verification
bun scripts/test.ts tests/agent-task-recovery.test.ts tests/agent-task-recovery-security.test.ts tests/agent-task-recovery-cache.test.ts tests/config.test.ts tests/cancel-body-on-abort.test.ts— 183 passed, 0 failed on currentdevbase.bun run prepush— passed on exact head253b8b551rebased ontodev494ce1db2: typecheck, 11,382 tests passed, 8 skipped, 0 failed across 701 files, privacy scan passed, and the conditional GUI checks correctly skipped because the diff does not touchgui/.git diff --check— passed.cd docs-site && bun install --frozen-lockfile && bun run build— passed; 265 pages built.gpt-5.6-solparent spawned routedopencode-go/deepseek-v4-flash; child received the recovered canary task and completed through the routed provider.Exact-head repository CI on supported platforms and explicit maintainer security review are still required before merge.
Checklist
Review readiness checklist
This PR stays in draft until every box below is ticked. Tick all four boxes once the requirements are met:
All CI tests are green on my local testing.
I pushed my PR to the latest dev commit.
I resolved all correct Codex and CodeRabbit findings.
My PR is ready for review.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation