Repository navigation
Retain execution records and sampled instruction receipts - #5500
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
Security findingsAdvisory findings (1)
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 9b1faae. Reviewed native receipt capture/publication, instruction-file provenance, portable checkpoint redaction, retained execution events, history deduplication, completion ownership, bounded observations, exports, and regression coverage against the immediate base. No actionable findings or missing split dependency identified.
Static review only; no candidate code or tests run. Hosted lint/docs passed; Linux/macOS native CI is still running, Windows jobs skipped. Leave unmerged as requested.
9b1faae to
c516553
Compare
95d38b9 to
1863681
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at c516553. Reviewed the public executor receipt projection and updated tests/API fixture. Internal native permission profiles are withheld from public receipts while retained native policy checks remain intact. No actionable findings.
Also independently examined the repair-parent gap finding. The direct gap call does bypass parent forwarding, but output-correction sessions are constructed without tools, executors, MCP servers, inherited state, or user/project configuration. I could not identify a supported execution-completion producer in those sessions; ordinary sessions have no repair_parent. A supported reproduction would change this assessment, but I am not treating an injected impossible event as a blocker.
Static review only; no candidate code or tests run. Fresh hosted native CI remains pending. Leave unmerged as requested.
![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: c5165530b9
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Changes requested at c516553. This supersedes my approval after independently validating the newly posted child instruction-file provenance finding.
A supported session with instruction_files, NativeAgents and an executor can spawn a default-role child with fork_turns="none". The child receives concatenated file text through subagent_developer_instructions, but file receipts are published only on the root. The child receipt consequently omits the application-file provenance despite sampling its contents. Carry the metadata into the child receipt seed/publication and add a regression that checks the child receipt against the supplied file hashes. The existing thread contains the inline finding, so no duplicate added.
The earlier repair-parent execution-gap assessment remains separate; that proposed trigger lacks a supported producer. Static review only; no candidate code/tests run. Leave unmerged.
c516553 to
f36abef
Compare
1863681 to
f52af83
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The child/grandchild instruction-file provenance fix now records files actually sampled by default NativeAgents children and excludes custom roles. The earlier receipt blocker is addressed; no further actionable finding in this layer.
Static review only; no candidate code or tests executed. Hosted CI: pending: native-linux / Native package (ubuntu-24.04), native-linux / Native package (ubuntu-24.04), native-macos / Native package (macos-latest), native-macos / Native package (macos-latest); no failed checks observed.
f36abef to
199eb43
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The receipt reconstruction fix is sound: only validated sampling observations matching an existing canonical turn context are skipped, while canonical settings remain available even with malformed or foreign receipt metadata. Reviewed the regression coverage and exact tree changes; no actionable findings.
Static review only; no candidate code or tests executed. Hosted checks pending: native-linux / Native package (ubuntu-24.04), native-macos / Native package (macos-latest). Failed checks: none observed.
f52af83 to
f23f470
Compare
199eb43 to
0bc9468
Compare
722e750 to
367bb23
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed this exact commit and the inherited changes. The latest-sampling receipt fix is correct: a later non-null unsupported receipt invalidates earlier metadata for that turn, while ordinary contexts without a sampling receipt preserve it. The regression source covers the intended distinction. No actionable findings in the updated delta. This addresses the previously blocking finding at #5500 (comment) and supersedes my request-changes decision on the earlier head.
Static review only; no candidate code or tests executed locally. Hosted CI: pending checks remain. No current failures observed.
367bb23 to
d892d29
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-reviewed this exact commit. Strict public receipt validation correctly rejects malformed fields and nested values, preserves canonical turn identity and supported approval-policy forms, and removes internal extras. This integrates correctly with latest-sample invalidation. No actionable findings in the delta.
Static review only; no candidate code or tests executed locally. Hosted CI: pending checks remain, including platform validation where scheduled. No current failures observed.
5cde9ce to
607c06b
Compare
d892d29 to
c397c77
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c397c7750f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at c397c77. Reviewed the incremental Linux/Windows runner routing and matching change-detection assertions against the previously reviewed commit; no new correctness or security findings. Runtime code is unchanged. Hosted CI is still pending. Static review only; no candidate code or local tests executed.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved the incremental instruction-file provenance fix and late-completion documentation/regressions. Restore keeps historical receipts intact while using the current application sources for future sampling, including explicit clearing; supplied compaction carries the live file provenance under its existing exclusive lease. Native-agent option propagation remains paired. Coordinator and independent static reviews found no new actionable issue.
Hosted lint, docs, and change selection passed; native Linux/macOS validation remains pending. No candidate code or local tests executed.
1770bd7 to
d2fb57c
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Re-approved this merge-only update: its complete Git tree is identical to the previously reviewed head. No source, configuration, documentation or test content changed. Coordinator and independent static verification; current-head hosted CI is still pending. No candidate code or local tests executed.
Review stack: #5504 (CI) > #5499 (turn controls) > #5500 (receipts) > #5501 (executor) > #5502 (recovery) > #5503 (example) > #5495 (docs). Combined reference: #5494.
Requirements ·
REQ734for turn and execution evidence andREQ732for the recorded effective-policy fields.Summary
Portable restore preserves the latest canonical turn context even when a sampling record carries a malformed or foreign receipt. A matching duplicate sampling record can still be skipped.
A default child or grandchild retains the instruction-file digest when the native model actually sampled that file. A custom child that supplied the same text keeps its own instruction provenance.
A supervisor needs command, patch, and MCP evidence after a tool completes or an event gap occurs. By default,
ExecutionItemCompletedreports the item kind, status, and native IDs. Trusted readers can passinclude_execution_details=Truetosession.events()orturn.events()for command, patch, and MCP payloads. Each detailed reader gets its own copy. When an oversized or late observation reports a gap, read the full retained record throughSession.items().Add trusted
instruction_filesinput and optionalRecordedTurn.receiptmetadata for sampled instruction hashes, model/settings and native build identity. The receipt records the prepared sampling context; it does not attest to later tool reads or provider routing. Public executor receipts expose selected typed fields and omit the rawpermission_profile; projection filters unrecognized executor fields. Portable checkpoints withhold executor policy details. Old records and unknown receipt versions remain explicitly unavailable.#5501 applies and restores the executor grants. Layer 2 of 5; base
feat/supervisor-turns. Core patches 0019–0020 build on layer 1. Local execution and the early sandbox-denial completion path follow in layer 3. Model auth remains API-key-only;REQ731is excluded.Acceptance dependencies enabled here:
REQ780needs execution evidence for artifact checks, andREQ779/REQ781use recorded context and outcomes through revision and recovery. This layer supplies records; it does not perform those application checks.Instruction loading verifies the selected regular file's identity before reading it. Stable application-selected symlinks work. The application must select trusted files and parent directories.
The turn history reports the latest sampling attempt. If a newer attempt uses an unsupported receipt version, the SDK omits the receipt for that turn. An ordinary context record with no sampling receipt preserves the last observation. A later supported receipt becomes visible again.
After restore, a new turn records the instruction source that the caller supplied, including when two files or inline instructions contain identical text. Earlier turns retain their original receipts.
A turn event iterator finishes after the root turn ends and pending observations settle. A detached cell can finish later. A supervisor can follow session events from the terminal event's sequence and read retained command details from session history.
Test plan
Installed Linux wheels passed all eight focused provenance tests on Python 3.10 and 3.13. These covered six restore combinations, supplied compaction, and receipt history. Three Linux WSS cases also passed with an installed wheel. They exercised a detached command that finished while the session was idle, during the next turn, and after it.
A native portable restore reproduced two invalid version-1 receipts, one with a null model and one with a numeric instruction-file list. The public history now omits the invalid receipt and recovers when a later valid sample arrives. All 23 focused history and receipt tests passed.
The integrated Python source passed 24 history, turn-receipt and crash-receipt checks.
Focused tests replace the selected file with another regular file and with a symlink between inspection and open. Both replacements are rejected. Stable symlinks and native receipts passed.
The receipt layer passed 87 focused tests. The two portable replay regressions failed on its previous native runtime and passed on the updated build.
The finalized regression failed on the previous runtime with an empty file receipt and passed on the rebuilt runtime. The related suite passed 40 tests and skipped 17 platform-dependent cases in the local test sandbox.
The observation update passed 28 focused tests. Three more native cases passed in an unprivileged Linux sandbox. They covered WSS command observations, terminal and patch details, and withholding rejected command output.
Issue number
None linked. Owning requirements database IDs are listed above.
Checks
AGENTS.md, including.agents/skills/code-change-verification/scripts/run.shwhen requiredAGENTS.md