🤖 refactor: define inactive compaction cancellation state - #4140
Conversation
This comment has been minimized.
This comment has been minimized.
|
@codex review Please review this inactive state core's nonce/retry ownership, witnessed retirement, failed deletion debt and post-await identity checks. Filesystem adapter and complete runtime activation are separate prerequisites; no current Stop/recovery caller is switched here. Generated with |
This comment has been minimized.
This comment has been minimized.
![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: 52a5febaa0
ℹ️ 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".
52a5feb to
96cab2a
Compare
|
Addressed 3958384487: fallback publication now rechecks the captured mutation and pending promise after rejection resumes, preserving a newer Stop or retry. Addressed 3958384495: failed witnessed retirement for A no longer blocks narrowing a freshly read B. B's own failed writes remain blocking and retryable. Four deterministic regressions failed before the fix. All 36 focused core tests and final full static checks pass; independent review approved the 18 added/1 removed production lines. No filesystem behavior or runtime activation is added. @codex review Generated with |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 96cab2a8c9
ℹ️ 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".
|
Addressed finding 3958898439: read completions now track the newest accepted read generation as well as local mutation ownership. Once B is accepted, an older A cannot overwrite B, expose its stale error, or start fallback cancellation. A merely pending B does not hide A's valid cancellation or repair failure. All 48 focused behavioral tests pass, including reordered success/error completions, repair ownership, pending-successor controls, and replacement fallback races. Independent review approved this bounded change: 23 added and 5 removed production lines. @codex review Generated with |
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This comment has been minimized.
This comment has been minimized.
Wait for the warning's visible state after opening it so the assertion can tolerate the app entrance transition without accepting persistent invisibility. 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: Ieeb9c6b96af089dab27156d43a7f54fcb9b38dbe
Keep cancellation nonce and retry ownership across failed persistence. Model witnessed retirement and deletion debt without activating runtime behavior; a later adapter supplies shared-lock storage and acceptance witnesses. 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: Ic12b78c45520044eba01a6857ada8179c91a62d7
Recheck fallback ownership after read rejection and let foreign cancellation narrowing bypass obsolete witnessed deletion debt. 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: Ib8a1b50afce050febe46cbb9a110bc1686832d69
Reject stale completions after an accepted successor without treating an in-flight read as evidence that cancellation is absent. Preserve error, repair, and replacement fallback ownership across the same boundary. 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: Icd8b3f1eb6e009c8f843712a08a4ac4c75ecd836
|
@codex review Prioritizing this bottom layer for merge. Integrated the shared CI fixes: #4143 is merged and the validated one-line Storybook fix #4145 is queued immediately ahead. All three cancellation-core commits remain identical by git range-diff, including the reviewed accepted-read ordering fix. All 48 focused tests and full static checks pass on the combined tree. No new behavior or scope was added. The storage layer remains untouched until this parent lands. Generated with |
01b0ea1 to
65826f4
Compare
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65826f476d
ℹ️ 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 Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65826f476d
ℹ️ 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".
Storage supplies the exact committed record, including inherited retention, through a synchronous receipt before lock release or cleanup. Apply cache and read invalidation there so delayed settlement cannot resurrect retired state or discard a newer foreign cancellation. Preserve existing public mutation outcomes. All 60 core tests and full static checks pass. Regressions cover inherited retention, stale predecessor reads, both foreign-successor completion orders, and failed or superseded publication. Independent review approved. 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: Ie0a730120182d8ccce38ae4e2b3ae4ad74c85cb9
|
Addressed both findings:
All 60 core tests and full static checks pass; independent review approved. The production delta is 25 changed lines. Existing public mutation outcomes remain strings, and runtime activation remains separate. H2a is integrating this receipt contract while retaining its validated lease guards; both members of this phase will be queued together once ready. @codex review Generated with |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f374bf0296
ℹ️ 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".
Preserve unknown state after superseded reads and refresh once before replacement. Reject another overlap or unresolved persistence; unsupported storage records bypass fallback. Validation: 71 targeted tests, full static checks, and independent review pass. 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: I3581c075367f653fba39e6dedf8f62c350952652
|
Addressed 3960295475: a superseded read leaves state unknown, never evidence that the cancellation file is absent. Replacement performs one fresh read and propagates failure. A second overlap or outstanding persistence rejects conservatively instead of returning a tentative Stop. Unsupported or oversized storage records explicitly bypass replacement fallback. The bounded refresh race reproduced before the final guard. All 71 targeted tests and full static checks pass; independent review approved. The change is 26 production lines, with coverage for both overlap completion orders and persistent failures. No runtime activation was added. @codex review Generated with |
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0a367ba750
ℹ️ 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".
Return current cancellation state after replacement reads and invalidate earlier reads synchronously when repair commits. Preserve successor state and bounded refresh behavior. Both races reproduced before the fix. All 77 core tests, full static checks, and independent review pass. 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: I77e0089454adfee985ba8aaa72cf51c2a28f5100
|
Addressed both read-ownership findings:
Both reported races reproduced before the fix. All 77 core tests and full static checks pass; independent review approved. This round changes ten production lines using existing ownership counters. Generic refusal, bounded refresh, and the inactive scope remain intact. @codex review Generated with |
This comment has been minimized.
This comment has been minimized.
|
@codex review The current head is 4684034. Code and security status rows report completion, all review threads are resolved, and CI passes, but there is no explicit current-head code approval or PR-description thumbs-up. Please review this unchanged candidate and provide the normal approval or actionable findings. No code changes have been added since the 77-test/full-static validated read-ownership fix. Generated with |
|
Codex Review: Didn't find any major issues. What shall we delve into next? Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
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". |
This comment has been minimized.
This comment has been minimized.
1 participant
Introduce an inactive cancellation state core for the agent-loop refactor. It keeps cancellation nonce and retry ownership across failed persistence, distinguishes witnessed replacement from unfinished deletion, and prevents stale narrowing or retirement from superseding newer work.
Storage is an injected contract. A later adapter must enforce shared-lock nonce/frontier comparison, verify replacement witnesses, and repair malformed storage safely. No runtime caller uses this core yet; Stop and recovery behavior are unchanged by this PR.
This independent H1 prerequisite replaces part of closed #4121. It does not depend on the pending-attachment protocol, so it can be reviewed separately without unrelated ancestry.
Validation: 48 behavioral tests pass, including narrowing versus retirement, fallback read rejection versus newer Stop/retry, foreign narrowing after failed witnessed deletion, and reordered reads and repairs. A newer accepted read supersedes older results; a merely pending successor cannot hide valid cancellation or repair errors. Final full static checks and independent review pass. Nix formatting was skipped because Nix is unavailable. Diff: 318 production lines and 727 test lines in two new files.
Risk is limited while inactive. Injected-adapter tests prove state-machine invariants, not filesystem durability or cross-process compare-and-swap; the real adapter and complete runtime activation remain separate deliveries.
Generated with
xum• Model:unavailable• Thinking:unavailable• Cost:$unavailable