Reproduce the registration races and the lock this package can actually hit - #35
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. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3c4363243d
ℹ️ 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: 9a9bfe5c5b
ℹ️ 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".
| first = self.daemon.tick(now=self.clock.now()) | ||
| self.assertFalse(first.quiet, "the shared daemon went quiet when one project paused") |
There was a problem hiding this comment.
Verify B progresses before archiving A
While A is paused, first.quiet == false only proves that some daemon pass did work; observation or reconciliation can make the tick non-quiet even if B receives no delivery. The test archives A before inspecting B, so the subsequent drain proves only the archived case and a regression where pausing one relationship starves another can still pass. Assert B's sends or delivery state immediately after this tick.
Useful? React with 👍 / 👎.
| and never opens the store, so the crossing shows the supervisor preserving records across | ||
| replacements. Whether a replacement worker reads them back is a separate question, | ||
| answered by a real process in `test_operational_scale.py` and reported separately. |
There was a problem hiding this comment.
Stop claiming the real worker reads every preserved record
The real-process case only demonstrates that the worker reads the relationship and the newly queued second delivery: its assertions inspect tick notes, a poll observation, and that queued delivery's state. Nothing makes that worker consume or validate the original acknowledgement or sync-outbox row, while the four-hour crossing uses FakeWorker instances that never open the store and merely checks those rows remained unchanged. Therefore the statement that a replacement reads “them” back overstates the evidence; either exercise the acknowledgement/outbox through the real worker or narrow the claim.
Useful? React with 👍 / 👎.
What this adds
Regression coverage for management-registration contention and process-failure recovery in
packages/codex-session-relay: 29 new cases across five modules, plusdocs/contention-regression.md, which records what each required scenario is proven by and separates evidence reused from existing tests from evidence added here.No source change. One finding that would have needed one is reported in the map instead.
A premise the issue got wrong, verified
The issue expected the management-marker and completion-hook landings to have moved this package.
git log ff3c69b5^1..6f77ab4 -- packages/codex-session-relayis empty: the first changed six files underskills/, the second changedscripts/anddocs/runtime-install.md. The contention those landings introduce is in what now calls this package — a coordinator that publishes marker facts and can stop between any two of them, and a Stop hook that runsguard-evaluateas a separate process against a store the daemon is writing. That is what the new tests reproduce.Modules
test_registration_contention.pytest_failure_recovery.pystate_unreadablerather thanreceipt_missing, and the same check deciding on the receipt once the holder lets go. The hold bound releasing into the next generation while the rolling window still counts what the last one spent.test_multi_parent_isolation.pytest_operational_scale.pyRetryPolicy.test_regression_map.pySeparating the evidence
Reused evidence is named by class in the map rather than rewritten. Criterion 4 (needs_changes into the next generation; duplicate, out-of-order and late events) adds no test at all:
test_anchor_binding.py,test_supersession.pyandtest_receipts.pyalready cover it through real entry points, and a fourth version would be volume rather than coverage.Clock provenance is a column because most of this package moves time by hand and some of it does not. The rule is per module and deliberately coarse: a module spends real time if it reads a real clock or starts a process, and a criterion naming such a module is mixed rather than injected.
test_regression_map.pyderives that set with anastscan and fails when the map disagrees — it caught two of my own labels being wrong.The four-hour work is deliberately two proofs: the scripted-clock crossing shows the supervisor preserving the handoff across nine replacements, and a real worker in a real process for one short segment shows a replacement reading it back. A
FakeWorkernever opens the store, so the first cannot answer the second, and the map says so.Scale, stated as a bound
6 parents, 12 events each, 72 events, declared as module constants so a report quotes a number read from source. It drains inside a ceiling derived from the policy, no tick exceeds the per-tick or per-parent send ceiling, the backlog only shrinks, and no parent is drained before another is reached. That is a measurement of this harness on one machine with a clock that never sleeps. It is not a throughput figure and it is not support for unbounded parallel operation.
Reported, not fixed
guard.SQLITE_TIMEOUTdocuments the lock wait the readiness check may spend and nothing passes it anywhere. Its name occurs once in the repository, its own definition;intent.read_only_connectioncarries a separate literal of the same value, andguard.lookup_receiptcalls that function without a timeout. The wait is real — an exclusive writer makes the read raise after a measured 2.00s — but the number producing it is the literal, so changing the constant would change nothing. Wiring it would also moveintent.dispatch_generation_state, which this change has no authority over, sotest_failure_recovery.pypins both halves and leaves the decision.The workflow-restore section is the only non-essential block in the revision direction of
report.render_revision, so a tight budget removes it first. CRW-94 owns that.Validation
CRW_PACKAGES_TMPDIR=/var/tmp python3 scripts/ci/packages.py— passed, codex-session-relay 1235 tests, up from a 1206 baseline ondev, no empty collection and no skipped case.python3 scripts/ci/validate.py— exit 0.python3 -m unittest discover -s scripts/ci/tests— exit 0.git diff --check— clean.Why
scripts/crw_runtime/components.jsonis in this diffcomponents.jsonrecordssubdirectoryTreeas the git tree hash of all ofpackages/codex-session-relay, so adding files undertests/anddocs/invalidates it even though no source changed.scripts/ci/contracts.pypasses ondevand failed on this branch for that reason alone.Re-derived through the same path
verify-definitionchecks —crw_runtime.definition.git(["rev-parse", "HEAD:" + subdirectory])— rather than typed by hand, and the rewrite refuses if the recorded value is not unique in the file. Exactly one line moved:packageTreeandsourceDigestare unchanged, which is the check that no source was touched: both coversrc/codex_session_relayonly.git diff --name-only origin/dev...HEADconfirms no file undersrc/is in this diff. The bridge component's three fields are unchanged too.python3 scripts/ci/contracts.pynow exits 0 with"findings": [].