test(advisory): accept peer-won Codex stop ingest - #1800
Merged
ScriptedAlchemy merged 17 commits intoSep 19, 2026
Merged
Conversation
`map_execution_error` mapped `SessionTemporalExecutionError::Empty` to
`CompleteZero` whatever freshness rode with it. After a hook ingest
commits rows, the project store publishes the temporal generation but
its relation receipt is applied later by the LCM summary convergence
page, so `root_readiness` answers `Partial { generation_lag: 1 }` and
the candidate cohort is empty. The first `tracedecay_message_search`
then reported `outcome: complete_zero` — "nothing exists, and that is
final" — for a transcript the store had already committed.
`map_report` already refuses that for an empty ranked page: a `Partial`
generation returns the typed partial outcome instead. The
execution-error path owed the same refusal. It now answers
`Partial { items: [], omitted: generation_lag }`, which the retained
message-search surface already renders as `outcome: "partial"`, so a
caller re-reads rather than believing the zero.
`production_codex_hook_ingest_survives_message_search_reopen` loops on
that typed partial the way the refresh assertions loop on `running`,
and still fails on an empty `complete_zero` or any other outcome.
Measured on `taskset -c 0,1`, 6 concurrent copies, 42 runs:
25 failures before, 0 after.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`tracedecay update` installed v0.1.0-beta.47, restarted the daemon, then refused the binary it had just installed: protocol identity mismatch (name=tracedecay, version=0.1.0-beta.47+84598a0b..., expected version=0.1.0-beta.47) and the daemon logged `daemon_version_skew` on every readiness poll, with the same two strings. Both sides were one binary. One value drove both symptoms. The GitHub-release upgrade path reports the release it installed as the bare tag (`upgrade.rs` `run_versioned_upgrade`), while the package-manager path reports the installed binary's own `--version`. That value becomes the maintenance window's `expected_version` (`service.rs` `adopt_maintenance_outcome`), which readiness compared to the daemon's advertised `build_version()` with a raw string `==`, and which `query_daemon_identity_stream` also sends as the probe's own `client_version`, where `client_version_skew` compared it with a second raw `==`. Build metadata never changes SemVer precedence, so a side reporting only the release is less specific, not different. `versions_name_same_build` is now the one comparison every identity check runs: equal release precedence, and the same commit whenever both sides name one. That keeps the skew 367a44a added this comparison to catch, two checkout builds of one release differing only by commit, while the release tag and the binary it ships resolve to one identity. Tests cover both halves and fail without the fix: readiness classifies 0.1.0-beta.47+<sha> as Ready against a bare 0.1.0-beta.47, a stale release and a second commit of the same release stay mismatches, and unparseable versions still compare literally. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A daemon spent eleven hours emitting 70,985 identical warnings:
session temporal refresh pass will retry class=Storage
retry_attempt=25107 error="Storage { operation: \"persist session;
refresh progress\", source: Sqlite { code: Some;
( 19 );
, extended_code:;
Some;
( 1811 );
, message: \"invalid session refresh progress\" } }"
Three workers each resubmitted the same progress row about once a
second for the whole process lifetime, pinning two tokio workers.
Two independent defects combined:
1. The worker used `SessionStoreError::is_storage` as its retryability
predicate. That only says the failure came from the storage adapter.
A `SQLITE_CONSTRAINT` abort is a schema-contract trigger refusing
this exact row, and an exact-SQL materialization ceiling (6,627 more
warnings on the same loop, reported as an untyped `Runtime` message)
refuses this exact statement. Replaying either unchanged can only
spin at the backoff cap, which for `Storage` is 800ms forever.
2. Even classified terminal, the projection arm only counted the error
and left the operation `running`, so the next pass rediscovered it
and rebuilt the same row. Only the projector's terminal errors
durably failed a refresh.
`Error::is_deterministic_refusal` now names the two engine failures
that cannot be replayed, `QueryLimitExceeded` maps to the typed
`InvalidOperation` its sibling limit refusal already used, and the
worker retires a refused refresh through the existing terminal-attempt
and discovery-suppression machinery.
`validate_successor` also admitted an equal committed frontier where
the durable guard requires a strict advance, so a stalled successor
reached the trigger as a constraint abort instead of typed state. It
now matches the guard.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A daemon mounted /fast/projects/tracedecay at 15:25:35, decided the
sealed generation predated the build and had to be rebuilt, and then
recorded no further scheduler progress for eleven hours. A full thread
dump taken while it was stuck shows two blocking-pool threads in
registry::mount_worktree_inner
-> LatestCodeTextGenerationV1::advance_text_serving_inner
-> CodeLexicalCloneSuccessorV1::verify_resumed_page
-> verify_clone_fingerprint_page_rows
-> sqlite3_step / sqlite3BtreeNext / readDbPage
so the mount was not blocked on a seat, a permit or a store lock: it was
running SQL, which is why the process also sat at 100% on two workers.
`clone_exact_postings` and `clone_fingerprint_postings` are WITHOUT
ROWID tables whose primary keys start at `class` and `language`, so
`WHERE symbol_occurrence_id = ?` has no index and scans the whole table.
Resume verification issued two of those per clone body, making the pass
quadratic in a repository's postings.
Both tables are now read once per page and bucketed by occurrence. Each
primary key is unique within one occurrence, so a sorted bucket
reproduces the `ORDER BY` the per-body queries used and the comparisons
stay byte-identical; `clone_body_payloads` and `clone_occurrences` are
already keyed by their lookup column and are untouched. Resume
verification is covered by
`v16_clone_payloads_are_content_addressed_and_postings_page`, which
drops and reopens a successor mid-corpus and then compares finished
section digests.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
cc10394 made every non-retryable failure of the projection batch persist retire the running refresh. A cancelled worker control also fails that persist, and it is not a refusal: the row was never submitted, and the next pass may hold a live control. Retiring it through `claim_terminal_attempt` under a cancelled state claimed nothing and counted nothing, so `cancelled_worker_control_prevents_projection_batch_persistence` observed zero terminal errors on CI. Only a deterministic engine refusal (constraint abort, exact-SQL ceiling) retires the refresh; every other terminal error counts as before and leaves the operation running. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
8b75523 cut resume verification from one postings scan per body to one per page, but a restart still replays every committed page, so a daemon resumed against a large repository sat inside `verify_clone_page_rows` for over thirty minutes with no scheduler progress: N pages times a full scan of both postings tables. The clone successor now installs `symbol_occurrence_id` indexes on both postings tables when it opens (idempotent, so a copied prior built without them gains them), and page verification reads postings per occurrence through them. Nothing digests or enumerates the index schema; the receipt records the file size after the indexes exist. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
`single_analytics_append_commits_in_one_writer_dispatch` asserted the append future was still `Poll::Pending` after its first poll. That poll enqueues the INSERT on the writer OS thread via a synchronous `try_send` and then awaits a `bounded(1)` reply channel, so the writer can execute, autocommit and answer before the caller first polls the receiver. The assertion was racing that thread, not checking a contract: 9/60 failures under `taskset -c 0,1` with 6 concurrent copies, matching the CI flake in run 35422336760 (TRY 1 fail, TRY 2 pass). The contract commit 4fc1439 added is that a *detached* append still commits: the writer runs the request and only then replies, ignoring a dropped receiver. Make the test enforce that with a gate it owns -- scope the future so it is dropped right after the single poll, before anyone reads the reply -- and keep the existing commit and no-retained-transaction checks. Restoring the transaction-framed append fails the restructured test 30/30 with "single append did not autocommit", so the regression guard is intact and now deterministic. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit ea8113159372c2f4ab1f7c1e8138ce4c5d846494) (cherry picked from commit 80341240b4e94c5fbe075d024337c87b256cc3f3)
`mounted_code_generation_retention_continues_capped_segment_reclamation`
failed both nextest tries at ~3.9 s on master run 35422072661 with `code
generation retention plan: Storage("No such file or directory (os error
2)")`; the identical tree passed the same partition minutes earlier on run
35420910183, so it is a race. Under `taskset -c 0,1` with 6 concurrent
copies it reproduced 4/78.
Instrumenting every enumerate-then-open loop in the planner with the path
and a re-stat named the vanishing file on both ENOENT reproductions: the
text-artifact inventory's `read_dir` + `symlink_metadata` loop, on
`.text-artifact-<digest>.staging` and on a `.staging-journal` sidecar,
both `present_now=false` while their siblings were still listed.
`prepare_next_code_generation_retention_cancellable` scans without the
generation-store lock on purpose - the full-digest read routinely covers
several GiB and must not pin the daemon writer gate - and the text-artifact
builder retires a whole `.staging` family (`discard_incompatible_staging`
-> `retire_text_artifact_staging_family`) under that lock in the background
pass tail, the same tail fenced in 656b532 and 760e221dda. So an entry
the listing just named can be gone before the scan stats it, and the whole
plan failed. Production reads that as `retention_plan_failed` and fails the
pass with a loud degraded log, so every publish that raced a maintenance
tick lost the tick.
An entry that vanished mid-scan is already reclaimed, which is what the
inventory would have planned for it anyway: skip it, the way this loop
already skips an absent active-staging path fifteen lines above. Completed
artifacts the durable index references are verified before this scan and
stay fail-closed. The unit test retires the staging family from inside the
scan's own cancellation probe and asserts the plan survives without naming
the vanished files; it fails with the ENOENT before this change.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 012965c52514790e56d5531e51b48acb808cd1c5)
The same 6-copy `taskset -c 0,1` reproduction of `mounted_code_generation_retention_continues_capped_segment_reclamation` also failed with `code generation retention plan: GenerationStoreBusy` (1 of 4 failures in 78 runs). That is not a failure. The planner's recovery probes the generation-store lock with `try_acquire_code_generation_store_lock` and answers `GenerationStoreBusy` when a writer owns the store; production maintenance consumes it with `defer_generation_store_busy` and comes back on the next tick. The route under test stays mounted, and `publish_code_edit` returns as soon as the serving generation id changes, so the pass tail that sealed that generation can still own the store when the test plans - the same pass-tail exposure as 760e221dda. Consume the typed busy answer under a bounded wait instead of reading it as fatal. Every assertion on the resulting plan is unchanged, and any other error still panics. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 833a3f30a2c77c426864123c0065bc4d8b664586)
`tracedecay_unsafe_patterns` picked `enclosing` as the smallest-line-span
symbol covering the match's line. When several declarations share one line
every candidate has span 1, so the winner was whichever symbol the page
happened to yield first, and pages arrive in occurrence order over
per-project digests. `#[test] fn a() { x.unwrap(); } pub fn b() { panic!(); }`
therefore reported `@test` (the annotation-usage node the Rust extractor
names `::@name`), `a`, or `b` at random: 7 of 12 identical local runs
disagreed, which is what broke the CI job deterministically on its runner.
Resolve the enclosing declaration by the byte range the graph already
publishes in `CodeGraphSymbolBindingV1::source_span`, containing the offset
of the matched construct. That is stable, names the innermost declaration
that really contains the site, and drops attributes for free: `#[test]`
spans only its own bytes, so it can never contain a call.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit 178afe414b370165fd8910620085e922d7e9fe27)
`adapter_leaves_repository_byte_identical` snapshots `.git` before and after the read-only adapter calls. The fixture's `git commit` spawns a detached `git maintenance run --auto`, whose `.git/objects/maintenance.lock` was still present for the first walk and gone for the second, so the byte-identical assertion failed on both nextest tries with the lock as the only difference. Disable auto maintenance and gc for every fixture git call, as the scheduler and daemon fixtures already do. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The packaged host advisory journey required the Codex stop ingest to report committed. A project catch-up sweep that admits the rollout first reports exact_duplicate, which is the durable replay, and the assertion panicked. Accept both terminals, reject accepted_for_replay, and ride out the same typed warming or deferred progress the Cursor ingest already does. Co-authored-by: Zack Jackson <ScriptedAlchemy@users.noreply.github.com>
|
An explicitly published branch worktree is never a project-open route, so nothing mounted a core query authority on it. An exact branch read could only borrow one already mounted on a peer checkout of the same repository (`mount_query_authority_from_project_peer`), and that peer's own mount is deferred until it seats a text generation. A read taken right after the publication sealed its provenance therefore failed closed with a non-retryable `authority_unavailable`, even though the branch generation was committed and servable. That is the CI flake in the tracedecay-cli `core_cli_suite` linked-worktree branch-add journey, where a freshly added peer worktree searches the just-published branch before it has seated anything of its own. Mount the published branch's own authority from the project's durable cursor-key authority, reusing the existing project-open mount rather than adding a second mount path. It runs inside the daemon-owned publication task after the generation is committed, so the admitting `branch add` caller — which returns at admission, before activation even starts — never waits for it and never pays for the profile session-registry lock. An earlier attempt (51402cd) mounted on the admitting caller's path and correlated with mount and publish failures elsewhere; this one stays off that critical section. Best effort by design: the generation is already committed, so a missing session mount or cursor key must not retract it, and the exact read still falls back to borrowing a peer authority when the mount could not run. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 6d887a0c3ce25c2838f2943200e133a610cdf2e8)
`tracedecay_field_sites` picked each site's `enclosing` declaration by filtering the file's symbols to those whose line range covers the site's line and taking the smallest `line_span`. When several declarations share one line every candidate has span 1, so `min_by_key` broke the tie on the order the graph page yielded symbols, and pages arrive in occurrence order over per-project digests: a coin flip per run, the same defect 63c0784 fixed in `tracedecay_unsafe_patterns`. Resolve `enclosing` by the byte range the graph already publishes in `CodeGraphSymbolBindingV1::source_span` containing the site's own byte offset, innermost containing span winning. `FieldSite` already carried that offset (`byte`, the end of the field name, keyed the same way as the receiver-type map), and masking preserves byte layout, so no matcher change was needed. `enclosing_declaration` moves from `unsafe_patterns` up to the shared `analysis` module and now returns the symbol rather than its name, since the field-site qualifier check needs the occurrence id. Both handlers then apply one attribution rule instead of two copies that can drift. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> (cherry picked from commit 71522df546dae3b7cff729d60988b2f7bb13b750)
`elapsed_freshness_window_alone_does_not_make_dashboard_state_stale` read the dashboard right after `wait_for_dashboard_ready`, which joins only the running pass. The mount leaves clone backfill behind, and the wakes that drain it leave a banked permit whose no-op pass projects `Verifying` instead of `Fresh` at the sample (CI run 35425541839, both tries). Use the single-permit registry, settle the mount-era chain, hold the admission so no pass can start, and prove the pending-wake slot stays empty, as the text-progress test already does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Under `--all-features` the binary binds the hotpath metrics port on start. When a sibling test's daemon already holds it, the bind error lands on stderr and `shipped_binary_stops_quietly_when_a_pipeline_reader_exits` fails on its empty-stderr assertion. Turn the metrics server off for this command, as `hotpath_command` already does. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
ScriptedAlchemy
marked this pull request as ready for review
September 19, 2026 06:48
Owner
Author
|
Pairs with #1794 (advisory |
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. |
![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: ba835fc173
ℹ️ 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".
| "--json", | ||
| ]) | ||
| .current_dir(&project) | ||
| .output() |
There was a problem hiding this comment.
When a tracedecay tool invocation stalls or internally retries, this blocking .output() can run until the CLI's separate two-minute default deadline; moreover, a terminal response then breaks before stop_deadline is checked. Consequently, the advertised 60-second bound does not actually bound this new retry window and can substantially extend a stuck CI run. Apply the remaining deadline to each child invocation rather than checking it only afterward.
AGENTS.md reference: AGENTS.md:L177-L178
Useful? React with 👍 / 👎.
Owner
Author
|
Cancelled CI run 35427625063: this fixes the same defect as f2ab808 + 8b75523 (clone-postings occurrence indexes, per-page resume reads) on |
Owner
Author
|
Correction to my previous comment: this PR is not a duplicate of the clone-postings fix; that text was meant for #1824/#1830. I cancelled the run only to keep the shared runner queue (2 concurrent, ~20 deep) moving while #1797 waits; this PR gets its own review agent and a fresh dispatch afterwards. Apologies for the noise. |
8 tasks
…or/advisory-codex-stop-durable-b050
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
exact_duplicate, the same durable terminal the Cursor ingest already treats as proof.accepted_for_replaystill fails the journey..github/workflows/ci.ymlis unchanged. Nopull_requesttrigger was added. test(ci): close three master-tip readiness races #1794 was not absorbed: its three races (HTTPafter_delay, ignored-dependency publication, process-group socket close) are a different failure set, and master already waits out a predecessor test daemon.Motivation
packaged_host_ingest_delivers_a_registered_advisory_cyclerequired the Codex stop ingest to reportcommittedon the first answer. The project catch-up sweep and the hook route both admit the same rollout. When the sweep wins, the hook persists nothing and reportsexact_duplicate. That is a durable replay, not a missing commit, and the assertion panicked. The Cursor half of this journey andproduction_codex_hook_ingest_survives_message_search_reopenalready accept both terminals.Changes
crates/tracedecay/tests/runtime_acceptance_suite/advisory_runtime_acceptance.rs: the stop ingest loop now acceptscommittedorexact_duplicate, retries a retryable incomplete admission, and retries a daemon that is still warming.Test plan
packaged_host_ingest_delivers_a_registered_advisory_cycle— 1/1 passed (runtime_acceptance_suite, exact filter, 2.88s)taskset -c 0,1— 4/4 passed, including two runs that took 32.70s and 32.44s and still finished inside the typed retry windowDispatch SHA:
ba835fc173654f3be17190f4a1c3a8685367b7a4Checklist
.envfiles includedCHANGELOG.mdupdated (test journey only; no user-facing change)