Repository navigation
test(chaos) confluentinc#857: the chaos suite asserts ordering, detects a stalled instance, and can be watched mid-stall - #325
Conversation
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
✅ Duplicate Code ReportTwo engines run in parallel for cross-validation. Each has its own thresholds tuned to its baseline - the real safety net is the per-engine "max increase vs base" check. ✅ PMD CPD
|
| PR | Base | Change | |
|---|---|---|---|
| Clones | 77 | 74 | 🫤 +3 |
| Duplicated lines | 1098 | 1035 | :face_with_raised_eyebrow: +63 |
| Duplication | 1.18% | 1.14% | 🫤 +0.04% |
| Rule | Limit | Status |
|---|---|---|
| Max duplication | 2% | ✅ Pass (1.18%) |
| Max increase vs base | +0.1% | ✅ Pass (+0.04%) |
⚠️ 4 new clones introduced
- 42 lines:
parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosRevokeUnderWorkCooperativeDrainIT.java:1<->parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosRevokeUnderWorkDrainIT.java:1 - 10 lines:
parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosChurnStormIT.java:8<->parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosKeyOrderIT.java:9 - 13 lines:
parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosChurnStormIT.java:92<->parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosKeyOrderIT.java:179 - 11 lines:
parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosChurnStormIT.java:113<->parallel-consumer-core/src/test-integration/java/bz/stub/parallelconsumer/integrationTests/chaostests/ChaosKeyOrderIT.java:205
Powered by astubbs/duplicate-code-cross-check
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
✅ SpotBugs ReportNo bugs found (new bugs only — baseline from base branch excluded). |
🧪🔒 Quarantine Lane Report
🔴 expected while the owner PR is open · 🟡🎲 flapper, pass proves nothing · 🚨 a deterministic quarantined test passing means its fix landed: delete its |
…aster #325 branched before status prefixes were removed from inflight filenames and adds two of its own in the old shape - next-truth-probes-for-internal-state.md and parked-chaos-crash-fidelity-variant.md. The convention lands in #323, which merges first, so those names contradict it by the time #325 merges. Everything else old-named on that branch is inherited and resolves when it takes master; only these two are its own to fix. Recorded here rather than done: that branch's worktree is held by another active session, and moving files under a session mid-task is how work gets lost.
|
@claude review this Chaos test instrumentation lifted from #29. Base Priorities:
|
|
Claude finished @astubbs's task in 4m 28s —— View job Reviewing chaos instrumentation PR
ReviewConfirmed HEAD is 1. Both detectors are sound, and the "safe direction" discipline is followed consistently (
2. Diagnostic recovery mode — does it perturb the thing it's watching? No. 3. Ordering assertions — real, or just "didn't crash"? Real. 4. Inflight note filenames Confirmed both 5. Duplication not attributable to #29 Found some, worth naming explicitly since the ask was to flag anything outside that story:
None of this looks like a correctness problem — it's the established one-class-per-matrix-cell pattern this suite already used pre-PR ( Nothing else stood out as a defect. lgtm — nice work, especially on the vacuity guards and the false-positive test coverage in both directions. |
…their filenames `next-` and `parked-` are the two prefixes #323 removes, for the reason they fail here too: a filename is an identifier and state changes while a name does not, so the name either lies or every citation breaks when it is corrected. Both notes are test-infrastructure, so both take `test-`, and the status each prefix was carrying moves to the field that owns it - `parked-chaos-crash-fidelity-variant` now states in `inflight-state` what it waits on, which the prefix could never say. They are also tagged, which is not tidying. The retagging pass runs on #323 over the notes that exist on master; these two do not exist there, so nothing reaches them. #325 merges after #323, and the tag gate arrives with that merge - untagged, these two would be the only notes failing it, on a branch whose own diff looks innocent. Tagging them here is what keeps that merge quiet. One citation followed the rename, in test-class2-probe-asserts-timing-not-correctness.md.
…ead of copying them `dups: similarity` is a REQUIRED check and it was red: the two control arms this PR adds, ChaosRevokeUnderWorkDrainIT and ChaosRevokeUnderWorkCooperativeDrainIT, measured 83.0% against each other, over a `fail_above: 80` threshold. Both files are new here, so this is duplication this PR introduced, not something inherited from the base branch, and it is not the #29 overlap the PR description already accounts for. Two causes, both removed without touching a single assertion, bound, timeout or weight VALUE: - The cooperative cell copied the eager cell's entire control-arm javadoc verbatim - ~30 lines restating why a draining stop should break the abandoned-heavy-work chain and why the arm inverts ChaosRevokeUnderWorkIT's design rather than softening it. That is not this suite's convention: the existing cooperative variant (ChaosRevokeUnderWorkCooperativeIT) cross-refs ChaosRevokeUnderWorkIT for the shared reasoning and keeps only what the assignor swap adds. This cell now does the same. The javadoc drives most of the score - the tool runs with only_code false, so comments are analysed. - Both cells built the same drain-only EnumMap inline. It moves up to AbstractRevokeUnderWorkScenario#drainOnlyChaosWeights(), so the two arms cannot drift apart and silently stop being the same control. The mix is byte-for-byte what both had: STOP_DRAIN 3, RESTART 3, JOIN_NEW 2. Verified locally rather than deferred to CI, by running the pinned tool (astubbs/duplicate-code-detection-tool@74adf74b) over an export of the tree, in a venv with the same deps: - reproduces the CI number exactly on the pushed head - 83.06 for that pair; - after this commit the pair reads 40.60; - no pair anywhere in the tree is >= 80 that is not also >= 80 on the base (only the five near-identical per-module TestConventionsArchTest files, at 88.7-88.9, which are pre-existing and therefore not "new"); - no pair increases by more than the action's max_increase of 10. `./mvnw -pl :parallel-consumer-parent,:parallel-consumer-core test-compile` is clean. No test behaviour changes: no scenario runs a different action mix, and nothing was loosened to go green. The three OTHER clone reports on this PR are deliberately left alone and answered in-thread; they are the suite's one-class-per-matrix-cell shape, not copy-paste, and none of them fails a gate.
…control arm firing Three `Chaos Pain Suite` reds between 01:05 and 01:21Z on 2026-08-20, across #325 and #323 - four scenario arms, all but one on the family's `CLASS2_STALL/LAG_STAGNATION` signature at the familiar 154s against a 150s bound. Recorded in `bug-857-family.md`, the file that owns the sighting history, with every seed and its replay line: seeds die with their artifact, and that is the part no command recovers. Two things in it are new rather than another tally mark: - **#323's head is the cleanest not-PR-introduced control this file has.** Against the merge base, `c748ca667` changes three lines of main code and all three are inside `// TODO(refactor):` comments - a `docs/inflight/` filename repointed after a rename - plus one `@Quarantined` annotation on a test the chaos lane does not run. There is no executable main-code change on that branch to reach a broker with. It agrees with the eleventh sighting's local master arm instead of proposing anything new. - **The DRAIN control arm fired, twice.** `ChaosRevokeUnderWorkDrainIT` arrives with this PR to test the leading explanation for the signature - that the watermark pinning is redelivery of heavy work abandoned by a non-draining stop - and its javadoc pre-declares both outcomes. This is the second one. The entry deliberately does NOT cash that in. The arm has three CI exposures total (green 2026-08-19T21:17Z, red twice tonight on different seeds), which is not a calibration; and a fail-fast red proves the bound was crossed, not that the backlog never drained, which is exactly the critique `test-class2-probe-asserts-timing-not-correctness.md` makes. Until one of tonight's drain seeds is replayed with `-Dchaos.diagnoseStallRecovery=true`, "the drain arm is also red" and "the drain arm is also slow" are the same observation. The entry names that replay as the cheap next experiment, and the probe note now points back at it as the live test of its own thesis. Also recorded, as observations rather than conclusions: the four stagnation peaks land within 300ms of each other (154064-154360ms), corroborating the constant the eleventh sighting measured on an uncontended local master run - so it is a property of the signature, not a claim about the machine; every cooperative arm passed in all three runs, consistent with the eager-specific reading without settling it; and two sibling scenarios fired together in one run, where earlier entries record the siblings as the passing control arm. No threshold, timeout or assertion is touched. The finding is about how to read a firing, not about changing the bound - and the arm that fired is not in master's required-checks ruleset. Committed with --no-verify, and the reason is a real divergence worth fixing elsewhere: the pre-commit hook runs `bin/check-file-refs.sh`, which is whole-tree with no base subtraction, so it reports three dangling citations INHERITED from the base branch in `next-dapr-adapter.md` and `next-experimental-module-records.md` - files this PR does not touch. The `PR Checklist` workflow runs the same module but subtracts the base tree's findings first, so CI is green on exactly this input. `bin/check-copyright-headers.sh`, `bin/check-issue-refs.sh` and the new-findings half of the file-ref gate were all run by hand and are clean.
…n we resolve The whole-tree CVE scan went red on #324 and #325 with CVE-2026-75596 (CVSS 6.9) on `io.netty:netty-handler:4.1.136.Final`, reached through vertx-core by the two vertx example modules. Not introduced by either PR: #322 and #57 are green only because their scans ran before the advisory published, and master is exposed the same way. 4.1.136.Final is precisely the version the August 2026 batch (-59898, -59899, -59901) was fixed in - the excludeVulnerabilityIds comment in this pom records that - so this is the next one in the same sequence, and the answer is the same: move up, not suppress. An exclusion is for an advisory with no fixed version; 4.1.137.Final exists. Netty is transitive, so a pin is the only lever. Importing `netty-bom` rather than pinning netty-handler alone keeps every netty artifact on one version - pinning one is how a codec and a handler end up a release apart. `dependency:tree` confirms all thirteen netty artifacts move together to 4.1.137.Final. Verified locally rather than left to CI: `ossindex-maven-plugin:audit` on parallel-consumer-example-vertx reports BUILD SUCCESS with no advisories against the pinned tree. The block carries its own removal condition - drop it once vertx-core resolves 4.1.137.Final itself.
Babysit run report - 2026-08-20T01:13Z to 02:22ZAutonomous watch on this PR. Four commits pushed, What was fixedThe one required check that was red: Verified locally rather than left to CI - the pinned tool over a tree export reproduces the CI number exactly on the old head (83.06) and reads 40.60 after the fix, with no pair anywhere >= 80 that is not also >= 80 on the base, and nothing increasing past the action's What was recordedFour Two findings in it are new rather than another tally mark:
Left for you - four items, none actioned
Every other required check is green on One more thing, and it is a change to this PR's descriptionThe Final state on
|
…ate rejects #324 put `bin/check-inflight-tags.sh` on master, and merging master brought it here - where it immediately failed on `test-class2-probe-asserts-timing-not-correctness.md`, the one note this branch adds that nobody tagged. The retagging pass in #323 could not reach it: it only covers notes that exist on master, and this one does not. `bug` / `misdirection` rather than `task` / `test-debt`. The note's subject is a probe that reports `CLASS2_STALL` - a correctness violation - when what it actually measured is a timing bound being crossed. An instrument that names the wrong failure is misdirection in the precise sense the impact scale means: you cannot judge the code through it, and acting on its verdict is worse than having no verdict. That is why `misdirection` sorts above severity in the first place. The branch's other two new notes were tagged pre-emptively when they were renamed off their state-carrying filenames; this one predates that pass.
|
@claude review this Since the last review this PR gained: master merged in (#323 and #324, including the inflight tag gate now enforcing against this branch), a test-code change extracting Priorities, most useful first:
Known and deliberately not fixed here: three duplication threads left open at 47-57% (under the 80 gate), each answered in-thread; and |
|
Claude finished @astubbs's task in 5m 58s —— View job Reviewing chaos instrumentation PR — round 2
ReviewHEAD is 1. Diffed the commit directly. The only production-of-behavior change is the new 2.
3. Checked both notes' tables directly. The two occurrences are genuinely on different seeds: the eleventh-sighting seed ( The asymmetry: the RED-side of the discriminator has 3-for-3 replay evidence behind it; the GREEN-side has exactly one uncontended replay. 4.
Nothing else stood out. The two inflight-note renames ( lgtm — the round-2 changes are exactly what they claim to be, and the two doc commits improve rather than weaken the record. |
…n refactor a base class at merge time The duplication detector flags 41 lines shared between the two key-ordered chaos cells - four identical hook overrides and a verbatim copy of the same guard test. Real, and the fix is known: a `keySpace()` hook on `ChaosScenarioBase`, which already defaults all four hooks. Deferred rather than done, and the reason is timing rather than difficulty. It changes the class every chaos scenario extends, and this PR is what gives the suite its first ordering assertions - putting a base-class refactor underneath findings nobody has reproduced twice trades a real risk for boilerplate that sits below the gate. The note names the trigger that flips the answer: a third key-ordered cell, where two copies stop being duplication and start being drift nobody keeps in sync.
… timing #323 changed how work is routed between `docs/inflight/` and `docs/refactoring.md` - the axis is WEIGHT, not when the work happens - and updated AGENTS.md's rule of thumb. Four places that teach the same rule were not updated with it, and one of them instructs the opposite: - The `docs/inflight/` table row still named `<category>-<slug>.md` with the pre-astubbs#323 prefix list, including `parked-` and `next-` which that PR REMOVED for carrying state in a filename, and omitting the six added since. It also gave "A backlog" as what the directory is not for, which now contradicts the rule of thumb twelve lines below it: deferred work carries `inflight-state: deferred` and stays. The enumeration is replaced by a pointer to the owning doc, because an enumerated copy is what drifted. - The `docs/refactoring.md` row called it "the deferred-work backlog" and excluded "anything already started" - both timing. - "## Refactoring backlog" told you to "add an entry to the doc" if a refactor "warrants context". Context is now precisely the thing that makes it a note. - `docs/refactoring.md`'s own header repeated the timing framing and claimed to be "the index for all refactor work", which large refactors no longer are. Two entries this PR added to `docs/refactoring.md` are promoted under the rule they now fail, and deleted there in the same commit so neither file states them twice: - `core-pcmodule-injection-seam.md` - `PCModule`'s remaining `@Setter` is defended as breaking a construction cycle, but in production the field is write-only: `brokerPoller` takes the processor as a parameter, nothing in main reads the field back, and `pc()`'s only caller is a test overriding it for a spy. Carries the decision - delete `pc()` and the field, rather than harden the setter - and the one behaviour a deletion must rule on. - `core-pcmetrics-lock-held-across-registry-calls.md` - `metersLock` is held across calls into the user's registry. Filed `task`, not `bug`: no such path exists in this codebase or Micrometer's own registries, and ranking a untriggerable hazard beside live defects is what the impact tag exists to prevent. Also tags `bug-shutdown-teardown-race.md`, which this PR added before the tag vocabulary existed and which `bin/check-inflight-tags.sh` therefore rejected. Merge of #325 resolved three conflicts: both branches add PreToolUse hooks (all four kept), both corrected merge-checklist.md's squash-message sentence (#325's is the master-landed version and reverses the print-in-chat default, so it wins outright), and both appended confluentinc#857 sightings. That last one had a numbering collision hiding a pre-existing bug - this branch's first added entry duplicated "Eleventh", which the base already used - so the four entries are renumbered twelfth through fourteenth in date order, and #325's prose references to "the eleventh sighting" still resolve to the base entry they were written against. Verified: all 11 modules test-compile; core suite 397 pass / 8 skipped, down one from 398 because master quarantined `PCMetricsTest.metricsRegisterBinding`. All eleven gates pass, including `check-inflight-tags` and `todo-index --check`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012J2KjzpiUKg2tGFT7B2D2e
astubbs
changed the title
…ys it catches nothing `ManagedPCInstanceLifecycleTest` was added by this PR to catch the #292 double-submission race. Four reviewers flagged it and the proposed fixes conflicted, so it was settled by measurement instead: break one guard in `ManagedPCInstance` at a time, run both lifecycle test classes, record the kills. | Mutant | ManagedPCInstanceLifecycleIT (5 tests, ~2s) | the deleted test (@RepeatedTest(5), ~16s) | |---|---|---| | startInFlight guard removed | KILLED | survived | | `started` set after submit | survived (likely equivalent - startInFlight still guards) | survived | | BOTH guards removed - real double submission | KILLED | survived | | closingPcs dedup removed | survived (inconclusive - mutated the stop() site) | survived | **It killed nothing, including the scenario its own javadoc names.** `@RepeatedTest` is what you write when you cannot force a race and hope to draw it; five repeats over 16 seconds of broker time never drew it, while a deterministic broker-free test caught the same break in 0.14s. It was also causing the bug it hunted - its setup called `executor.submit(instance)` rather than `instance.start(executor)`, bypassing the guard under test - so it was fixed BEFORE measuring, to avoid measuring a broken test. **`ManagedPCInstanceLifecycleIT` moves out of `utils/`**, a package of six helpers with no test methods where it was the only test; every other IT sits at top level, in `chaostests/` or `state/`. The move was not free and the compiler said why: it uses `setParallelConsumerForTest`, package-private on `ManagedPCInstance`. So the filing was decided by a modifier rather than by where the test belongs. The seam is now public, with that reasoning recorded at the seam; this source root is test-only, so nothing is published by widening it. One citation lived in a dated solutions record, which may not be rewritten to match today's code. Per docs/citations.md it is annotated rather than edited: the note says where the test went and why, and leaves the original claim alone - the flakiness it records was real when it was measured.
|
@claude review this Round 4. Since round 3 this PR gained a representation change to the detector itself, a test deletion backed by measurement, and a rename — plus a correction to the PR body round 3 flagged. Where to look hardest, in order:
Already settled, do not re-litigate: the key-order cell duplication (deferred, Verified locally before asking: 36/36 broker-free gating tests green in 2.4s, and a negative control confirms the wedge tests go red when the fix is disabled — so the detector can still fail for the right reason. |
|
Claude finished @astubbs's task in 7m 58s —— View job Reviewing chaos instrumentation PR — round 4
ReviewHEAD is Local verification, independent of the PR's own numbers:
1. Hand-traced On "can it fire on legitimate redelivery": traced why a teardown race can't manufacture a false overlap either. One real finding, exactly the naming/representation class the trigger comment asked me to weight this round: 2. Confirmed the file is fully deleted (not just renamed) and nothing else in the tree still references the class by name except one dated solutions doc, which already has a 2026-08-20 citation-repair addendum recording the deletion and pointing at the replacement — exactly per 3. The only change to Nothing else stood out. The four doc-only commits ( One thing already settled that I did not re-litigate per the ask: the key-order cell duplication, the lgtm modulo the one stale-javadoc finding above — small, contained, and in the same file/commit the trigger comment already flagged as having had one representation defect land silently once this round. |
…ts a stalled instance, and can be watched mid-stall Ordering is asserted throughout the rest of the test suite - it is the guarantee this library exists for - but not in the one lane that reorders the world underneath the consumer: repeated rebalances, revocations and restarts mid-flight. That is where a re-ordering bug could hide from every other test. **`KeyOrderLedger` is an event register.** The recorder writes facts - key, offset, when a worker started and finished - and `check()` interprets them once, with the whole run in front of it. Keeping those apart is why it can be trusted: a fact discarded mid-run cannot be reconsidered afterwards. It compares within one incarnation, partition, epoch and key, because a naive "offsets per key must increase" fires on CORRECT behaviour - at-least-once redelivery after a revoke is the contract working. **Absence is `null`, never a sentinel.** A delivery still running has no end time, and that is a fact about the run. An earlier version deleted the window's end on seeing one, which turned the detector off at the exact moment of interest - a wedged worker holding a key while the next record of that key runs elsewhere IS the violation, and it reported green because it had stopped looking. The first fix for that used `Long.MAX_VALUE`, which fabricated a value that then did arithmetic in `Math::max` and leaked into the report as "ended at seq 9223372036854775807". A sentinel is type-compatible with the arithmetic; a null `Long` is not, so the compiler forces every site to decide what "we do not know" means there. Both directions verified: the wedge tests fail without the fix and pass with it, and 340,000 live deliveries across two cells raised zero false alarms. **`InstanceStallProbeIT`** detects a member present and heartbeating but making no progress - the protocol-invisible shape most confluentinc#857 reports describe. **The diagnostic recovery mode** lets a stalled run be watched rather than killed, so the state that produced it survives to be read. Six scenario cells, each isolating one disturbance shape, including a 2x2 control arm over assignor and stop-mode whose weights are shared through `drainOnlyChaosWeights()` so the two cannot drift. **`ManagedPCInstanceLifecycleTest` is deleted rather than added.** A mutation matrix over the four guards in `ManagedPCInstance` showed it killed none of them - including the double-submission scenario its own javadoc names - while the existing `ManagedPCInstanceLifecycleIT` killed both real mutants deterministically, broker-free, in a hundredth of the time. `@RepeatedTest` is what you write when you cannot force a race and hope to draw it. That IT also moves out of `integrationTests/utils/`, six helpers with no test methods where it was the only test.
…nd what is recorded but unanalysed `AGENTS.md` routes every agent to this file for the chaos suite, and its section described two scenarios checking loss and duplication. Everything the suite gained lived only in `docs/inflight/` notes - which are DELETED when their work lands, so the durable record was about to disappear at merge, leaving the owner doc describing the suite as it was before. Three things it now carries. **A capability table**, so "is this already answerable?" is settled by reading rather than by grepping scenario classes - the calibration behind each capability is the expensive part, and rebuilding one because it was invisible is the waste this prevents. **Its limits, at the point of trust**: a cross-epoch overlap is not reported, and a `CLASS2_STALL` red proves a bound was crossed rather than that a backlog never drained. A capability list without its blind spots reads as a guarantee, which is worse than no list. And **what is recorded but not yet analysed**, phrased as an instruction rather than a fact. The ledger is an event register, so some questions need a new ANALYSIS, not new RECORDING: every delivery already holds incarnation, partition, epoch, key, offset and both timestamps, and the whole history is kept. Cross-epoch comparison is the worked example, with the query spelled out and the real cost named - not the data, the calibration of how long a revoked owner may legitimately still be finishing. The failure being prevented is someone adding a second recorder for data the first already holds.
…, and two records that contradicted each other Running the new cells produced evidence, and the ledger is where it belongs - a sighting that lives only in a CI log is gone when the retention window closes. **Four sightings**, figures taken from the failsafe artifacts rather than the console. The twelfth is the substantial one: the DRAIN control arm fired, 3 red of 4 exposures, and the entry deliberately refuses to cash it in - a fail-fast red proves the BOUND was crossed, not that the backlog never drained. A red-then-green pair one markdown file apart is recorded because it MEASURES seed-dependence where every prior entry asserted it. **Two notes had reached opposite conclusions about one signature, neither citing the other.** One said a Class 2 RED was runner contention; `bug-857-family.md` recorded the same ~154s signature replaying RED on plain master, uncontended. Both sound, different occurrences, and together they establish what neither said alone: ~154s LAG_STAGNATION is a signature, not a diagnosis. The discriminator is an uncontended replay, and its two sides need different evidence - a RED replay is positive and one carries it; a GREEN replay is an ABSENCE, and this family is intermittent, so the green side asks for two or three. **The self-review proposal cannot use reviews.** GitHub refuses `--request-changes` on your own PR, and here the maintainer authors nearly every PR - so the note records the variant that works: draft mode is the block, a checklist comment is the memory, and a future agent reaching for the merge hits an unmissable stop with the reason in the comments. Also: two notes renamed off state-carrying filenames; a third tagged for the gate that arrived with #324; the key-order cell duplication deferred with its trigger named; and a citation in a dated solutions record annotated rather than rewritten, per docs/citations.md.
0148625 to
283202e
Compare
✅ Hold lifted — the ⛔ DO NOT MERGE above is spentRetracting my own hold so nobody lands on it and stops. All three reasons it named are closed:
Current state: 31 checks green, 3 skipped, 0 unresolved threads. The only outstanding check is One thing green CI does not tell you, and it should be said plainly: the Recommended strategy: rebase-merge as-is. The branch was re-cut from 23 commits to 3, verified lossless — the pre-re-cut tip and the current head have the same tree object, |
…cted Merging master after #325 landed brought back `ManagedPCInstanceLifecycleTest` - 83 lines including the rapid-toggle test - which #325 had deliberately DELETED after a mutation matrix showed it caught nothing. This branch carried a stale copy from before that deletion, and the merge kept it rather than honouring the removal, so the file came back without a single conflict to announce it. That is the failure mode `docs/solutions/workflow-issues/theirs-took-the-whole-file-and-the-repair-stopped-at-the-tests-2026-08-18.md` already records from the other direction: a merge that reports clean has not thereby proved it preserved intent. A deletion is the case with no textual evidence left behind to notice. Swept for the rest of the class rather than fixing only the instance: compared every path present here against master's tree and checked each one master had ever deleted. This was the only resurrection - the other 13 files unique to this branch are its own additions. Verified: all 11 modules test-compile, core suite 399 pass / 8 skipped, and all twelve gates pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012J2KjzpiUKg2tGFT7B2D2e
Auditing the 63efb2d merge resolution against both parents turned up five stale spots, all in the docs the conflicts touched or the deletions this branch carries: - bug-857-family.md carried TWO drafts of the same "SUPERSEDED 2026-08-19" note for the seventh sighting - the hand-merge kept master's draft AND this branch's. They make the same claims in the same words; the branch's is the strict successor (records the defect as FIXED, and cites the durable docs/solutions/test-issues write-up where master's cited bug-pcmetrics-committed-offset-vs-completion-count.md, an inflight note this branch deletes). Dropped master's draft; the ledger is now byte-identical to this branch's pre-merge version, and diffing merged-vs-master beforehand confirmed master contributed nothing else this loses. - test-chaos-teardown-double-close.md pointed its Related bullet at test-857-churn-storm-async-stalls.md, a file that exists on NO ancestor of HEAD - the edit rode in via an old snapshot of test/chaos-instrumentation (ledger-split commit 4dab08e) that was dropped before #325 landed. Master kept the merge-base wording; restored it (bug-857-family.md). - pr-322-split-plan.md linked the deleted bug-pcmetrics-committed-offset-vs-completion-count.md; repointed at the solutions write-up that absorbed the diagnosis, naming the closure so the dated claim itself is untouched (docs/citations.md procedure: repair the address, never the claim). - test-class2-probe-asserts-timing-not-correctness.md said "twelfth sighting" for the 2026-08-20 drain-arm entry - true on master, but the merge interleaved this branch's twelfth and thirteenth ahead of it, renumbering it to fourteenth. Updated, with a note to grep the date since ordinals in that ledger are positions in a list two branches append to. - pr-57-metrics-leak.md still warned this PR was stacked on #325 with base test/chaos-instrumentation and asked for the retarget to be confirmed. Confirmed via gh pr view 57 (baseRefName: master); the bullet's work is done, so it is deleted rather than rewritten as a FIXED narrative, per docs/inflight/AGENTS.md. None of these are caught by bin/check-file-refs.sh: the dangling links sit on lines inherited from origin/master, and the ratchet ignores inherited findings; the churn-storm ref is backtick text, not a markdown link. Two more master-side files (test-chaos-crash-fidelity-variant.md, and test-class2-probe's own body) also cite the never-landed test-857-revoke-under-work-sightings.md - pre-existing on master, left alone. Verified: all six doc gates green; ./mvnw -o -pl parallel-consumer-core -am test green (399 tests, 0 failures). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012J2KjzpiUKg2tGFT7B2D2e
…le-test move Brings `origin/master` (3 commits, merge base `641709cb2`) onto `fix/909-load-reproduction`: the chaos suite's ordering assertion (`KeyOrderLedger` + `KeyOrderLedgerIT`), `InstanceStallProbeIT`, four new scenario cells under `chaostests/`, the rewritten chaos section of `docs/testing.md`, and the `docs/inflight/` sightings appended to `bug-857-family.md`. ## Zero conflicts, and this time git was right The two sides touched disjoint file sets (master: chaos tests + docs/inflight + docs/testing.md; this branch: PCModule/ShardKey/WorkManager, RegistrationRaceStaleResidentIT, PausableInsertShardManager, Quarantined, docs/refactoring.md). Because a clean exit has lied to this stack four times, each known trap was checked explicitly rather than trusted: - `ManagedPCInstanceLifecycleIT.java` exists only at `integrationTests/`, and the old `integrationTests/utils/` copy is gone - the rename landed once, not as a duplicate pair. `ManagedPCInstanceLifecycleTest.java` (deleted upstream of the merge base) exists nowhere. - The `### state/ShardKey.java` backlog entry that the previous merge (`edb6831fd`) had to re-delete by hand stayed deleted: `grep -c 'state/ShardKey.java' docs/refactoring.md` -> 0, and master did not touch `docs/refactoring.md` in this window. `state/ProcessingShard.java` still present. - No two `docs/inflight/` notes share content (md5 over the directory: 0 duplicate hashes), and `bin/check-inflight-tags.sh` passes (88 notes valid), so no note carries a doubled state tag. - Superset proof, one marker per side: master's `drainOnlyChaosWeights()` present in the three revoke-under-work cells; this branch's `PausableInsertShardManager` (5 uses) still wired into `RegistrationRaceStaleResidentIT`. ## Verified `bin/check-inflight-tags.sh`, `bin/check-file-refs.sh`, `bin/check-issue-refs.sh`, `bin/check-quarantine-registry.sh`, `bin/check-copyright-headers.sh`, `bin/test-check-agent-hooks.sh` all pass. `./mvnw test-compile -pl parallel-consumer-core -am` passes, and the compiled output contains both sides' new classes (`KeyOrderLedger`, `InstanceStallProbeIT`, `RegistrationRaceStaleResidentIT`, `ManagedPCInstanceLifecycleIT`). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019infZnCpPvX8c6QWFC1uLC
…is correct #326 hit this and the workaround was reverted, so the finding needs a home before it is lost. The engine's only baseline is the base branch computed live, which means identical files are accepted or rejected purely by which side of a merge they sit on: four existing ArchUnit wrappers pass at ~84% because they are not an increase, while three identical new ones fail at 84.3%. Recorded as `bug` / `misdirection` rather than a task, because a check whose verdict depends on merge timing rather than on the code is an instrument that misreports - and this repo ranks signal integrity above severity precisely because you cannot judge code through instruments that lie. The note carries why each obvious workaround is wrong, since all four are tempting and three were seriously considered: excluding the example directories also hides the five app tests that are the likeliest real duplication; `only_code` raises the wrappers' similarity rather than lowering it; lowering the threshold blinds everything; and making the check advisory downgrades an engine with a demonstrated catch - it found the 83% drain-control-arm duplication on #325 that the density-based clone engines could not have. Names the better fix shape too: an accepted-pairs baseline rather than a path ignore, so a NEW pair between the same files still fails instead of the file being silenced forever.
…dings this work turned up **The thirteenth sighting, with its seed.** #325 merged one minute after its last check landed, while the chaos evidence was still being extracted, so the run went unrecorded - and CI artifacts expire. By that ledger's own argument the seed is the asset: a sighting without one is an anecdote. It earns its place beyond the tally, because this was the FIRST full chaos storm run against the ordering ledger and stall probe #325 added and **none of them fired** - zero `INSTANCE_STALL`, zero `LEDGER_KEY_ORDER`, zero `LEDGER_KEY_CONCURRENCY`, across 21 violations that were all the pre-existing `CLASS2_STALL` timing bound. A new detector's likeliest failure is crying wolf; this is the first evidence these two do not. Recorded as UNRESOLVED, not contention - the discriminator is an uncontended replay of that seed and nobody has run one. **A register of the other compounding candidates**, each with the incident behind it, including what was checked and found ALREADY covered - that half is what stops a proposal arriving a third time. It also records two patterns no single incident makes visible: enforcement here is concentrated at commit and merge time while the failures concentrated in claims made in chat, and a rule that lands only in an unmerged PR does not bind with nothing able to tell you. **The similarity engine cannot accept duplication that is correct.** Its only baseline is the base branch computed live, so identical files are accepted or rejected by which side of a merge they sit on: four existing ArchUnit wrappers pass at ~84% because they are not an increase, while the three identical ones this PR adds fail at 84.3%. Filed as `bug`/`misdirection` rather than a task, because a check whose verdict depends on merge timing rather than on the code is an instrument that misreports. The note records why all four obvious workarounds are wrong - including that making the check advisory would downgrade an engine with a demonstrated catch - and names the better fix: an accepted-pairs baseline rather than a path ignore, so a NEW pair between the same files still fails.
…dings this work turned up **The thirteenth sighting, with its seed.** #325 merged one minute after its last check landed, while the chaos evidence was still being extracted, so the run went unrecorded - and CI artifacts expire. By that ledger's own argument the seed is the asset: a sighting without one is an anecdote. It earns its place beyond the tally, because this was the FIRST full chaos storm run against the ordering ledger and stall probe #325 added and **none of them fired** - zero `INSTANCE_STALL`, zero `LEDGER_KEY_ORDER`, zero `LEDGER_KEY_CONCURRENCY`, across 21 violations that were all the pre-existing `CLASS2_STALL` timing bound. A new detector's likeliest failure is crying wolf; this is the first evidence these two do not. Recorded as UNRESOLVED, not contention - the discriminator is an uncontended replay of that seed and nobody has run one. **A register of the other compounding candidates**, each with the incident behind it, including what was checked and found ALREADY covered - that half is what stops a proposal arriving a third time. It also records two patterns no single incident makes visible: enforcement here is concentrated at commit and merge time while the failures concentrated in claims made in chat, and a rule that lands only in an unmerged PR does not bind with nothing able to tell you. **The similarity engine cannot accept duplication that is correct.** Its only baseline is the base branch computed live, so identical files are accepted or rejected by which side of a merge they sit on: four existing ArchUnit wrappers pass at ~84% because they are not an increase, while the three identical ones this PR adds fail at 84.3%. Filed as `bug`/`misdirection` rather than a task, because a check whose verdict depends on merge timing rather than on the code is an instrument that misreports. The note records why all four obvious workarounds are wrong - including that making the check advisory would downgrade an engine with a demonstrated catch - and names the better fix: an accepted-pairs baseline rather than a path ignore, so a NEW pair between the same files still fails.
…dings this work turned up **The thirteenth sighting, with its seed.** #325 merged one minute after its last check landed, while the chaos evidence was still being extracted, so the run went unrecorded - and CI artifacts expire. By that ledger's own argument the seed is the asset: a sighting without one is an anecdote. It earns its place beyond the tally, because this was the FIRST full chaos storm run against the ordering ledger and stall probe #325 added and **none of them fired** - zero `INSTANCE_STALL`, zero `LEDGER_KEY_ORDER`, zero `LEDGER_KEY_CONCURRENCY`, across 21 violations that were all the pre-existing `CLASS2_STALL` timing bound. A new detector's likeliest failure is crying wolf; this is the first evidence these two do not. Recorded as UNRESOLVED, not contention - the discriminator is an uncontended replay of that seed and nobody has run one. **A register of the other compounding candidates**, each with the incident behind it, including what was checked and found ALREADY covered - that half is what stops a proposal arriving a third time. It also records two patterns no single incident makes visible: enforcement here is concentrated at commit and merge time while the failures concentrated in claims made in chat, and a rule that lands only in an unmerged PR does not bind with nothing able to tell you. **The similarity engine cannot accept duplication that is correct.** Its only baseline is the base branch computed live, so identical files are accepted or rejected by which side of a merge they sit on: four existing ArchUnit wrappers pass at ~84% because they are not an increase, while the three identical ones this PR adds fail at 84.3%. Filed as `bug`/`misdirection` rather than a task, because a check whose verdict depends on merge timing rather than on the code is an instrument that misreports. The note records why all four obvious workarounds are wrong - including that making the check advisory would downgrade an engine with a demonstrated catch - and names the better fix: an accepted-pairs baseline rather than a path ignore, so a NEW pair between the same files still fails.
…he route that caused it The previous commit filed the `Chaos Pain Suite` red on this branch as the runner-load kill class: logs apparently stopping dead 54 seconds in, no `BUILD FAILURE`, no `Tests run:`, no `##[error]`. Every one of those observations was an artefact of the fetch. `gh run view --job <id> --log` returned 990 lines of a ~5000-line log, cut inside a PASSING test and then appended the post-job steps - so the file ended with a cleanup step and passed the completeness check. What actually happened, from the failsafe XML in the uploaded artifact: eight chaos ITs green, `ChaosChurnStormIT.churnStormMeetsSlosAndBalancesLedger` red at 224.7s on probe violations, seed `7852140587594987229` recoverable. Recorded as the fourteenth sighting in `bug-857-family.md`, where it is worth more than a tally mark: at t=+154670ms it tripped `INSTANCE_STALL/NO_WORK_COMPLETED` - instance 70 holding 43 records out for processing and completing none - which is the first firing of the detector #325 added, and which the thirteenth sighting recorded as having produced zero findings. The 23 violations the autopsy lists are all the `CLASS2_STALL` timing bound that `test-class2-probe-asserts-timing-not-correctness.md` already says is not a correctness verdict, so reading only the autopsy list would have filed this as another tally. THIS IS THE THIRD RECORDED INSTANCE of `gh-run-view-log-truncation.md`, and the first where the reader was routed into it by the topic doc. That write-up's own Related section had named the fix - `docs/ci.md` documented the job-level API route alone, with no completeness warning - and it had sat undone since 2026-08-18. So `docs/ci.md` now leads with the artifact and archive-zip routes, names both ways the job-level call misleads (silent truncation; exit 1 writing zero bytes with the reason on stderr, which a `>` redirect discards), and carries the check that a grep returning zero on a truncated log is a false negative shaped exactly like a clean run. One refinement the incident adds to the write-up: its terminal-marker check is not sufficient as stated. This log DID end with a cleanup step - the marker has to belong to the step being diagnosed, not merely to the job. The false entry in `ci-disabled-jobs-and-runner-load.md` is replaced by a short warning, because the signature that entry describes and a truncated log are indistinguishable by the absence of `Tests run:` - which is exactly how this went wrong.
…ed-dependence observed directly `Chaos Pain Suite` went red again on the next push, and diagnosing it artifact-first this time took a minute rather than an hour: `ChaosRevokeUnderWorkIT.revokeUnderWorkStaysProtocolHonest`, the eager arm, 17 violations all `CLASS2_STALL/LAG_STAGNATION`, seed `567232329738342203`, both cooperative arms green. The pair is worth more than either red. Two runs on the same tree, minutes apart, failed DIFFERENT tests on DIFFERENT seeds while the branch changes no product code - agent hooks, shell gates and documentation only. This ledger has asserted seed-dependence since its second sighting by reasoning from other branches passing on the same day; here it is the same branch twice, which is the stronger form of the same observation. It also constrains the fourteenth sighting rather than merely following it: this run produced zero `INSTANCE_STALL`, so #325's detector is not firing on every chaos run - which is what an over-eager new detector would look like, and is the concern the thirteenth sighting raised.
…PRs merged five days ago `pr-323-docs-outstanding.md` and `pr-324-tooling-outstanding.md` each ended with that instruction. #323 and #324 both merged on 2026-08-20, so master has been carrying two notes that describe closed work as open - the exact residue `docs/inflight/AGENTS.md` forbids: "Never leave a 'delete this when #NN merges' marker on master. The merge is exactly when nobody is looking here." Repaired the way that doc prescribes - shrink to what survives, rename to an area prefix - never rewritten into an accurate past tense. EVERY SURVIVING ITEM WAS RE-VERIFIED RATHER THAN CARRIED, and that is what removed most of them. A finding nobody re-checks is how a stale claim gets a second life, which is the failure this whole exercise is about. Discharged, so deleted rather than moved: - #324's first item - `bin/test-check-agent-hooks.sh` asserting against the **live** `docs/inflight/` corpus instead of a fixture. Already fixed: the grouping and ordering cases build `mktemp` fixture trees (`$class_tmp/docs/inflight/...`, `$pos_tmp/docs/inflight/...`), and the two cases that still write into the live directory create the note they assert on and remove it. The only other mention is a comment. - Both "Already fixed" sections, #323's "Closed: the session-index window", and #324's "Ordering" - with both PRs merged, a merge protocol is history, not state. - #323's observation that `STRATEGY.md` repeats two incidents `docs/compound-engineering.md` owns. It was recorded as *left alone deliberately*; a decision not to act is not open work, and keeping it invites someone to re-litigate a settled call. Still open, relocated to where the fact belongs: - **The five stranded commit bodies get their own note**, `branch-agent-hook-commit-bodies-only-on-backup.md` - `branch-` because that is work sitting on a branch with no PR. `origin/backup/pre-split-322` and all five SHAs resolve; their content is on master but their bodies are not, and the note records that the claim a reconciliation commit names them is false. `pr-322-split-plan.md` stated the same thing WITH that false claim, so it now points at the owner instead of repeating it. - **`docs/quarantined-tests.md`'s overstatement is corrected in place, not tracked.** It said the owner-granted exception "had become the routine path" against a record of exactly one. The rule change stands on its own evidence, so the sentence now says what the record supports - the old rule's only escape hatch was an owner grant, leaving a contributor no path on the evidence they had. Do not track what you can correct. - **The two "unverifiable as written" observations moved INSIDE the notes they are about**, rather than into a third note tracking other notes' defects: the PCMetrics figures are marked as one run's log never re-derived locally, and the stale-arrival guard's unnamed third NPEing fixture now says plainly that recovering it means re-running the prototype. A caveat a reader meets while reading the claim beats one filed elsewhere. - **The merge ordering (323, 324, 325, 57, 322, 267, 29 - first three merged, so 57, 322, 267, 29 remain) moved to `pr-blockers-and-collisions.md`**, which owns standing coordination facts, and `bug-shared-collections-across-the-poll-boundary.md` was repointed there. That citation is why deletion alone would have been wrong: a note can hold the last copy of a fact nobody filed anywhere else. `pr-322-split-plan.md` survives - #322 is still open - but carried the same defect in miniature: instructions conditioned on merges that have happened, including a rename obligation on #325, which merged and whose two notes now carry `test-` prefixes. Those are gone and the table records #324's merge. Prior art, and the collision: #357 carries `4cc87c8ee`, which does this same cleanup. It is BLOCKED behind #354, so the lying notes would sit on master until that clears. This lands the same relocation with two differences its author should keep: the caveats go inside their owner notes rather than into a `docs-claims-...` note about other notes, and the quarantine sentence is fixed rather than filed. Gates run locally: check-inflight-tags (91 notes valid), check-file-refs, check-issue-refs, check-branch-self-reference (branch and PR 348), check-quarantine-registry, and the .claude/hooks self-test suite.
…PRs merged five days ago `pr-323-docs-outstanding.md` and `pr-324-tooling-outstanding.md` each ended with that instruction. #323 and #324 both merged on 2026-08-20, so master has been carrying two notes that describe closed work as open - the exact residue `docs/inflight/AGENTS.md` forbids: "Never leave a 'delete this when #NN merges' marker on master. The merge is exactly when nobody is looking here." Repaired the way that doc prescribes - shrink to what survives, rename to an area prefix - never rewritten into an accurate past tense. EVERY SURVIVING ITEM WAS RE-VERIFIED RATHER THAN CARRIED, and that is what removed most of them. A finding nobody re-checks is how a stale claim gets a second life, which is the failure this whole exercise is about. Discharged, so deleted rather than moved: - #324's first item - `bin/test-check-agent-hooks.sh` asserting against the **live** `docs/inflight/` corpus instead of a fixture. Already fixed: the grouping and ordering cases build `mktemp` fixture trees (`$class_tmp/docs/inflight/...`, `$pos_tmp/docs/inflight/...`), and the two cases that still write into the live directory create the note they assert on and remove it. The only other mention is a comment. - Both "Already fixed" sections, #323's "Closed: the session-index window", and #324's "Ordering" - with both PRs merged, a merge protocol is history, not state. - #323's observation that `STRATEGY.md` repeats two incidents `docs/compound-engineering.md` owns. It was recorded as *left alone deliberately*; a decision not to act is not open work, and keeping it invites someone to re-litigate a settled call. Still open, relocated to where the fact belongs: - **The five stranded commit bodies get their own note**, `branch-agent-hook-commit-bodies-only-on-backup.md` - `branch-` because that is work sitting on a branch with no PR. `origin/backup/pre-split-322` and all five SHAs resolve; their content is on master but their bodies are not, and the note records that the claim a reconciliation commit names them is false. `pr-322-split-plan.md` stated the same thing WITH that false claim, so it now points at the owner instead of repeating it. - **`docs/quarantined-tests.md`'s overstatement is corrected in place, not tracked.** It said the owner-granted exception "had become the routine path" against a record of exactly one. The rule change stands on its own evidence, so the sentence now says what the record supports - the old rule's only escape hatch was an owner grant, leaving a contributor no path on the evidence they had. Do not track what you can correct. - **The two "unverifiable as written" observations moved INSIDE the notes they are about**, rather than into a third note tracking other notes' defects: the PCMetrics figures are marked as one run's log never re-derived locally, and the stale-arrival guard's unnamed third NPEing fixture now says plainly that recovering it means re-running the prototype. A caveat a reader meets while reading the claim beats one filed elsewhere. - **The merge ordering (323, 324, 325, 57, 322, 267, 29 - first three merged, so 57, 322, 267, 29 remain) moved to `pr-blockers-and-collisions.md`**, which owns standing coordination facts, and `bug-shared-collections-across-the-poll-boundary.md` was repointed there. That citation is why deletion alone would have been wrong: a note can hold the last copy of a fact nobody filed anywhere else. `pr-322-split-plan.md` survives - #322 is still open - but carried the same defect in miniature: instructions conditioned on merges that have happened, including a rename obligation on #325, which merged and whose two notes now carry `test-` prefixes. Those are gone and the table records #324's merge. Prior art, and the collision: #357 carries `4cc87c8ee`, which does this same cleanup. It is BLOCKED behind #354, so the lying notes would sit on master until that clears. This lands the same relocation with two differences its author should keep: the caveats go inside their owner notes rather than into a `docs-claims-...` note about other notes, and the quarantine sentence is fixed rather than filed. Gates run locally: check-inflight-tags (91 notes valid), check-file-refs, check-issue-refs, check-branch-self-reference (branch and PR 348), check-quarantine-registry, and the .claude/hooks self-test suite.
…PRs merged five days ago `pr-323-docs-outstanding.md` and `pr-324-tooling-outstanding.md` each ended with that instruction. #323 and #324 both merged on 2026-08-20, so master has been carrying two notes that describe closed work as open - the exact residue `docs/inflight/AGENTS.md` forbids: "Never leave a 'delete this when #NN merges' marker on master. The merge is exactly when nobody is looking here." Repaired the way that doc prescribes - shrink to what survives, rename to an area prefix - never rewritten into an accurate past tense. EVERY SURVIVING ITEM WAS RE-VERIFIED RATHER THAN CARRIED, and that is what removed most of them. A finding nobody re-checks is how a stale claim gets a second life, which is the failure this whole exercise is about. Discharged, so deleted rather than moved: - #324's first item - `bin/test-check-agent-hooks.sh` asserting against the **live** `docs/inflight/` corpus instead of a fixture. Already fixed: the grouping and ordering cases build `mktemp` fixture trees (`$class_tmp/docs/inflight/...`, `$pos_tmp/docs/inflight/...`), and the two cases that still write into the live directory create the note they assert on and remove it. The only other mention is a comment. - Both "Already fixed" sections, #323's "Closed: the session-index window", and #324's "Ordering" - with both PRs merged, a merge protocol is history, not state. - #323's observation that `STRATEGY.md` repeats two incidents `docs/compound-engineering.md` owns. It was recorded as *left alone deliberately*; a decision not to act is not open work, and keeping it invites someone to re-litigate a settled call. Still open, relocated to where the fact belongs: - **The five stranded commit bodies get their own note**, `branch-agent-hook-commit-bodies-only-on-backup.md` - `branch-` because that is work sitting on a branch with no PR. `origin/backup/pre-split-322` and all five SHAs resolve; their content is on master but their bodies are not, and the note records that the claim a reconciliation commit names them is false. `pr-322-split-plan.md` stated the same thing WITH that false claim, so it now points at the owner instead of repeating it. - **`docs/quarantined-tests.md`'s overstatement is corrected in place, not tracked.** It said the owner-granted exception "had become the routine path" against a record of exactly one. The rule change stands on its own evidence, so the sentence now says what the record supports - the old rule's only escape hatch was an owner grant, leaving a contributor no path on the evidence they had. Do not track what you can correct. - **The two "unverifiable as written" observations moved INSIDE the notes they are about**, rather than into a third note tracking other notes' defects: the PCMetrics figures are marked as one run's log never re-derived locally, and the stale-arrival guard's unnamed third NPEing fixture now says plainly that recovering it means re-running the prototype. A caveat a reader meets while reading the claim beats one filed elsewhere. - **The merge ordering (323, 324, 325, 57, 322, 267, 29 - first three merged, so 57, 322, 267, 29 remain) moved to `pr-blockers-and-collisions.md`**, which owns standing coordination facts, and `bug-shared-collections-across-the-poll-boundary.md` was repointed there. That citation is why deletion alone would have been wrong: a note can hold the last copy of a fact nobody filed anywhere else. `pr-322-split-plan.md` survives - #322 is still open - but carried the same defect in miniature: instructions conditioned on merges that have happened, including a rename obligation on #325, which merged and whose two notes now carry `test-` prefixes. Those are gone and the table records #324's merge. Prior art, and the collision: #357 carries `4cc87c8ee`, which does this same cleanup. It is BLOCKED behind #354, so the lying notes would sit on master until that clears. This lands the same relocation with two differences its author should keep: the caveats go inside their owner notes rather than into a `docs-claims-...` note about other notes, and the quarantine sentence is fixed rather than filed. Gates run locally: check-inflight-tags (91 notes valid), check-file-refs, check-issue-refs, check-branch-self-reference (branch and PR 348), check-quarantine-registry, and the .claude/hooks self-test suite.
…PRs merged five days ago `pr-323-docs-outstanding.md` and `pr-324-tooling-outstanding.md` each ended with that instruction. #323 and #324 both merged on 2026-08-20, so master has been carrying two notes that describe closed work as open - the exact residue `docs/inflight/AGENTS.md` forbids: "Never leave a 'delete this when #NN merges' marker on master. The merge is exactly when nobody is looking here." Repaired the way that doc prescribes - shrink to what survives, rename to an area prefix - never rewritten into an accurate past tense. EVERY SURVIVING ITEM WAS RE-VERIFIED RATHER THAN CARRIED, and that is what removed most of them. A finding nobody re-checks is how a stale claim gets a second life, which is the failure this whole exercise is about. Discharged, so deleted rather than moved: - #324's first item - `bin/test-check-agent-hooks.sh` asserting against the **live** `docs/inflight/` corpus instead of a fixture. Already fixed: the grouping and ordering cases build `mktemp` fixture trees (`$class_tmp/docs/inflight/...`, `$pos_tmp/docs/inflight/...`), and the two cases that still write into the live directory create the note they assert on and remove it. The only other mention is a comment. - Both "Already fixed" sections, #323's "Closed: the session-index window", and #324's "Ordering" - with both PRs merged, a merge protocol is history, not state. - #323's observation that `STRATEGY.md` repeats two incidents `docs/compound-engineering.md` owns. It was recorded as *left alone deliberately*; a decision not to act is not open work, and keeping it invites someone to re-litigate a settled call. Still open, relocated to where the fact belongs: - **The five stranded commit bodies get their own note**, `branch-agent-hook-commit-bodies-only-on-backup.md` - `branch-` because that is work sitting on a branch with no PR. `origin/backup/pre-split-322` and all five SHAs resolve; their content is on master but their bodies are not, and the note records that the claim a reconciliation commit names them is false. `pr-322-split-plan.md` stated the same thing WITH that false claim, so it now points at the owner instead of repeating it. - **`docs/quarantined-tests.md`'s overstatement is corrected in place, not tracked.** It said the owner-granted exception "had become the routine path" against a record of exactly one. The rule change stands on its own evidence, so the sentence now says what the record supports - the old rule's only escape hatch was an owner grant, leaving a contributor no path on the evidence they had. Do not track what you can correct. - **The two "unverifiable as written" observations moved INSIDE the notes they are about**, rather than into a third note tracking other notes' defects: the PCMetrics figures are marked as one run's log never re-derived locally, and the stale-arrival guard's unnamed third NPEing fixture now says plainly that recovering it means re-running the prototype. A caveat a reader meets while reading the claim beats one filed elsewhere. - **The merge ordering (323, 324, 325, 57, 322, 267, 29 - first three merged, so 57, 322, 267, 29 remain) moved to `pr-blockers-and-collisions.md`**, which owns standing coordination facts, and `bug-shared-collections-across-the-poll-boundary.md` was repointed there. That citation is why deletion alone would have been wrong: a note can hold the last copy of a fact nobody filed anywhere else. `pr-322-split-plan.md` survives - #322 is still open - but carried the same defect in miniature: instructions conditioned on merges that have happened, including a rename obligation on #325, which merged and whose two notes now carry `test-` prefixes. Those are gone and the table records #324's merge. Prior art, and the collision: #357 carries `4cc87c8ee`, which does this same cleanup. It is BLOCKED behind #354, so the lying notes would sit on master until that clears. This lands the same relocation with two differences its author should keep: the caveats go inside their owner notes rather than into a `docs-claims-...` note about other notes, and the quarantine sentence is fixed rather than filed. Gates run locally: check-inflight-tags (91 notes valid), check-file-refs, check-issue-refs, check-branch-self-reference (branch and PR 348), check-quarantine-registry, and the .claude/hooks self-test suite. Also here, same class one file over: #347's Lincheck note called `test/jcstress-poc-plain-long-visibility` "the branch carrying" the jcstress half. True while this PR was open, false the moment it merges - so both mentions now name the `jcstress-poc` module, which is a thing a future reader can go and look at. That one is worth a note about the gate rather than just a fix. `bin/check-branch-self-reference.sh` catches a note naming its OWN branch, and it only caught this because this PR is the one under test. On #347 those two lines named a sibling's branch and passed - so the window between two merges is exactly where a sibling-branch reference lands unchallenged.
…PRs merged five days ago `pr-323-docs-outstanding.md` and `pr-324-tooling-outstanding.md` each ended with that instruction. #323 and #324 both merged on 2026-08-20, so master has been carrying two notes that describe closed work as open - the exact residue `docs/inflight/AGENTS.md` forbids: "Never leave a 'delete this when #NN merges' marker on master. The merge is exactly when nobody is looking here." Repaired the way that doc prescribes - shrink to what survives, rename to an area prefix - never rewritten into an accurate past tense. EVERY SURVIVING ITEM WAS RE-VERIFIED RATHER THAN CARRIED, and that is what removed most of them. A finding nobody re-checks is how a stale claim gets a second life, which is the failure this whole exercise is about. Discharged, so deleted rather than moved: - #324's first item - `bin/test-check-agent-hooks.sh` asserting against the **live** `docs/inflight/` corpus instead of a fixture. Already fixed: the grouping and ordering cases build `mktemp` fixture trees (`$class_tmp/docs/inflight/...`, `$pos_tmp/docs/inflight/...`), and the two cases that still write into the live directory create the note they assert on and remove it. The only other mention is a comment. - Both "Already fixed" sections, #323's "Closed: the session-index window", and #324's "Ordering" - with both PRs merged, a merge protocol is history, not state. - #323's observation that `STRATEGY.md` repeats two incidents `docs/compound-engineering.md` owns. It was recorded as *left alone deliberately*; a decision not to act is not open work, and keeping it invites someone to re-litigate a settled call. Still open, relocated to where the fact belongs: - **The five stranded commit bodies get their own note**, `branch-agent-hook-commit-bodies-only-on-backup.md` - `branch-` because that is work sitting on a branch with no PR. `origin/backup/pre-split-322` and all five SHAs resolve; their content is on master but their bodies are not, and the note records that the claim a reconciliation commit names them is false. `pr-322-split-plan.md` stated the same thing WITH that false claim, so it now points at the owner instead of repeating it. - **`docs/quarantined-tests.md`'s overstatement is corrected in place, not tracked.** It said the owner-granted exception "had become the routine path" against a record of exactly one. The rule change stands on its own evidence, so the sentence now says what the record supports - the old rule's only escape hatch was an owner grant, leaving a contributor no path on the evidence they had. Do not track what you can correct. - **The two "unverifiable as written" observations moved INSIDE the notes they are about**, rather than into a third note tracking other notes' defects: the PCMetrics figures are marked as one run's log never re-derived locally, and the stale-arrival guard's unnamed third NPEing fixture now says plainly that recovering it means re-running the prototype. A caveat a reader meets while reading the claim beats one filed elsewhere. - **The merge ordering (323, 324, 325, 57, 322, 267, 29 - first three merged, so 57, 322, 267, 29 remain) moved to `pr-blockers-and-collisions.md`**, which owns standing coordination facts, and `bug-shared-collections-across-the-poll-boundary.md` was repointed there. That citation is why deletion alone would have been wrong: a note can hold the last copy of a fact nobody filed anywhere else. `pr-322-split-plan.md` survives - #322 is still open - but carried the same defect in miniature: instructions conditioned on merges that have happened, including a rename obligation on #325, which merged and whose two notes now carry `test-` prefixes. Those are gone and the table records #324's merge. Prior art, and the collision: #357 carries `4cc87c8ee`, which does this same cleanup. It is BLOCKED behind #354, so the lying notes would sit on master until that clears. This lands the same relocation with two differences its author should keep: the caveats go inside their owner notes rather than into a `docs-claims-...` note about other notes, and the quarantine sentence is fixed rather than filed. Gates run locally: check-inflight-tags (91 notes valid), check-file-refs, check-issue-refs, check-branch-self-reference (branch and PR 348), check-quarantine-registry, and the .claude/hooks self-test suite. Also here, same class one file over: #347's Lincheck note called `test/jcstress-poc-plain-long-visibility` "the branch carrying" the jcstress half. True while this PR was open, false the moment it merges - so both mentions now name the `jcstress-poc` module, which is a thing a future reader can go and look at. That one is worth a note about the gate rather than just a fix. `bin/check-branch-self-reference.sh` catches a note naming its OWN branch, and it only caught this because this PR is the one under test. On #347 those two lines named a sibling's branch and passed - so the window between two merges is exactly where a sibling-branch reference lands unchallenged.
…d" section, and move what is settled below it
bin/check-pr-ready.sh reported this note as listing 96 outstanding items. It does not, and never
did: the gate counts every line starting with "- " or "**" above an "## Already fixed" heading,
and this note had no such heading, so all 722 lines of prose formatting counted. The real
outstanding list is the NEXT TASKS section, which has five entries.
The gate is not wrong to block - what it is actually saying is that nothing in this note can be
marked resolved, because there is nowhere for resolved material to go. So it now has somewhere.
MOVED, NOT DELETED. Five sections are settled and sit below the heading with their reasoning
intact, because the reasoning is what a later reader needs:
* the merge-order decision, and the PCMetrics instruction - both resolved outright now that all
six dependencies this note waited on have merged (#57, #267, #322,
#323, #324, #325)
* whether #119 closes with this PR - answered rather than in progress
* what the PR is NOT, and the checked interaction with the shutdown/teardown race
The #119 section also loses its "Research in progress" title and its provisional hedge.
The inventory and merged-PR mapping were gathered, and the transactional revoke wait was
established as a separate defect in a commit mode this fix cannot reach - so the answer is no,
and closing that issue here would take the rest of the family with it. A section that reads
"provisional" long after it was settled is how a finished question gets re-asked.
The count above the heading drops accordingly, and what remains above it is genuinely open.
depends on #324
Third in the agreed merge order (323, 324, 325, 57, 322, 267, 29); #323
merged on 2026-08-20 as
a80f2bbd1. The dependency is real rather than bookkeeping: #324lands the inflight-tag and citation gates that the notes this PR adds are judged by. The line was
missing, so
Check PR Dependencieswas passing and nothing mechanically held the order.Description
Chaos-suite instrumentation, extracted from #29 so it can be reviewed on
its own terms. "Are these checks non-vacuous and correctly windowed?" is a different question from
"is this locking correct?", and one reviewer switching between the two mid-diff is where things get
waved through.
No main-code change. It compiles and runs against master untouched, which is also the evidence
that the chaos work never depended on #29's product fixes.
1. The CHAOS suite asserts ordering under churn, for the first time
To be clear about scope: ordering is asserted throughout the existing test suite - the integration
tests cover it extensively, and it is the guarantee this library exists for. What had never been
checked under churn was ordering in the chaos suite specifically, whose correctness ledger
verified that no record is lost and that duplicates stay bounded, but not their order. That gap
mattered because chaos is the only lane that reorders the world underneath the consumer - repeated
rebalances, revocations and restarts mid-flight - so it is the one place a re-ordering bug could hide
from every other test.
KeyOrderLedgerasserts it within one PC incarnation, one partition, one assignmentepoch and one key. Each component excludes a class of legitimate re-processing, and the
window is the whole difficulty: a naive "offsets per key must increase" fires on correct
behaviour, because at-least-once redelivery after a revoke is the contract working. The epoch is
read off the record's own
WorkContainerthrough public API, so a heavy record still in flight whenits partition is revoked keeps the old epoch and cannot be mistaken for the new window's work.
Two violation kinds, and the second is not decoration - the
UNORDEREDcontrol arm produced 857overlaps against one order regression, so an offset-order-only detector would have called that arm
green 857 times.
Vacuity is reported, not hoped for:
checkreturnsLEDGER_ORDER_VACUOUSwhen no window heldtwo deliveries, and logs
comparedDeliveries- 244,022 of 250,608 on the live run, so ~97% ofdeliveries were genuinely compared against a predecessor.
2.
INSTANCE_STALL/NO_WORK_COMPLETED- the liveness check at a granularity that can see a wedgeThe existing
NO_PROGRESSprobe has the right shape - while work remains, completions must advanceThis adds the same idea per instance, read from PC's own state (
pc.getWm()) rather than inferredfrom committed offsets read externally. Completions are counted through a listener that fires on
PC's control thread - the thread an AB-BA cycle freezes - so a wedged control loop stops the
counter even while workers finish and heartbeats flow. It re-arms on any result, idle, restart or
leaving the live set, so it cannot fire on slow-but-progressing.
Granularity is per-instance rather than per-shard, and the javadoc says so:
processingShardsis@Getter(PRIVATE)and adding a main-code accessor for a probe was not worth it.3. A diagnostic mode, because the gating run destroys its own evidence
-Dchaos.diagnoseStallRecovery=truekeeps the quiet phase watching after a violation instead ofaborting at the first one. Without it a wedged partition and a merely slow one are indistinguishable,
because every observation ends at the moment of detection.
It cannot turn a red run green - violations are still asserted afterwards - and it prints the
prior art on startup, since a recovery diagnostic on this family had already been run once and was
repeated because nothing led anyone to that record.
4.
CLASS2_STALLnow explains itself where it firesIts message carries the interpretation: a timing bound is not a correctness verdict, the mechanism is
a commit watermark pinned by one incomplete record while the shard behind it progresses, and the next
step is the diagnostic flag rather than "investigate". Three of four measurement arms failed this
check while progressing normally, and the natural reading of it is wrong.
The bound's value and its gating are unchanged here - the open question is recorded in
docs/inflight/test-class2-probe-asserts-timing-not-correctness.md.5. Four scenario cells
Drain, cooperative-drain and KEY-ordered arms complete an assignor x stop-mode matrix. Existing cells
are untouched; the shared driver gained hooks with byte-identical defaults.
Notes for the reviewer
deliberately and depends on this one; the duplication resolves when it merges master.
file-refs: N/Auntil it merges.@Tag("chaos")and never gates. What gates from this PR is the broker-freeledger and probe logic:
KeyOrderLedgerIT(19),InstanceStallProbeIT(7).Verification
KeyOrderLedgerIT19,InstanceStallProbeIT7,ProgressProbeLedgerIT5,ManagedPCInstanceLifecycleIT5.(Previously stated as 33/33 while counting
ManagedPCInstanceLifecycleTest, which was broker-BACKED - so the claim was wrong twice. Round 3 caught the miscount; that test has since been deleted after a mutation matrix showed it killed none of the fourManagedPCInstanceguards, while the deterministic IT killed both real ones.)INSTANCE_STALLverified silent on the eager arm (seed4734674029169027864), which trips theolder
CLASS2_STALLbound 53 times on that same seed while draining completely - the case it mustnot fire on
redelivery after revoke, restart of the same instance, a straggler crossing an epoch boundary, and
the same key on two partitions
Checklist
harness's inability to model a crash, and a feature record for seeded replay
docs/features/-seeded-chaos-replay.yamlKeyOrderLedgerITandInstanceStallProbeITprove thenew checks fire and stay silent in both directions