🤖 fix: admit fresh automatic input after settled Stop - #4209
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. |
|
@codex review Please review current head Generated with |
![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: 8aab3bd7b3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
8aab3bd to
463d635
Compare
|
Addressed all three findings in
Validation: 923 tests across eight suites, 240 budget tests covering the final test-seam adjustment, and canonical static on final tree Generated with |
|
@codex review Please review current head Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 463d63508d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
🛡️ Codex Security Review · Automatically triggered
Here are some automated security review suggestions for this pull request.
Reviewed commit: 463d63508d
ℹ️ About Codex security reviews in GitHub
This is an experimental Codex feature. Security reviews are triggered when:
- You comment "@codex security review"
- A regular code review gets triggered (for example, "@codex review" or when a PR is opened), and you’re opted in so security review runs alongside code review
Once complete, Codex will leave suggestions, or a comment if no findings are found.
463d635 to
2f0a66a
Compare
|
The failed unit job on463d635 exposed14 obsolete test-seam cases after automatic publication moved to guarded batch acceptance. The fixtures now inject failure/cancellation at the real HistoryService acceptance boundary. Per-prefix failure/cancellation states no longer exist in this layer, so two batch cases replace seven old parameter cases while checking durable receipt, own-row rollback and foreign-row preservation. Five affected suites62/432 and canonical static pass; the composed lower-plus-top suite145/870 passes. Runtime Stop-cleanup review findings remain open and are being addressed separately. Generated with |
|
@codex review Generated with |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2f0a66a0f1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
🛡️ 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. |
2f0a66a to
9c3904c
Compare
|
Responses to all current findings:
Fresh reset heartbeat recovery also retains durable generation provenance through restart and optional-sidecar failure, while the real follow-up input remains the replacement witness. The combined candidate passed 1,077 tests / 4,472 assertions, followed by 39 focused cases / 169 assertions for the final cleanup-handler/test-typing delta. Canonical static checks passed on the final composed tree, and independent source/delta reviews are clean. Generated with |
|
@codex review Generated with |
🛡️ 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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9c3904c24c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Replay the existing reviewed B layer on its corrected prerequisites: versioned settlement proof, original automatic admission CAS, guarded batch publication, and receipt-based budget and rollback accounting. Its existing CI fixtures remain part of this layer. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
Wait for the captured producer and exact outer cleanup before V2 replacement authority. Preserve eventual wake after failed physical Stop or retryable monitor retirement, keep unresolved foreign V1 blocked, and retain legacy scoped-V1 ordinary admission. Stamp heartbeat boundary generation for safe fresh follow-up recovery after restart without an optional sidecar; actual input still owns replacement CAS. Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable -->
Signed-off-by: Thomas Kosiewski <tk@coder.com> --- _Generated with `xum` • Model: `unavailable` • Thinking: `unavailable` • Cost: `$unavailable`_ <!-- mux-attribution: model=unavailable thinking=unavailable costs=unavailable --> Change-Id: Ib96dfe46e536ed66daacef4347f6aca478a9102b
Change-Id: I654ec5bc5ef9b25c54bc9d294e1db0714ed0713f Signed-off-by: Thomas Kosiewski <tk@coder.com>
9c3904c to
4454aeb
Compare
|
Addressed both current findings:
The reported races plus the foreign-supersession notification race failed before the fix. All 765 affected tests / 3,058 assertions, canonical static and independent review pass afterward. Final phase integration on main Generated with |
|
@codex review Please review the current head after the complete phase was composed on current main. All known findings have fixes and replies in their owning layers; the eight-member phase stays held until all members are approved and green. Generated with |
1 participant
After an ordinary Stop settles, fresh automatic monitor or family input can start a new turn without reopening the canceled continuation. This layer restores that existing behavior above #4191's V1 deferral, using V2 settlement proof.
V2 replacement authority waits for the captured producer and exact outer workspace cleanup: monitor retirement, auto-retry/withdrawn work, descendants and queued-input handling. Unresolved foreign V1 without a summary blocks automatic admission. Existing scoped-V1 ordinary preserve-mode admission remains supported; this is not a universal ban on automatic input behind every scoped record.
A failed physical Stop returns its error promptly while supervised completion can observe that exact producer finishing naturally. Retryable monitor retirement keeps its original obligation and completion receipt. Initial cancellation I/O failure does not erase physical completion; an exact cleanup retry can later qualify V2. Supersession and disposal release the owned wait without affecting a newer Stop. Failed descendant cleanup grants no V2 proof; existing logged/API behavior remains, with a later explicit Stop or manual replacement as recovery.
Fresh reset heartbeats stamp the captured compactionPublicationGeneration with the existing boundary occurrence ID. Matching eligible V2 provenance permits immediate or restarted follow-up recovery even when the optional sidecar is absent or failed to write. Legacy unmarked/stale generations grant no permission. The actual follow-up user row still owns original nonce/generation CAS and replacement retirement; a summary is not a replacement witness.
Automatic batches retain ordinary rollback accounting. Real V2 acceptance becomes irrevocable at durable trigger publication, before fallible callbacks; observer errors cannot refund persisted rows.
Validation: independent source/delta review passes. The combined candidate passed 1,077 tests / 4,472 assertions across 11 suites. Its final cleanup-handler and test-typing delta passed 39 focused cases / 169 assertions, and canonical static checks passed on that reviewed candidate. Risk: ownership/cleanup mistakes can resume canceled work or strand legitimate wakes, so held-cleanup, cross-instance, restart, failed-I/O and shutdown cases are included.
Successful physical Stop also waits for the originally captured startup producer before granting V2 authority. Fast and deferred settlement share a notification guarded by an exact V2 publication receipt, successful durable completion and current nonce/generation. This wakes monitor output deferred during cleanup without notifying for a superseded or failed Stop. The reported startup/monitor races and a foreign-supersession notification race were reproduced before the fix; 765 affected tests / 3,058 assertions and canonical static checks pass afterward.
Final phase integration on main
ad8a01b: independent composition review and full canonical static checks pass on tree3ed04896. Across 35 affected files, 2,549 tests / 28,590 assertions pass: 34 files passed before a test-only lifecycle repair, then all 28 pinned-budget tests passed after it. Two final-flush fixtures now complete their fake stream on Stop instead of waiting for later disposal; their original assertions and production behavior are unchanged.The complete cancellation phase is ordered #4214 → #4215 → #4219 → #4221 → #4182 → #4187 → #4191 → #4209. The reader prerequisites and runtime changes merge together only after every member has current-head approval and green CI.
Generated with
xum• Model:unavailable• Thinking:unavailable• Cost:$unavailable