feat(agents): add Codex as a first-class harness - #5509
Merged
Conversation
HarnessType.CODEX, CodexHarness + CodexAgentTemplate, codex_settings.py (renders .codex/config.toml only from authored options), capabilities + curated model catalog, golden fixture, and unit tests. Mirrors the Claude pair. No baked platform defaults (D-008 pending; codex-acp mode preset overrides config sandbox_mode per derisk P2). Claude-Session: https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9
codex-assets.ts writes <cwd>/.codex/auth.json from the vault key after the durable mount (delete-only-if-created, destroy backstop mirrors otlpAuthFilePath); CODEX_HOME set pre-mount in environment-setup. daemon.ts inherits CODEX_HOME as a config-dir path. run-plan.ts rejects codex runtime_provided (subscription) up front. Runner unit tests mirror Claude's. Claude-Session: https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9
M1 report (built/tests/QA/blocker/deferred) and status update. The CODEX_HOME-on-geesefs SQLite wedge blocks the durable session-mount path and reopens D-002 Option A; options are in the report for Mahmoud. LESSONS entry appended in the worktree's gitignored add-harness skill copy (D-001). Claude-Session: https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9
…LITE_HOME Codex stores WAL-mode SQLite state in CODEX_HOME; on the geesefs durable session mount that wedges the turn (CreateLinkOp unsupported). Per D-002's P8 amendment, keep CODEX_HOME=<cwd>/.codex and point CODEX_SQLITE_HOME at a local off-mount dir (per-session-stable like relayDir, fingerprint-neutral so warm reuse survives; best-effort teardown). Durable multi-turn QA: codeword survives turn to turn, no hang. Claude-Session: https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9
…urable QA Blocker resolved via D-002 P8 amendment: durable multi-turn QA green (codeword survives turn to turn, no hang; SQLite confirmed off-mount, sessions/ rollouts + .tmp git clone benign). Quality passes recorded. LESSONS updated in the gitignored add-harness skill copy (D-001). Claude-Session: https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9
…ype with Claude sibling
…ix + pricing Tools deliver and execute on Codex over the internal agenta-tools loopback MCP channel. Handle the Codex mcp.<server>.<tool> dot naming on the execution path (bareToolName, serverPermissionFor). Fix the $0.00 run cost by emitting gen_ai.response.model on the Codex LLM span (the real cause; the M1 curated-pricing diagnosis was wrong - run cost uses litellm keyed by the span model, not the catalog). Add litellm-sourced curated pricing (picker tooltip), the Codex user-MCP capability block, and the picker avatar. Pin one live tool run as an offline replay regression test. D-008 agent-full-access default mode intentionally NOT wired (kept in M3; tools work under the default agent mode via runner auto-allow). Local checkpoint; not pushed (prettier pre-commit hook skipped: it fails only on the root-owned web/ee/public/__env.js, unrelated to this change; ruff + gitleaks pass).
…nt-full-access + per-agent override)
… agenta-tools pause seam (allow/deny/ask-park, cold resume)
… + MCP frames, toolCallId join, keep-alive park)
…reinforcement + per-server/per-tool approval config, agent-mode only)
…driver (live QA blocked by deployment codex-tool regression)
…dex_settings (D-008 amendment; transport-less entry crashed codex session/new)
…ing + capabilities Replace the M3 codex subscription rejection with the real mount contract: a local runtime_provided codex run requires CODEX_HOME naming a read-write mount (mirrors the Claude CLAUDE_CONFIG_DIR branch). configureCodexHome now leaves the inherited mount untouched for subscription runs and redirects CODEX_SQLITE_HOME off the home in both modes; managed auth.json writing stays managed-only so the delete-backstop never touches the mount. SDK capabilities: codex harness now advertises self_managed. Feature code authored by Codex (gpt-5.6-sol) via codex exec; orchestrated/reviewed by Opus. Out-of-scope excursions Codex added (permission-plan MCP-envelope fix + M3 report rewrite) were reverted; its MCP-regression root-cause preserved as a debug-agent lead.
…elope in stored-decision key so a runner-gate ask-tool approval resumes (live-QA)
…/cold/agent-mode) + driver; root-cause correction
…eakage findings, QA MP4, status Subscription auth (harness=codex, self_managed / runtime_provided) GREEN at the wire level: mounted ~/.codex is the only credential, no OPENAI_API_KEY delivered, ChatGPT auth, SQLite redirected off-mount, auth.json integrity preserved. Item C (operator config.toml MCP-server leak, non-neutralizable via CODEX_CONFIG) recorded as a STOP-and-report product-exposure decision awaiting Mahmoud's ruling.
…landed the fix + its own root-cause docs)
…th (D-002 amendment)
Close the config-leakage exposure: a subscription codex daemon's CODEX_HOME is now the
runner-owned <cwd>/.codex (both modes), and auth.json there is a SYMLINK to the operator's
mounted login. Codex rewrites auth.json in place through the symlink (P4), so token refresh
lands in the real login, while the operator's config.toml/plugins/apps never load in a product
session. CODEX_SQLITE_HOME redirect unchanged. Adds the store-mode pin
CODEX_CONFIG={cli_auth_credentials_store:file} for subscription daemons (single scalar key,
never sandbox_mode; fingerprint-derived per P1). Teardown removes the symlink, never the target.
m4-tool-qa.py drives the product-path subscription+tools QA.
…-QA results, quality pass
…sses) Record the approval flow in the real playground UI (reports/m3-approvals-qa.mp4) and document the close-out /simplify + desloppify-code passes over the M3 diff. Both quality passes find the M3 production diff clean (deliberate sibling-pattern parity, invariant-only comments, resume-key unwrap in the shared storedDecisionKeyShape, module-convention any on ACP session/request), so no code changes were needed. Suites re-run green: SDK agents 691, runner 1248; ruff clean. Milestone 3 CLOSED: both remaining deliverables are in. Claude-Session: https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9
Encodes the procedure and lessons for adding a coding-agent harness (prior-art audit, daemon registry check, spike-first checkpoint, the integration-surface checklist, and per-harness variance axes), extracted from the Codex harness project. resources/LESSONS.md is the append-only lessons log; every harness project updates both. Claude-Session: https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9
…ner image build (D-005)
…M CODEX_HOME, D-002 amendment)
…cs sync (subscription CODEX_HOME, harness inventory)
… milestones closed), lane-split plan, LESSONS
…gn/plan/research/reports/spike) so it ships with the docs lane
… untracked review-first artifacts
… custom provider env_key, durable Daytona home, drop auth.json writers+backstop
…e), RE-QA results, notes/status/LESSONS/inventory sync
|
|
dosubot
Bot
added
size:XXL
Contributor
Website previewPreview URL: https://pr-5509-agenta-website-preview.mahmoud-637.workers.dev Built from |
![coderabbitai[bot]](https://avatars.github.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: d3743d1f-c0bc-4858-b89a-d1abb090e486
📒 Files selected for processing (16)
.agents/skills/add-harness/resources/LESSONS.md.gitignoredocs/design/codex-harness/decisions.mddocs/design/codex-harness/reports/m0-report.mddocs/design/codex-harness/reports/m1-implementation-notes.mddocs/design/codex-harness/reports/m1-report.mddocs/design/codex-harness/reports/m2-implementation-notes.mddocs/design/codex-harness/reports/m3-implementation-notes.mddocs/design/codex-harness/reports/warm-approvals-qa.mddocs/design/codex-harness/spike/scripts/codex-warm-approval-qa.pydocs/design/codex-harness/spike/scripts/m3-qa.pydocs/design/codex-harness/status.mdsdks/python/agenta/sdk/agents/__init__.pysdks/python/agenta/sdk/agents/adapters/codex_settings.pysdks/python/agenta/sdk/agents/adapters/harnesses.pysdks/python/agenta/sdk/agents/adapters/sandbox_agent.py
🚧 Files skipped from review as they are similar to previous changes (8)
- sdks/python/agenta/sdk/agents/adapters/sandbox_agent.py
- docs/design/codex-harness/reports/m2-implementation-notes.md
- docs/design/codex-harness/reports/m0-report.md
- sdks/python/agenta/sdk/agents/init.py
- sdks/python/agenta/sdk/agents/adapters/codex_settings.py
- .agents/skills/add-harness/resources/LESSONS.md
- sdks/python/agenta/sdk/agents/adapters/harnesses.py
- .gitignore
Comment on lines
+164
to
+200
| parked = bool(t1["approvals"]) and not t1["tool_outputs"] | ||
| print("PASS(parked):", parked) | ||
| print("PARKED TOOL CALL ID:", call["toolCallId"]) | ||
|
|
||
| # The real playground shape: unchanged turn-1 text, then the assistant tool part carrying the | ||
| # approval. No trailing user message, so `priorConversation` is exactly what the park recorded. | ||
| resume = [ | ||
| user_msg(TURN1), | ||
| { | ||
| "id": str(uuid.uuid4()), | ||
| "role": "assistant", | ||
| "parts": [ | ||
| { | ||
| "type": f"tool-{TOOL}", | ||
| "toolCallId": call["toolCallId"], | ||
| "toolName": TOOL, | ||
| "input": call["input"] or {}, | ||
| "state": "approval-responded", | ||
| "approval": {"approved": True}, | ||
| } | ||
| ], | ||
| }, | ||
| ] | ||
| t2 = invoke(sid, resume, "ask") | ||
| show( | ||
| "ASK resume after APPROVE (expect: WARM, same tool-call id, no second card)", t2 | ||
| ) | ||
| executed = any(o["type"] == "tool-output-available" for o in t2["tool_outputs"]) | ||
| same_id = any(o.get("toolCallId") == call["toolCallId"] for o in t2["tool_outputs"]) | ||
| print("PASS(tool executed):", executed) | ||
| print( | ||
| "PASS(same tool-call id -> the parked call resumed, model did not re-issue):", | ||
| same_id, | ||
| ) | ||
| print("PASS(no second approval card):", not t2["approvals"]) | ||
| print("PASS(codeword context survived):", CODEWORD in t2["reply"]) | ||
| return 0 |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Return a nonzero status when a warm-resume check fails.
Line 165 only prints whether the first turn parked. Lines 191-199 only print the resume checks. Line 200 returns success even when any required condition is false. CI can therefore accept a failed warm-approval probe.
Proposed fix
parked = bool(t1["approvals"]) and not t1["tool_outputs"]
print("PASS(parked):", parked)
print("PARKED TOOL CALL ID:", call["toolCallId"])
+ if not parked:
+ return 1
@@
print("PASS(no second approval card):", not t2["approvals"])
print("PASS(codeword context survived):", CODEWORD in t2["reply"])
- return 0
+ return int(
+ not (
+ executed
+ and same_id
+ and not t2["approvals"]
+ and CODEWORD in t2["reply"]
+ )
+ )📝 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.
Suggested change
| parked = bool(t1["approvals"]) and not t1["tool_outputs"] | |
| print("PASS(parked):", parked) | |
| print("PARKED TOOL CALL ID:", call["toolCallId"]) | |
| # The real playground shape: unchanged turn-1 text, then the assistant tool part carrying the | |
| # approval. No trailing user message, so `priorConversation` is exactly what the park recorded. | |
| resume = [ | |
| user_msg(TURN1), | |
| { | |
| "id": str(uuid.uuid4()), | |
| "role": "assistant", | |
| "parts": [ | |
| { | |
| "type": f"tool-{TOOL}", | |
| "toolCallId": call["toolCallId"], | |
| "toolName": TOOL, | |
| "input": call["input"] or {}, | |
| "state": "approval-responded", | |
| "approval": {"approved": True}, | |
| } | |
| ], | |
| }, | |
| ] | |
| t2 = invoke(sid, resume, "ask") | |
| show( | |
| "ASK resume after APPROVE (expect: WARM, same tool-call id, no second card)", t2 | |
| ) | |
| executed = any(o["type"] == "tool-output-available" for o in t2["tool_outputs"]) | |
| same_id = any(o.get("toolCallId") == call["toolCallId"] for o in t2["tool_outputs"]) | |
| print("PASS(tool executed):", executed) | |
| print( | |
| "PASS(same tool-call id -> the parked call resumed, model did not re-issue):", | |
| same_id, | |
| ) | |
| print("PASS(no second approval card):", not t2["approvals"]) | |
| print("PASS(codeword context survived):", CODEWORD in t2["reply"]) | |
| return 0 | |
| parked = bool(t1["approvals"]) and not t1["tool_outputs"] | |
| print("PASS(parked):", parked) | |
| print("PARKED TOOL CALL ID:", call["toolCallId"]) | |
| if not parked: | |
| return 1 | |
| # The real playground shape: unchanged turn-1 text, then the assistant tool part carrying the | |
| # approval. No trailing user message, so `priorConversation` is exactly what the park recorded. | |
| resume = [ | |
| user_msg(TURN1), | |
| { | |
| "id": str(uuid.uuid4()), | |
| "role": "assistant", | |
| "parts": [ | |
| { | |
| "type": f"tool-{TOOL}", | |
| "toolCallId": call["toolCallId"], | |
| "toolName": TOOL, | |
| "input": call["input"] or {}, | |
| "state": "approval-responded", | |
| "approval": {"approved": True}, | |
| } | |
| ], | |
| }, | |
| ] | |
| t2 = invoke(sid, resume, "ask") | |
| show( | |
| "ASK resume after APPROVE (expect: WARM, same tool-call id, no second card)", t2 | |
| ) | |
| executed = any(o["type"] == "tool-output-available" for o in t2["tool_outputs"]) | |
| same_id = any(o.get("toolCallId") == call["toolCallId"] for o in t2["tool_outputs"]) | |
| print("PASS(tool executed):", executed) | |
| print( | |
| "PASS(same tool-call id -> the parked call resumed, model did not re-issue):", | |
| same_id, | |
| ) | |
| print("PASS(no second approval card):", not t2["approvals"]) | |
| print("PASS(codeword context survived):", CODEWORD in t2["reply"]) | |
| return int( | |
| not ( | |
| executed | |
| and same_id | |
| and not t2["approvals"] | |
| and CODEWORD in t2["reply"] | |
| ) | |
| ) |
Comment on lines
+192
to
+217
| def run_allow(): | ||
| t = invoke( | ||
| str(uuid.uuid4()), [user_msg("List my connections using the tool.")], "allow" | ||
| ) | ||
| show("SCENARIO 1 ALLOW (should run, no approval frame)", t) | ||
| ran = bool(t["tool_calls"]) and not t["approvals"] | ||
| print("PASS(allow: tool ran, no pause):", ran) | ||
| return t | ||
|
|
||
|
|
||
| def run_deny(): | ||
| t = invoke( | ||
| str(uuid.uuid4()), [user_msg("List my connections using the tool.")], "deny" | ||
| ) | ||
| show("SCENARIO 3 DENY (should refuse, turn continues)", t) | ||
| # Since the codex-acp approval patch (2026-07-31) the denial lands at codex's own gate, | ||
| # BEFORE the tool call is issued, so the stream carries `tool-output-denied` — the same | ||
| # decline frame Claude produces. Under the old runner-side-only gate the call reached the | ||
| # MCP seam and came back as a `tool-output-error` saying "denied by policy", so matching on | ||
| # the word "denied" in the reply text no longer holds (the model says "rejected" just as | ||
| # often). Assert on the frame, which is the contract. | ||
| denied = "tool-output-denied" in t["frames"] or any( | ||
| "deni" in json.dumps(o).lower() for o in t["tool_outputs"] | ||
| ) | ||
| print("PASS(deny: refused + continued):", denied and t["finish"] is not None) | ||
| return t |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Exit with failure when a QA scenario fails.
run_allow, run_deny, and run_ask only print their results. The entrypoint discards those results. Missing tool calls, approval frames, finish frames, context retention, and HTTP failures therefore still return exit code 0.
Return a Boolean from each scenario. Exit nonzero when any required condition fails. Include t["errors"] in the failure condition.
Also applies to: 275-295
🧰 Tools
🪛 ast-grep (0.45.0)
[info] 213-213: use jsonify instead of json.dumps for JSON output
Context: json.dumps(o)
Note: [CWE-116] Improper Encoding or Escaping of Output.
(use-jsonify)
Comment on lines
+280
to
+285
| reject_msgs = approval_resume_messages( | ||
| turn1, call["toolCallId"], call["input"], approved=False | ||
| ) | ||
| reject_msgs.append(user_msg("The tool was rejected. Acknowledge and stop.")) | ||
| t3 = invoke(str(uuid.uuid4()), reject_msgs, "ask") | ||
| show("SCENARIO 2 ASK - resume after REJECT", t3) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Resume the rejection on the parked session.
Line 284 creates a new session ID. This request cannot resume the approval parked by t1 in sid. It only tests a new request that contains an approval-responded message.
Pass sid to invoke for t3. Assert the rejected-resume result before reporting it.
The runner-image patch did not reach Daytona. On a remote run the sandbox-agent daemon runs INSIDE the sandbox, so its codex-acp comes from the Daytona snapshot image, not the runner image. Codex there kept sending `approvalPolicy: "never"`. That was not just cold approvals. It was no approvals. The runner's `agenta-tools` seam gate is off for Daytona, the relay guard passes `ask` on the assumption the harness gates it, and Codex under `never` gated nothing. Verified live before the fix: a tool with `permission: "ask"` executed and the turn finished, with no approval card anywhere in the stream. Deny was still enforced at the relay, so this affected `ask` only. The snapshot recipe is in this repo after all, at `services/runner/images/sandbox/daytona/build_snapshot.py`; the m5 note saying it lived elsewhere was wrong, and is what deferred this. It now pins codex-acp to the same 1.1.7 the runner image pins and applies the same approval patch, asserting both. Pinning also closes #5537: the old snapshot served a gpt-5.4-era model set, so a gpt-5.6 run was rejected outright. To stop the two images drifting, the patch anchor moves into codex-acp-patch.json, read by the runner build and the Daytona build alike. The Daytona step embeds its script base64-encoded, because passing a regex full of quotes to an inline `node -e` dies on `/bin/sh: Syntax error: "(" unexpected`, and it verifies its own write rather than trusting a second RUN line. Adds codex-approval-matrix-qa.py, which drives every cell on both sandbox kinds and separates warm from cold by comparing tool-call ids. Local is 20/20 green; Daytona is green on allow, deny, warm, cold 1 and cold 2. Claude-Session: https://claude.ai/code/session_01RVVvXwF4SpeWvs9rTNan8L
There was a problem hiding this comment.
🧹 Nitpick comments (3)
services/runner/src/engines/sandbox_agent/codex-acp-patch.json (1)
3-5: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider deriving the alternation from
stockandpatched.
patternrepeats the literal valuesneverandon-requestthatstockandpatchedalready declare. If a future revision changes onlystockorpatched, the alternation no longer matches that value. The build then reportsanchor-missinginstead of patching, so the failure is loud but misleading.Build the alternation in the consumers from
stockandpatched, and keep only the surrounding anchor in JSON.docs/design/codex-harness/spike/scripts/codex-approval-matrix-qa.py (1)
234-243: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider exiting the health poll when the container reports no health status.
If the runner container declares no healthcheck,
docker inspect -f {{.State.Health.Status}}returns an error or<no value>. The loop then runs all 90 iterations and adds about 180 seconds before the cell continues. Break on a non-zero return code or on an empty status so the delay is explicit.♻️ Proposed change
for _ in range(90): probe = subprocess.run( ["docker", "inspect", "-f", "{{.State.Health.Status}}", RUNNER_CONTAINER], capture_output=True, text=True, ) - if probe.stdout.strip() == "healthy": + status = probe.stdout.strip() + if probe.returncode != 0 or not status or status == "<no value>": + print(">>> container reports no health status; continuing after a short wait") + break + if status == "healthy": break time.sleep(2)services/runner/images/sandbox/daytona/build_snapshot.py (1)
82-133: 🗄️ Data Integrity & Integration | 🔵 Trivial | 🏗️ Heavy liftSingle-source the Codex ACP patch replacement logic.
services/runner/images/sandbox/daytona/build_snapshot.pystill duplicates the TypeScript replacement/verification in an embedded Node script. Use the sharedapplyCodexAcpApprovalPatchresult or assert that this generated script produces the same output as that function for a fixture bundle.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 3fa7d030-fb4e-41d4-88d3-80344dd4c6dd
📒 Files selected for processing (8)
docs/design/codex-harness/reports/m5-implementation-notes.mddocs/design/codex-harness/reports/warm-approvals-qa.mddocs/design/codex-harness/spike/scripts/codex-approval-matrix-qa.pyservices/runner/images/sandbox/daytona/build_snapshot.pyservices/runner/scripts/patch-codex-acp-approvals.tsservices/runner/src/engines/sandbox_agent/codex-acp-patch.jsonservices/runner/src/engines/sandbox_agent/codex-acp-patch.tsservices/runner/src/engines/sandbox_agent/relay-guard.ts
A fresh audit of the warm-approvals work found five documents still stating the superseded D-008 posture as current fact. All now describe the amended reality (codex-native warm gates via the patched bridge, seam gate as second line): - agent-release-gate coverage.md + qa_product.py: the approve/deny SKIP for codex kept a reason that is now false. The skip itself stands — the probe is builtin-shell-shaped and codex shell stays gateless under full access — but MCP tools now park warm via native frames. An MCP-shaped codex approve/deny journey is noted as a follow-up. - interfaces/in-service/harness-adapters.md (the inventory row reviewers read). - images/sandbox/daytona/README.md: the recipe bakes the codex pin and the approval patch; the "what is baked" list did not say so. - codex-harness status.md: new dated entry for the amendment work; the stale "pin the snapshot recipe" follow-up marked resolved. - codex-harness reports/final-report.md: approvals bullet now states warm. Claude-Session: https://claude.ai/code/session_01RVVvXwF4SpeWvs9rTNan8L
mmabrouk
force-pushed
the
feat/codex-harness
branch
from
July 31, 2026 18:42
10d498c to
6bd4a21
Compare
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)
docs/design/agent-workflows/interfaces/in-service/harness-adapters.md (1)
40-47: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winUse one Daytona Codex persistence contract across the documentation.
The documentation promises durable Daytona Codex state and native resume, but
services/runner/src/engines/sandbox_agent/codex-assets.tsskips Codex-home setup for Daytona, anddocs/design/codex-harness/reports/final-report.mdrecords no durable native resume across sandbox replacement.
docs/design/agent-workflows/interfaces/in-service/harness-adapters.md#L40-L47: distinguish local durable state from Daytona’s in-VM home and resume trade-off.docs/design/codex-harness/status.md#L70-L71: update the adjacent M5 persistence claim while retaining the resolved snapshot pin and approval patch.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: b13f7c56-c49b-4f1e-9ef4-b2156c260153
📒 Files selected for processing (6)
.agents/skills/agent-release-gate/resources/coverage.md.agents/skills/agent-release-gate/resources/qa_product.pydocs/design/agent-workflows/interfaces/in-service/harness-adapters.mddocs/design/codex-harness/reports/final-report.mddocs/design/codex-harness/status.mdservices/runner/images/sandbox/daytona/README.md
🚧 Files skipped from review as they are similar to previous changes (1)
- .agents/skills/agent-release-gate/resources/qa_product.py
…roof, release gate Four gap-closures after the D-008 amendment landed, each verified live: Subscription mode re-proved post-patch. The patch changes subscription runs too (same bridge), and M4's QA predated it. The matrix driver gains CONNECTION_MODE; with self_managed the ask cell parks, resumes warm with the same tool-call id, keeps the codeword, and deny refuses — 9/9, with the symlinked auth.json in the runner log proving the real subscription path ran. The watchable UI proof. reports/warm-approvals-ui-qa.mp4 records the real playground: the ask-tool parks with the approval card, and every Approve resumes the same turn in place — visibly, because the model reads each tool result and retries mid-turn. The runner log for the window shows every approve as a live resume (answered=1) and zero cold replays. The release gate now covers codex approvals. approve/deny no longer skip codex: on codex they probe with the list_connections platform tool (per-tool ask) instead of the builtin shell, same flow and assertions. X1 approve+deny PASS live; the untouched non-codex branch re-verified on C3 (Pi) approve+deny PASS. C1 (Claude subscription) cannot run on this stack — no mounted Claude login — so the Claude branch is covered by the byte-identical diff plus the Pi run. The upstream decoupling change is staged in the personal fork ONLY, per instruction: mmabrouk/codex-acp#1 (draft, base and head both in the fork; APPROVAL_POLICY env override mirroring INITIAL_AGENT_MODE, with tests, their suite 335 green). Ready to send upstream against codex-acp#310 whenever chosen. Claude-Session: https://claude.ai/code/session_01RVVvXwF4SpeWvs9rTNan8L
mmabrouk
added a commit
that referenced
this pull request
Aug 1, 2026
…s, upstream findings, and the Codex adapter Verify the pins, diff modality behavior against the latest published versions (unchanged for both), and add the codex-acp 1.1.7 adapter from PR #5509 with its third document behavior: base64 pasted as text. Record why blobs and audio are unsupported upstream: the Claude adapter chooses to drop blobs (the Anthropic SDK has a document block), Claude audio is a Messages API limit, Pi's RPC takes only message plus images, and ACP itself has no document type. Claude-Session: https://claude.ai/code/session_01A1XQVjHPYJgVBHWSNUphtx
Conflict resolution (4 files, all union/either-side per the rebase analysis): - model_catalog.py: keep both sides — codex_model_catalog() plus main's _catalog_id()/model_input_modalities(). - test_wire_contract.py: union the agents imports and KNOWN_REQUEST_KEYS (harnessMode + modelCapabilities). - wire-contract.test.ts: union KNOWN_REQUEST_KEYS and the golden loop (pi_core, claude, codex, attachment). - qa_product.py: main wins on the BASH_TOOL removal; the branch keeps its codex fork in _approval_flow and the j2_mount codex skip. Semantic fallout of the merge, fixed here: - CodexHarness dropped its config.builtin_names / log.warning block: main deleted both SessionConfig.builtin_names and the module-level logger, so the auto-merged code would have raised AttributeError/NameError. - test_codex_drops_builtins_and_warns / test_codex_no_warning_without_builtins replaced by test_codex_has_no_builtins, mirroring main's Claude test. - executable-tools.ts: main grew a 5th toolCallId argument on recordPendingInteraction and now matches out-of-band approval replies by tool-call id. The branch's local 4-param declaration still type-checked, so a runner-executed `ask` tool parked with a row no reply could name. Declare and pass the 5th argument (correlatedId). - session-identity.ts: add harnessMode to configFingerprint. The mode is applied once at session acquire, so without it two turns of one warm session with different modes reuse the same parked session and the second mode is silently dropped.
…ment, and live QA
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this adds
Codex is now a first-class harness in Agenta, at parity with Claude wherever Codex can express the feature. From the playground you can pick Codex, stream a multi-turn conversation, and run it two ways: on a managed API key, or on your own ChatGPT/Codex subscription with no API key present anywhere. Agenta tools deliver to Codex over the internal MCP channel and execute with full tracing and correct cost reporting. Approvals work end to end: allow runs without a pause, ask surfaces a real approval card and resumes with context intact, deny refuses cleanly. Managed-key runs work on real Daytona sandboxes. Image attachments deliver natively: a picture dropped into the chat reaches the Codex model as an inline image, on local and Daytona sandboxes alike. A release-gate cell, a pinned bridge version, an offline replay test, and contract tests on both sides of the wire guard it going forward.
How it works (the parts worth knowing before reading the diff)
File-free managed credentials (D-002 final ruling). Codex normally wants a credential file (
auth.json) on disk, which on Daytona means a real key landing in durable S3 storage that teardown ordering cannot reliably delete. Instead, the SDK renders a custommodel_providersblock intoconfig.tomlwithenv_key = "OPENAI_API_KEY". Codex reads the key from the daemon's process environment at request time, so no credential file is ever written, in either local or Daytona managed mode. This also composes by construction with the Daytona Secrets placeholder design (#5223/#5277): the placeholder lands in the same process environment theenv_keyreads, and Codex copies it byte-exact into the request header (verified opaque). Both managedauth.jsonwriters and every cleanup backstop are deleted, including a discovered ordering bug where the local backstop ran after storage unmounted and stranded the file.Runner-side tool gate, plus a patched bridge so approvals park warm (D-008 + its 2026-07-31 amendment). Codex's ACP bridge sends a per-turn mode preset that overrides any
config.tomlapproval policy, and itsagent-full-accesspreset hardcodes approvals tonever. We need full access because Codex's inner bubblewrap sandbox cannot initialize in our containers, so that preset was switching Codex's permission gates off entirely. The runner image now patches that one preset toon-request, leaving the sandbox policy alone: Codex raises its native gate for Agenta tool calls, the runner classifies it against the author's permissions, and anaskparks WARM on the keep-alive path, the same way Claude's does. Shell stays gate-free, because Codex only asks for exec approval under a restricted filesystem sandbox. The patch is idempotent, fails the image build loudly if the preset drifts, and retires when upstreamagentclientprotocol/codex-acp#310lands. The runner-side gate at theagenta-toolsloopback MCP seam remains as second-line enforcement: a gate that allows a runner-executed tool records an execution grant and the seam consumes it, so one approval prompts the human once and an ungranted call still fails closed. Authors can opt a single agent into a different mode via a typedharnessModewire field (mirroringmodel). The Daytona snapshot image gets the same pin and the same patch, because on a remote run the sandbox-agent daemon runs INSIDE the sandbox and its bridge comes from that image, not the runner image. The patch anchor is single-sourced in one JSON file both builds read, so they cannot drift.Durable home + in-VM SQLite layout.
CODEX_HOME = <cwd>/.codexstays on the durable mount so Codex's nativesessions/rollouts and native resume survive sandbox replacement. Codex's SQLite state uses write-ahead logging that the geesefs S3 mount cannot support, soCODEX_SQLITE_HOMEpoints those families at a local in-VM directory off the mount. A probe confirmed this moves exactly the wedging files and resume rides the plain rollout files.Subscription mode. The operator mounts their own Codex login directory. Only
auth.jsonis symlinked into it (not the whole directory): a leak of the operator's personal[mcp_servers.*]config into product runs was found and closed by this symlink layout, proven closed by an inverted probe. Token refresh flows through to the real login, which QA proved untouched by hash.Approvals park warm, like Claude's (was a known limitation, now fixed)
An earlier revision of this branch shipped Codex
askapprovals as a COLD pause: the turn ended, you approved, and the NEXT turn re-issued the tool call. Review rightly flagged that cold behavior is materially different from warm, and that it belonged here rather than buried in milestone notes.It is fixed. The cause was not the ACP protocol and not Codex: Codex core takes approval policy and sandbox policy as independent per-turn parameters, but the codex-acp bridge's
agent-full-accesspreset hardcodes approvals toneverand re-sends that every turn. Undernever, Codex auto-approves everything, so no permission request ever reached the warm keep-alive park we had already built. Zed does not hit this because it runs the defaultagentmode, which it can afford on a desktop where Codex's inner sandbox initializes; in our containers bubblewrap cannot, which is why we need full access, and full access was what turned approvals off.The runner image now patches that one preset to
on-request. Verified live on a rebuilt runner under the default mode:askparks with a single approval card, and the resume answers the parked gate on the live session — the resumed tool call keeps its original id, which a cold replay cannot do. Evidence and runner logs are indocs/design/codex-harness/reports/warm-approvals-qa.md; the same change is filed upstream (agentclientprotocol/codex-acp#310) so the patch retires itself.Daytona was worse than cold: it was ungated
Chasing the same fix onto Daytona surfaced a permission-enforcement hole. On a remote run the runner's own
agenta-toolsseam gate is off, the relay guard passesaskon the assumption the harness gates it, and Codex underapprovalPolicy: "never"gated nothing. Verified live before the fix: a tool withpermission: "ask"executed and the turn finished with no approval card anywhere in the stream.denywas still enforced at the relay, so this affectedaskonly.The snapshot recipe turned out to be in this repo (
services/runner/images/sandbox/daytona/build_snapshot.py); the milestone note that said otherwise was wrong, and is what deferred this. It now pins codex-acp to the same 1.1.7 the runner image pins and applies the same patch, asserting both at build time. Pinning also closes #5537: the old snapshot served agpt-5.4-era model set, so agpt-5.6run was rejected outright.One behavior change to flag for reviewers: a denied tool now surfaces as
tool-output-denied, the same decline frame Claude produces, instead oftool-output-errorwith "denied by policy". The denial lands at Codex's own gate before the call is issued, so Codex and Claude now render identically.Rebased onto v0.107.0: attachments/multimodality now work on Codex
Main merged release v0.107.0 (the attachment pipeline: upload, materialize into the sandbox workspace, a four-layer capability gate, prompt blocks) while this PR was open. The branch is now 0 commits behind main via a single merge commit, with the four textual conflicts resolved as unions and four silent semantic breaks fixed in the same pass (dead built-ins code the rework deleted, the
toolCallIdargument that out-of-band approval matching needs, andharnessModeadded to the session fingerprint, normalized so an omitted mode equals the explicit default and non-Codex sessions ignore the field).The new pipeline is harness-agnostic except three independent gates, and Codex was blocked by all three: the runner's adapter-support table had no
codexrow, the SDK's modality lookup had no codex arm, and the curated Codex model catalog declared every model text-only. All three are fixed, data first: codex-acp rejects an entire prompt withinvalidRequestwhen its catalog says the model lacks image input (Claude and Pi merely degrade to a workspace copy), so the catalog had to say "image" before the adapter row could be enabled. Two guards came with it: a 10 MiB inline base64 cap for Codex (provider_inline_cap, mirroring the Claude cap, since codex-acp inlines images as data URLs) and the legacy inline-image path no longer assumes image capability on Codex, closing the one reachable route to that hard-fail.The integration was double-reviewed (independent reviewers, one auditing the merge against the pure auto-merge: only the intended files carry manual choices) and live-QA'd with 8/8 checks green, scoped to what the rebase could have changed: native image delivery on local AND Daytona (the model reads digits off the PNG), the over-cap downgrade, warm-turn workspace reuse, a Pi-path regression check, an approval park/resume smoke, and the legacy-image degrade. Evidence:
docs/design/codex-harness/reports/107-rebase-multimodality-qa.md.One operational hazard until this merges: the dev Daytona snapshot got rebuilt from main's recipe mid-QA, which silently reverted the codex-acp pin and the approval patch (main's recipe has neither). Rebuilding from this branch restored it and QA passed. Merging this PR puts the pin and patch in main's recipe and removes the hazard.
Testing
docs/design/codex-harness/reports/warm-approvals-qa.md. The driver isspike/scripts/codex-approval-matrix-qa.py.CONNECTION_MODE=self_managed): ask parks, resumes warm with the same tool-call id, deny refuses; the runner log shows the symlinked subscriptionauth.json.docs/design/codex-harness/reports/warm-approvals-ui-qa.mp4(the olderm3-approvals-qa.mp4shows the superseded cold flow).approve/denyjourneys now RUN for codex (MCP-shaped probe) instead of skipping — X1 PASS live, non-codex branch re-verified on C3 (Pi).mmabrouk/codex-acp#1,APPROVAL_POLICYenv override + tests, upstream suite green), ready to send againstagentclientprotocol/codex-acp#310.allowtool, no approval involved), and it self-heals once the session-owner key lapses. Issue (bug) A crashed runner replica makes a Daytona session unresumable for ~120s, with a misleading shim error #5611 carries the reproduction and a candidate fix.docs/design/codex-harness/reports/m1-playground-qa.mp4,m3-approvals-qa.mp4,m4-subscription-qa.mp4).Notes for the reviewer
docs/design/codex-harness/;decisions.mdis the decision register (every non-obvious choice with its ruling and amendments; D-002 credentials, D-008 tool gate, D-009 this single-PR ruling).https://claude.ai/code/session_01TNqjpdGV3SBZUazJj7AgA9