fix(v2): harden unreadable routed agent tasks - #336
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c454af188f
ℹ️ 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".
c454af1 to
733cec4
Compare
📝 WalkthroughWalkthroughThe change structurally validates embedded Fernet payloads, refines unreadable agent-task detection, and restricts combo routing and retries to decrypt-capable native OpenAI targets with a shared error response. English and Chinese documentation describe the resulting delivery and recovery rules. ChangesEncrypted agent-task routing
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant Client
participant handleResponses
participant handleComboResponses
participant pickComboTarget
participant NativeOpenAI as canonical OpenAI forward provider
Client->>handleResponses: submit parsed agent task
handleResponses->>handleComboResponses: pass unreadable-task status
handleComboResponses->>pickComboTarget: select decryptable target
pickComboTarget->>NativeOpenAI: forward encrypted task
NativeOpenAI-->>handleComboResponses: response or failure
handleComboResponses->>pickComboTarget: retry with payload eligibility
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
87af85f to
9f75de0
Compare
|
Maintainer takeover update for PR #336:
This is a fail-fast mitigation for issue #92; it does not solve or close #92. Thank you, @MathiasHeinke, for the contribution. |
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 `@docs-site/src/content/docs/guides/sub-agent-surface.md`:
- Line 9: Synchronize the translated sub-agent surface documentation in the
Japanese, Korean, and Russian guides with the current English/zh-cn behavior.
Replace the outdated “task text can be lost” description with the
`unreadable_encrypted_agent_task` failure for direct native-to-non-native
routes, and document that combos prefer a decrypt-capable native ChatGPT target.
🪄 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: 20a5eaf5-367c-4a25-8411-6dd4baa1a1ef
📒 Files selected for processing (8)
docs-site/src/content/docs/guides/sub-agent-surface.mddocs-site/src/content/docs/zh-cn/guides/sub-agent-surface.mdsrc/combos/resolve.tssrc/server/responses/core.tssrc/server/responses/encrypted-payload.tstests/combos.test.tstests/multi-agent-compat.test.tstests/v2-agent-message-failfast.test.ts
|
|
||
| :::note | ||
| On the v2 surface (`multi_agent_v2`), a spawned sub-agent inherits the parent model **by default**: `fork_turns` defaults to `all`, and full-history forks reject overrides. Since v2.7.2 opencodex injects guidance that teaches the model how to break inheritance — a `spawn_agent` call that sets `fork_turns` to `"none"` (or a partial fork such as `"3"`) can pass `model` / `reasoning_effort` arguments, which the Codex runtime parses and applies even though the published tool schema hides them. Known limitation: when a **native** parent spawns a child routed to a **non-native** provider, the Codex client may send the `NEW_TASK` payload only as backend-encrypted `encrypted_content`, so the routed child receives an empty task body ([#92](https://github.com/lidge-jun/opencodex/issues/92)). The model override still applies, but the task text can be lost — the v1 surface remains the reliable choice for heterogeneous-provider delegation. | ||
| On the v2 surface (`multi_agent_v2`), a spawned sub-agent inherits the parent model **by default**: `fork_turns` defaults to `all`, and full-history forks reject overrides. Since v2.7.2 opencodex injects guidance that teaches the model how to break inheritance — a `spawn_agent` call that sets `fork_turns` to `"none"` (or a partial fork such as `"3"`) can pass `model` / `reasoning_effort` arguments, which the Codex runtime parses and applies even though the published tool schema hides them. Known transport limitation: when a **native** parent spawns a child routed to a **non-native** provider, the Codex client may send the `NEW_TASK` payload only as backend-encrypted `encrypted_content` ([#92](https://github.com/lidge-jun/opencodex/issues/92)). opencodex does not forward that unreadable task to an external provider: a direct route fails with HTTP 400 and code `unreadable_encrypted_agent_task`, while a combo skips non-decrypting targets and selects a canonical native ChatGPT target when one is available. Use v1 for heterogeneous-provider delegation, select a native ChatGPT child, or resend the task as plaintext v2 `agent_message` content. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for other locale copies of this guide and whether they still contain the old wording.
fd 'sub-agent-surface.md' docs-site/src/content/docs
echo "---"
for f in $(fd 'sub-agent-surface.md' docs-site/src/content/docs); do
echo "== $f =="
rg -n 'unreadable_encrypted_agent_task|encrypted_content|task text can be lost' "$f" || echo "(no match — likely stale)"
doneRepository: lidge-jun/opencodex
Length of output: 6642
Sync the sub-agent-surface.md translations
docs-site/src/content/docs/ja/guides/sub-agent-surface.md:9, docs-site/src/content/docs/ko/guides/sub-agent-surface.md:9, and docs-site/src/content/docs/ru/guides/sub-agent-surface.md:9 still describe the old “task text can be lost” behavior. Update them to match the English/zh-cn text: direct non-native routes now fail with unreadable_encrypted_agent_task, and combos prefer a decrypt-capable native ChatGPT target.
🤖 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 `@docs-site/src/content/docs/guides/sub-agent-surface.md` at line 9,
Synchronize the translated sub-agent surface documentation in the Japanese,
Korean, and Russian guides with the current English/zh-cn behavior. Replace the
outdated “task text can be lost” description with the
`unreadable_encrypted_agent_task` failure for direct native-to-non-native
routes, and document that combos prefer a decrypt-capable native ChatGPT target.
Source: Path instructions
…il-fast contract (#336 improve)
Summary
This is the clean follow-up rebuild for #283 on current
dev(d9e06c8d). It closes the mixed-slot and failover bypasses documented after the original merge/revert, while leaving the responses facade unchanged.Current PR head:
87af85fb9df8bf8a4fbe0e1ae5fbb122bfe84fa9.What changed
agent_messagebefore history expansion, compatibility mutation, sanitization, parsing, collaboration injection, adapter construction, or fetch.unreadable_encrypted_agent_taskand never echo ciphertext.Verification
87af85fb: 10 passed, 0 failed, 0 pending (Linux, macOS, Windows, npm-global on all three, React Doctor, target enforcement, label, CodeRabbit).d0cf7b89abed89de44abb1584030f925d74bfb2fe4c952aeafc020364db118ab.c6defa10a4d6e6d43aff2c115369cce16e011a82.207ba99280383f4f706347869c7b092e480171f601dd41a24aaa4f459d775bee(unchanged from the independently reviewed733cec4dcode commit).Independent security review
All independent security reviews covered the unchanged six-file runtime diff. The only review finding on the PR requested documentation sync; it was addressed in
87af85fband the thread is resolved.No secrets, decryption keys, captured real task payloads, or ciphertext reflection are included.
Closes the follow-up identified in #283.
Summary by CodeRabbit
New Features
unreadable_encrypted_agent_taskerrors when no compatible destination is available.Bug Fixes
Documentation