Repository navigation
Add a supervised worker recovery and revision example - #5503
Conversation
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ 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 b2c6f2b. Reviewed the example and integration coverage against the reconciliation layer: isolated worker roots, exact steering, artifact and command verification, immutable publication records, cancellation cleanup, and application-confirmed recovery. No actionable findings. The full source tree matches the previously approved implementation at be0951f.
Static review only; no candidate code or tests run. Hosted lint/docs checks passed; Linux/macOS native CI remains running and Windows checks are skipped. Leave unmerged as requested.
b15f244 to
d2f6580
Compare
b2c6f2b to
114be08
Compare
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Approved at 114be08.
Reviewed the example update against its immediate base. Calls use steer_exact with the observed native ID and the reconciliation convenience method only after the existing application-record checks. No actionable findings in this layer; underlying executor blockers remain tracked on5501.
Static review only; no candidate code or tests run. Fresh hosted 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: 114be08838
ℹ️ 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".
d2f6580 to
94defa9
Compare
114be08 to
5b32b3e
Compare
|
@codex review |
|
Codex Review: Didn't find any major issues. Can't wait for the next one! Reviewed commit: ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
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". |
🛡️ Codex Security Review · Automatically triggeredSecurity review completed. No security issues were found in this pull request. Reviewed commit: Only the user who started this review can view the report in Codex. ℹ️ About Codex security reviews in GitHubThis is an experimental Codex feature. Security reviews are triggered when:
Once complete, Codex will leave suggestions, or a comment if no findings are found. |
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
The worker-layer changes are clean, but this stacked head inherits the confirmed recovery input-corruption issue in #5502 (comment). Please carry the upstream fix through this layer before merging. No additional inline finding.
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.
94defa9 to
109e6e0
Compare
5b32b3e to
09656bc
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09656bceac
ℹ️ 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.
The upstream custom-input recovery issue is fixed. Two existing worker findings remain valid: #5503 (comment) — return the verified journal result after recovery instead of arbitrary model narration; #5503 (comment) — subtract submission time before supervision so effect/recovery share one per-turn timeout budget, as work already does. No duplicate inline comments.
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.
09656bc to
86d35e4
Compare
22e656e to
deb5647
Compare
bdffdba to
38cea47
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 38cea47d38
ℹ️ 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.
The shared physical write-root and storage protections are sound: filesystem identities cover alternate paths, checklist aliases and hard links are checked, and private runtime symlinks fail closed. Disjoint storage and write grants remain supported. Both prior worker findings are addressed: verification output is compared exactly, and Unix-only helper cases are guarded on Windows. Acceptance is persisted only after owned resource cleanup succeeds; directory identity is captured once and primary read failures survive cleanup failures. The executor startup fixture removes the parent-selected port race.
Coordinator and independent static reviews of actual old/new file contents found no actionable issue. Hosted CI on this new head remains pending; prior-head results do not qualify it. 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 supported-platform guard. The only incremental change restricts the prior-snapshot publication test to Linux/macOS, matching the example’s directory-fsync requirements and neighboring tests. This resolves the reported Windows PermissionError without removing supported-platform coverage. Coordinator and independent static review found no new issue. Current-head hosted checks: {'SUCCESS': 3, 'IN_PROGRESS': 2, 'SKIPPED': 2}. Native validation remains pending; the earlier Cargo fixture timeout is separate. No candidate code or local tests executed.
jbeckwith-oai
left a comment
jbeckwith-oai
left a comment
There was a problem hiding this comment.
Static re-review: immutable-record publication now retries directory fsync after an identical existing record, so retry success follows durable directory publication. The regression covers two failed directory syncs followed by successful idempotent publication. Other feature changes remain unchanged. Coordinator and independent review found no actionable issues. Hosted CI remains 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) > #5509 (automatic compaction during cancellation). Combined reference: #5494.
Requirements ·
REQ733,REQ736,REQ737,REQ738,REQ779,REQ780,REQ781.Summary
The application checks the effect journal's session ID, turn ID and result before it reports completion. A stale journal entry or missing effect keeps the worker out of the completed state.
Applications need a concrete way to supervise work across client disconnects and process failure. Add a two-process worker example using separate workspaces, native histories and application journals. Each worker uses
Turn.steer_exact()for exact steering, denies unattended approvals, selects trusted MCP/instruction inputs and applies startup/turn deadlines with explicit cancellation cleanup.Validate the structured claim against the complete expected artifact and native command evidence before acceptance. The verifier checks the command output byte for byte and reads one byte beyond the expected file length to catch trailing data. The worker records acceptance in its original journal directory only after its session and provider close successfully. A secondary close error preserves the original artifact read failure. Request revisions in the same conversation, reconnect a status client to existing records and reconcile a simulated effect after a crash with
UncertainCall.reconcile(). Publication writes a separate local marker with a stable key. The application owns deployment, journals, acceptance and external publication policy.The example records separate startup, follow-up, first-tool and completion intervals. These reference measurements do not establish comparative speed or acceptance by a production application. Managed model login/billing remain excluded under
REQ731.Layer 5 of 5; base
feat/worker-reconciliation. Documentation/adoption is tracked separately underREQ753.When a deadline or observer fails and cancellation also fails, the worker re-raises the original error with its cause and logs the cleanup error with its traceback.
The supervisor must keep each private job directory at the same canonical path while a worker, status reader, or publisher is using it. The example supports reconnecting to that existing directory. Archive or replace it after those operations finish.
Test plan
The combined PR's Windows 3.10 and 3.14 jobs both failed two artifact tests on the missing Unix flag
O_NOFOLLOW. Those tests now run on the same Linux and macOS platforms supported by the local worker. The platform-independent worker tests still run on Windows.The dual-failure regression reproduced on the previous worker and passed on Python 3.10 and 3.13. The ordinary caller-cancellation case also passed.
The integrated worker source passed 141 focused tests, with five platform-dependent cases skipped locally. Those tests include receipt, recovery, executor and worker behavior. Current hosted checks and independent review are running.
Six focused cases failed before the fix. The updated worker passed all 15 cases, including a directory symlink retargeted during shutdown, withheld acceptance during close, preservation of a prior accepted record, exact command output, and artifact read/close errors.
The complete two-worker scenario passed on a hosted Ubuntu runner in 10.73 seconds with its original deadline. It exercised acceptance, reconnect, effect recovery and publish-once using the updated worker source and a previously built official native wheel. Installed worker run
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