feat(runtime): add the codex agent runtime (ADR 0099, ADR 0100) - #6926
Merged
Conversation
5 tasks
PR Summary by QodoImplement secure Codex agent runtime
AI Description
Diagram
High-Level Assessment
Files changed (22)
|
|
🤖 Review · Commit: |
Site previewPreview: https://e1a3d757-site.fullsend-ai.workers.dev Commit: |
Code Review by Qodo
1.
|
![qodo-code-review[bot]](https://avatars.githubusercontent.com/in/484649?s=60&v=4){kind=small}
waynesun09
force-pushed
the
codex-runtime-core
branch
from
September 2, 2026 19:01
d54cb89 to
ff0eab8
Compare
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
Codecov Report❌ Patch coverage is 📢 Thoughts on this report? Let us know! |
waynesun09
changed the title
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
waynesun09
force-pushed
the
codex-runtime-core
branch
from
September 2, 2026 19:17
08c2f8b to
7c0b764
Compare
|
🤖 Review · Commit: |
|
🤖 Review · Commit: |
Adds what a real `codex exec --json` run against api.openai.com inside the
pinned sandbox image proved, and folds the findings into ADR 0099's
consequences and the runtime matrix.
The three questions the source could not settle are answered: hooks.json loads
with no `[hooks]` table (no empty table needed); a hook block reaches the model
as a decline, not a command failure, in both phases; and a repo's own
`.agents/skills` are discovered even with the project untrusted, which is
Claude Code parity and is covered by the existing SKILL.md context scans.
Two findings change what a reader should expect:
* with a custom provider codex also issues `GET /v1/models` at startup, which
the fullsend-openai egress profile denies — non-fatal and logged, but
"only POST /v1/responses is attempted" is not true of codex the way it is
of pi;
* the tee'd output.jsonl keeps each command's raw aggregated_output even when
a PostToolUse hook blocked the result, so the hooks protect the model's
context and not the run artifact.
Refs #6920
Assisted-by: Claude (implementation)
Signed-off-by: Wayne Sun <gsun@redhat.com>
…earances The guard asserted "every handler still invokes the adapter" but only checked that the adapter appeared somewhere in the file, so an agent that replaced one handler's command with `true` and left another pointing at the adapter passed it. It now compares the number of `"command":` keys with the number of adapter references, which a replaced or added handler breaks. Deleting handlers outright still passes — that narrows the wiring rather than redirecting it, and is the same residue Claude Code has with its own hooks.json; both the comment and the contributing page now say so. Counting is by occurrence (`grep -o | wc -l`), not by matching line: the runner writes one handler per line but an agent rewriting the file is under no such obligation, and a compacted hooks.json would make a line count collapse to 1 = 1 and pass. The pattern is the key with its colon, since a bare `"command"` also matches the value in `"type": "command"`. Also corrects a claim the local smoke contradicted: codex's tracing is not silent without RUST_LOG. It logs at error level by default — the denied GET /v1/models and every hook block — so a codex run writes to the runner console where a pi run does not. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
The sandbox hook chain protects the model's context: a PostToolUse block withholds the tool result from the model, and a sanitizer's rewrite is dropped because codex accepts no output rewrite for built-in tools. Neither touches what codex writes down. The `exec --json` stream keeps each command's raw aggregated_output and the rollout session file keeps the same output, and both are uploaded as run artifacts — so a credential the chain masked for the model still landed in output.jsonl and transcripts/. Claude Code does not have this gap: its stream carries the post-hook result. Both are now filtered through the same security.SecretRedactor the other runtimes' progress parsers use — the shared pattern list, not a second copy. The stream is redacted on the tee branch, so the parser still sees exactly what codex emitted and only the file on disk is masked; the rollout is rewritten in place after download, preserving its mode. A line that is not JSON is redacted as text rather than passed through, since that is where a truncated or hostile payload would sit, and numbers decode through json.Number so an exit_code or a token count cannot come back in exponent form. This is redaction, not the hook chain: a canary is masked in the artifact if it looks credential-shaped, not withheld, and nothing is condensed or normalized. The matrix cells and ADR 0099 say so. A compressed rollout (.jsonl.zst) is reported as unredacted rather than silently shipped — decompressing one would mean shipping a zstd decoder for a file the current iteration did not write. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
The config guard checked config.toml with grep line patterns — that base_url
was the pinned one, that no `[projects` header appeared. TOML's dotted-key
form sets the same value with no bracket header:
projects."/sandbox/workspace/repo".trust_level = "trusted"
and slipped straight past it. That line makes codex load the target repo's own
.codex/config.toml, which then supplies developer_instructions, model and —
under --dangerously-bypass-hook-trust — repo-authored hooks. Reproduced
against codex 0.152.1 with a deterministic signal: with the line the repo
layer's model was used, without it the runner's was. Indented `base_url` or
`command` lines dodged the duplicate-count checks the same way.
Both files are now pinned by whole-file SHA-256, which has no such blind spot.
The expected digests come from the runner's own memory: Bootstrap records what
it uploaded and Run reads it back. They are deliberately NOT recorded in the
manifest, which is a file in the same agent-writable directory — an agent that
rewrote config.toml could rewrite a hash recorded there to match. Run therefore
requires Bootstrap to have run in the same process for the same sandbox
(internal/cli/run.go does) and fails closed with an error naming that
assumption otherwise. The runtime is absent from config.ValidRuntimes(), so no
configuration can reach this path yet.
Three more holes in the same area:
* the hook scripts had no integrity check at all, so tirith_check.py could be
rewritten to exit 0 while every other guard passed. Every file in the hooks
directory must now match one of this binary's embedded script digests —
compile-time constants, no anchor in the sandbox needed. This makes codex
stricter than Claude Code and pi, where the scripts stay unchecked.
* hooks.json invoked a bare `python3`, resolved through a PATH the agent
controls because codex spawns hooks after .env is sourced. Bootstrap now
resolves the absolute interpreter in its preflight and renders it with -I;
Run also unsets PYTHONPATH, PYTHONHOME and PYTHONSTARTUP.
* the adapter had no top-level exception handler, so an unexpected failure
would exit 1 — which codex records as Failed, which does not block. Any
exception now routes to a block, and a malformed payload blocks on both
phases rather than only PreToolUse.
base_url and auth.command are additionally passed as `-c` SessionFlags, which
were verified to beat the file. There is no such pin for project trust:
`-c projects={}` and a scalar trust_level="untrusted" were both tried and
neither overrides it, which is why config.toml integrity is enforced by digest
rather than by overrides alone.
Refs #6920
Assisted-by: Claude (implementation)
Signed-off-by: Wayne Sun <gsun@redhat.com>
…n the hook block
Three fixes the end-to-end gateway smoke and the review round surfaced.
`codex --version` prints "codex-cli X.Y.Z" where pi prints a bare number, and
the renderer writes "(v" + this + ")", so a run logged
`Agent: gpt-5-mini (vcodex-cli 0.152.1)`. The preflight now parses the number
out and fails closed on an unexpected shape rather than rendering it verbatim.
`--output-last-message` pointed at /sandbox/workspace/output/, which is the
agent's own result directory: nothing creates it up front, so codex reported
`Failed to write last message file ... No such file or directory`, and had it
succeeded the file would have been extracted as one of the agent's output
files. It goes under the runner-owned config dir instead, where
ClearIterationArtifacts sweeps it; the final message is in the stream as an
agent_message either way.
Hardening, all fail-open paths:
* the adapter's block() wrote to stderr unguarded. A stderr that is already a
broken pipe would take the interpreter down with exit 1 — which codex
records as Failed, and a failed hook does not block. The write is now
suppressed and exit 2 is reached unconditionally: a block without its
reason still beats a block that never happens.
* Run now asserts that the runner's hooks signal and the recorded hooks.json
digest agree. Both derive from SecurityEnabled() today, but nothing said
so, and a refactor that split them would drop hooks.json from the guard
while the adapter still loaded, with no failing test.
* codexHooksAdapterCheck's comment now says it is defense in depth, not the
boundary: the count comparison is bypassable on its own (pad the adapter
path into the free-form description field to restore parity), and what
makes hooks.json tamper-evident is the whole-file digest.
Refs #6920
Assisted-by: Claude (implementation)
Signed-off-by: Wayne Sun <gsun@redhat.com>
…file suffixes Three holes in the layer around the guards, each reproduced before it was fixed. The hook-script guard bound digests to a *set*, not to filenames. Overwriting hooks/tirith_check.py with the bytes of hook_io.py therefore passed: the digest is in the allowed set, hooks.json is untouched, and hook_io.py is a library module with no __main__, so it exits 0 with empty stdout — which the adapter reads as "allow". Tirith silently neutered with every guard green. Each digest is now bound to its filename, the expected names come from the runner's memory (Bootstrap records the set the harness enabled), and the directory must hold exactly those entries: every one enumerated with find, every one a regular file. That also refuses a symlink to an allowed file, which test -f and sha256sum both accept and find does not. The old check globbed `*.py`, so an agent could plant a package directory it never saw — hooks/json/__init__.py — and normal Python, which prepends a script's own directory to sys.path, imported it when a hook script did `import json`. The exhaustive directory check closes the planting vector, and the adapter now also runs each script with -I. That needed care: the scripts import their siblings (hook_io, the sanitizer stages), so the verified hooks directory goes back on sys.path explicitly — appended, not prepended, because prepending puts it ahead of the standard library and re-opens the very hole -I closes. Verified both ways, and that tirith_check and posttool_chain still block correctly under isolation. ExtractTranscripts trusted a file suffix in an agent-writable directory: it collected `.jsonl.zst`, and codexRedactFile skipped `.zst` with a warning, so a plaintext file merely *named* that shipped as an unredacted artifact. Codex writes the running session uncompressed, so `.zst` is never this iteration's transcript and is no longer collected at all; every remaining candidate must parse as a codex rollout envelope on its first line, and anything that fails that — or that cannot be redacted — is discarded rather than presented as the run's transcript. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
…block unreadable hook output Follow-ups from the second review pass, on the layer around the guards. ExtractTranscripts passed `find` output straight to a download. A path from an agent-writable directory is now checked before it is followed: it must sit under the sessions directory — a sibling like `<sessions>-evil/` is refused, not just a `..` segment — and carry no control characters, since a newline in a filename splits one listing entry into two. `find -type f` already skipped symlinks, so a link to /sandbox/workspace/.env was never listed; the prefix check covers the rest of the class. codex-debug.log was downloaded unredacted. codex logs at error level by default and raises to whatever RUST_LOG asks for, so it carries the same material output.jsonl does and now gets the same pattern redaction; a redaction that fails deletes the file rather than shipping it, since it is a convenience artifact the run does not depend on. The adapter treated hook output it could not interpret as "nothing to do": a script that printed a rewrite or a decision in a shape the adapter did not recognise was read as an allow. That is now a block, which is the whole reason the adapter exists. Its children also run with `-s` and a PYTHON*-stripped environment carrying PYTHONNOUSERSITE=1, so a `sitecustomize.py` cannot run at interpreter start — the scripts' own configuration (FULLSEND_CANARY_TOKEN, TIRITH_*, the allowlists) is passed through, and a container check confirms the allowlist hook still sees it and still blocks. Tests: the hooks directory now also refuses __pycache__, an extension module and a nested package; a spoofed transcript is absent from the artifact set; an empty or truncated stream with codex exiting 0 still fails the run. Verified in the sandbox image after the isolation change — allow 0, tirith block 2, PostToolUse chain 0, canary block 2, tool-allowlist block 2, and a planted hooks/json package no longer executes. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
Lifts codex_redact.go from 79.6% to 96.3% and codex_transcript.go from 79.2% to 89.6%, so the per-file view a reviewer clicks on matches the aggregate. Both were short only on error branches the fake sandbox always succeeds at, so the tests inject the failures: a sink that fails mid-tee, a stream with no newline reaching the line cap, a missing file, a download that fails, and an unreachable sandbox. Two of them document behaviour rather than just exercising it. The tee side of an io.TeeReader must report a full write even when the sink fails — reporting short would abort the read of an otherwise fine stream — while still surfacing the error, and both halves are now asserted. And a path outside the sessions directory is asserted never to reach a download at all, which is the property the prefix check exists for rather than an incidental one. Writing them turned up one thing worth stating in the test: a `find` that exits non-zero is deliberately not an error, because the command carries `|| true` — find reports a non-zero status for an unreadable subdirectory, which is no reason to fail a run. The error path there is an unreachable sandbox, and that is what the test now covers. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
…guarded dir
`-I` does not imply `-B`, and `-E` makes PYTHONDONTWRITEBYTECODE inert, so the
first hook that imported a sibling wrote hooks/__pycache__/*.pyc. Nothing
clears the hooks directory between iterations and Run's guard requires it to
hold exactly the files fullsend installed, so iteration 1 of a run armed a
fail-closed lockout for iteration 2: a validation-loop retry would refuse to
start and blame tampering. Reproduced as four .pyc files after a single
PostToolUse chain run.
The children now run with -B and set sys.dont_write_bytecode, and a test runs
both phases three times over and asserts both that the directory is unchanged
and that the guard still passes afterwards — the property that actually
matters for the next iteration. An agent-planted __pycache__ is still refused.
Four things around it:
* the sys.path ordering rests on -I keeping a script's own directory off
sys.path, so Bootstrap's preflight now asserts Python >= 3.11 rather than
assuming it, and an adapter-level test plants hooks/json/__init__.py and
proves the standard library still wins.
* "security enabled, every hook disabled" was a hard error. That is a legal
harness — pi allows it, distinguishing a nil hook plan from an empty one —
so the check is now on a nil digest map rather than an empty one.
* codexIsRolloutFile swallowed scanner.Err(), reporting an oversized first
line as "file is empty"; it still fails closed, now for the real reason.
* the contributing page still claimed .jsonl.zst rollouts are collected.
PYTHONNOUSERSITE is inert under -I; it stays as a second latch with a comment
saying so.
Refs #6920
Assisted-by: Claude (implementation)
Signed-off-by: Wayne Sun <gsun@redhat.com>
…er invocation Three more ways the hook chain could be neutered with every digest green. PATH was never re-pinned after the agent-writable .env was sourced, and the hook scripts resolve their tools by name — tirith_check.py runs a bare `tirith`. A .env prepending a directory with a fake `tirith` that exits 0 therefore disabled the entire PreToolUse chain for the whole run. Reproduced: the same blocked command returned exit 0 with the plant and exit 2 without it. Run now captures PATH into a readonly variable before .env and restores it after, and exports it so the adapter can set its children's PATH from a value that never passed through .env rather than trusting what it inherits — the defence should not rest on nothing having touched PATH in between. Verified in the sandbox image both ways. LD_PRELOAD, LD_LIBRARY_PATH and LD_AUDIT reached codex and the hook children from .env, where they would load code into any dynamically linked program the run starts — codex's own binary, tirith, git — before its main, which no digest of ours would see. They are unset after .env and stripped from the children's environment. The hook-script digests were checked once, at launch, leaving the agent the whole iteration to rewrite a script before the next tool call reached it — a window Claude Code and pi leave open too. The run command now exports the name→digest map into the codex process's environment after .env and before launch, and the adapter re-reads and re-verifies each script immediately before spawning it. A running process's environment is not something the agent can reach, so this is trustworthy at every invocation; a missing or malformed map is itself a block. Also: a rollout is validated at every line rather than only its first, so a file cannot open with one genuine envelope and carry anything after it; transcripts are downloaded to a staging name and renamed into place only after validation and redaction, so a crash cannot leave raw tool output where the artifact collector reads; and reads are bounded, so a planted multi-gigabyte file is refused rather than pulled into the runner's memory. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
The adapter forwards the hook scripts' configuration — FULLSEND_EGRESS_ALLOWLIST, TIRITH_REQUIRED, TIRITH_FAIL_ON — from codex's environment, and appendHookEnv writes those into the workspace .env at bootstrap. That file is agent-writable, so iteration 1 could widen the SSRF allowlist or clear TIRITH_REQUIRED for iteration 2 and the hooks would read the agent's values. Reproduced: sourcing an agent-appended .env yields the attacker's allowlist. The runner now re-exports what it derived from SandboxHookConfig after .env, so the .env copy cannot win, and the adapter passes exactly those to its children — the same shape as the PATH pin. FULLSEND_CANARY_TOKEN and FULLSEND_TOOL_ALLOWLIST are deliberately not covered: they come from the harness's own env/host_files rather than from the hook config, so there is no runner-side copy to re-export, and ADR 0100 says so rather than implying the whole set is pinned. ADR 0099 decided two things, which the repo's writing-adrs skill forbids and three reviewers have now flagged. Split: 0099 keeps the credential decision (custom provider, auth.command, runner-seeded token file) and states plainly that it amends ADR 0092, whose in-sandbox half is pi-specific; 0100 takes the sandbox-hooks decision (protocol translation, detect+block, per-invocation digest re-verification, pinned interpreter and PATH). Both are Accepted, both within the skill's sizing, and the implementation evidence stays in the contributing page. Living documents updated with them, as acceptance requires: a cross-reference annotation on 0092 (permitted for an accepted ADR; no rewrite of its Context, Decision or Consequences), and the threat model's claim that "agents cannot modify their own guardrails" now carries the qualification it needs — the hook scripts stay writable between iterations on Claude Code and pi, and are digest-verified per invocation only on codex. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
`agent-architecture.md` has no open question ADR 0100 resolves and no passage that substantively discusses sandbox tool hooks — its only "runtime" mention is about orchestration frameworks. `tool-call-risk-assessment.md` does: it names Tirith, the SSRF validator, the canary hook and the unicode normalizer, and has a "Relationship to existing hooks" section about exactly the layer this ADR wires into codex. So `relates_to` names that doc instead, Context links it, and the reciprocal backlink AGENTS.md asks for goes at the passage that motivates it — noting that how far those hooks reach depends on the runtime carrying them, which is the difference 0100 records. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
"Process memory" reads as agent memory in a codebase full of agents, which is the opposite of what it means here. The concept is now named for what it is throughout: **runner-held digests** — digests the runner records outside the sandbox at Bootstrap and injects into the launch command at Run. Renames codexArtifactHashes to codexRunnerHeldDigests, its record/lookup/forget helpers to match, codexUploadedHashes to codexRunnerHeldDigestSet, the local `hashes` variables to `digests`, and the affected test names. The guard's failure text now says "no runner-held config digests" rather than referring to recorded hashes. ADRs 0099 and 0100, the contributing page's anchor table and its prose carry the same term and state the definition once. "Anchor" survives in exactly one place, which is the sentence it earns: why the agent-writable manifest was rejected as the place to keep these digests. No behaviour change; the guards, their inputs and their exit codes are untouched. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
…ting the manifest model Three items from the review of #6926. PR B renamed the stream fixtures to `.ndjson`, and the codex runtime tests still asked for `.jsonl`. The fake sandbox then streamed nothing, so TestCodexRun_StreamVerdictOverridesExitZero passed for the wrong reason (an empty stream is also a failed run) while _SuccessfulRunReportsMetrics and _AcceptsManifestHookPlan failed outright. Every fixture reference now names `.ndjson`; the destination names in ParseTranscriptErrors stay `.jsonl`, because that is what the runtime actually writes and what it scans for. One stale `basic_run.jsonl` mention in PR B's own doc comment is corrected in passing. An empty PreToolUse payload exited 0, which the adapter reads as allow. The scripts treat empty stdin as "no tool call" and that is right for them — they also run standalone — but the adapter was invoked *because* a tool call is about to happen, so an empty payload means the call cannot be scanned rather than that there is nothing to scan. It now blocks. PostToolUse keeps the no-op: the call has already run and there is nothing left to prevent. Run took its fallback model from the manifest, which sits in the agent-writable config directory and carries no digest — so an agent could change the model, and the cost tier, a validation retry runs on. Bootstrap now records the model it parsed from the agent definition alongside the runner-held digests, and Run resolves from that; the manifest's copy is never read. The test seeds a manifest naming a different model and asserts the launch ignores it. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
FULLSEND_CANARY_TOKEN and FULLSEND_TOOL_ALLOWLIST reach the hooks through the agent-writable workspace .env like the other three, so iteration 1 could clear the canary token or rewrite the tool allowlist and disable those checks for iteration 2. I had recorded this as unfixable in the runtime because SandboxHookConfig does not carry them — that was wrong, and the review was right. The runner writes .env and then calls Bootstrap, before any agent iteration runs, so at that moment the file is still exactly what the runner put there. Bootstrap reads the two values back from it and records them beside the other security values in the runner-held state; Run re-exports all five after .env, so the agent's copy cannot win, and the adapter passes them to its children. Reading them in Run instead would read whatever the previous iteration left behind, which is the exposure itself. Unset values are skipped: there is nothing to re-assert, and an agent that *sets* one later can only cause spurious blocks, not slip past a check. A malformed answer from the sandbox fails the run rather than silently yielding no pins. ADR 0100's consequence loses its "these two stay at Claude/pi exposure" caveat, and the matrix and contributing text say the canary token is pinned on codex. Refs #6920 Assisted-by: Claude (implementation) Signed-off-by: Wayne Sun <gsun@redhat.com>
waynesun09
force-pushed
the
codex-runtime-core
branch
from
September 3, 2026 00:47
e95b116 to
11a562d
Compare
|
🤖 Finished Review · ✅ Success · Started 12:50 AM UTC · Completed 1:11 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $12.62 |
|
Review skipped — this PR is already merged. The Posted by fullsend post-review check |
|
🤖 Finished Retro · ✅ Success · Started 1:12 AM UTC · Completed 1:32 AM UTC Commit: Runtime: claude · Model: opus → claude-opus-4-6 · Effort: high · Cost: $6.72 |
Retro: PR #6926 — Codex agent runtime (ADR 0099, ADR 0100)PR: #6926 by waynesun09 — 6,651 additions across 26 files implementing the Codex CLI as a fullsend agent runtime with secretless OpenAI credential delivery, sandbox hooks, and security hardening. Timeline
What went wellBoth human and agent reviews drove real security improvements. The human reviewer caught a test-correctness bug (fixture naming mismatch causing vacuously-passing tests) and a security gap (fail-open on empty stdin for PreToolUse) that no agent identified. The review agent found a distinct security gap (canary token and tool allowlist not re-asserted after .env sourcing) that the human missed. Qodo additionally flagged hooks mutability, SSRF via mutable environment, and manifest-controlled model — all fixed before merge. The combined review surface was significantly stronger than either human or agent review alone. The author iterated diligently after approval. Despite receiving human approval at 21:16Z, the author continued addressing agent findings for 4 more hours, ultimately fixing the medium-severity canary token fail-open before merging. The system worked as designed. Evidence for existing issues
Proposals filed
|
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
2 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.
Summary
CodexRuntime: Codex CLI as a fullsend agent runtime with the same secretless OpenAI credential path pi uses. ADR 0099.fullsend-openai(base_urlapi.openai.com,wire_api = responses, HTTP/SSE — no websockets) whoseauth.commandprints the OpenShell gateway placeholder from a runner-seeded token file underCODEX_HOME; codex re-runs it on its refresh interval and on 401, so a running iteration follows every credential rotation. The runner re-seeds the file after each refresh through the seeder interface from PR C. The built-inopenaiprovider reads the key once at startup and cannot be overridden, so it is not used.config.toml(approvals off, codex's own sandbox off inside OpenShell,developer_instructionsfrom the agent definition, bundled skills off, web search off),hooks.json+ an embedded adapter that runs fullsend's shared hook scripts and translates the wire protocol (fullsend exit-1 block → codex exit-2 block; PostToolUse can block but not rewrite built-in tool output, so the sanitizer stages warn the model), skills toCODEX_HOME/skills.config.tomlandhooks.jsonheld in the runner's process memory (the sandbox manifest is agent-writable, so it is not an anchor) and checked in the launch command; the hooks directory must contain exactly the expected files, each bound to its own digest from the binary's embedded scripts; absolutepython3 -I -s -Bfor adapter and children with a stripped env;-cpins for approval, sandbox, provider,base_urlandauth.command; the repo stays untrusted so its.codex/layer never loads;OPENAI_BASE_URLand loader/Python variables unset after the agent-writable.env.codex exec --jsonwith the prompt on stdin, PR B's parser for events/metrics (verdict from the stream, not the exit code), rollout JSONL transcripts validated as codex envelopes and pattern-redacted,output.jsonlandcodex-debug.logredacted,.jsonl.zstnot trusted by extension, per-iteration cleanup with the shared stray-process sweep.openai/<id>or bare id); Claude aliases are refused with an error namingFULLSEND_CODEX_MODEL=openai/<id>ormodel: openai/<id>on the agents: entry — Claude aliases are never mixed with GPT.Review rounds
Two full rounds (Codex gpt-5.6-sol + Grok 4.6) and three targeted re-reviews. Every HIGH was reproduced before fixing: grep-based config guard bypassed by a dotted-key project-trust line (repo
.codex/layer loaded — reproduced with a repo-supplied model); existential hooks.json check; barepython3via PATH; adapter exceptions and malformed payloads fail-open; hook-script digests bound to a set, not filenames; plantedhooks/json/__init__.pyexecuted by unisolated children;.zstfiles kept unredacted; children writinghooks/__pycache__that the exhaustive guard then refused on the next iteration (self-inflicted lockout).Verification
dotfile_overwrite), PostToolUse canary block withholds the output, hooks.json loads with no[hooks]table, repo.agents/skillsdiscovered under an untrusted project,codex doctorreports the custom provider and no websocket transport.POST /v1/responsesallowed;GET /v1/modelsdenied once plus one retry (harmless);chatgpt.comandapi.github.comprobes denied at L4; metricsruntime: codex; credential rotation on a kept sandbox → re-seed → second successful turn; provider deleted at the end. A run refused at model resolution deletes its sandbox and provider.Part of a five-PR stack for #6920 (Codex as an agent runtime): A image pin → B stream parser → C OpenAI credential seeder → D runtime core (ADR 0099) → E enable + docs. Each PR is reviewable on its own diff; they merge bottom-up. Plan and verified Codex facts:
research/fullsend-codex-runtime-plan.mdin the ai-workspace-public research repo (to be linked once pushed).Refs #6920
Assisted-by: Claude (implementation and review orchestration), Codex gpt-5.6-sol (review), Grok 4.6 (review)
ADRs and living docs (after review)
auth.command+ runner-seeded token file) and amends ADR 0092, whose in-sandbox half was pi-specific (auth.json,**/node); ADR 0092 receives only a one-line cross-reference annotation, asdocs/contributing/adrs.mdallows for accepted ADRs. ADR 0100 decides sandbox hooks on codex (adapter, exit-code translation, detect + block, per-invocation digest re-verification, pinned interpreter/PATH, security env re-exported after.env).docs/problems/security-threat-model.md: the claim that agents cannot modify their own guardrails is qualified — hook scripts stay writable between iterations on Claude Code and pi; codex re-verifies them per invocation (ADR 0100).FULLSEND_CANARY_TOKENandFULLSEND_TOOL_ALLOWLISTcome from the harnessenv/host_files, so the runtime has no runner-side copy to re-export; stated in ADR 0100, follow-up on the runner side.