Repository navigation
test+docs: force the commit-timeout trigger, plus the rules and vocabulary - #220
Merged
Merged
Conversation
astubbs
force-pushed
the
test/commit-timeout-deterministic-trigger
branch
from
August 7, 2026 02:18
24fd310 to
437092c
Compare
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
Claude finished @astubbs's task in 4m 52s —— View job Claude's review of PR #220Todo list:
SummaryThis is a well-reasoned fix. I traced the mechanism claims against the actual source rather than taking the PR description at face value, and they hold up:
Minor nits (non-blocking)
AssessmentThe core insight — that the test was waiting 35s for a consequence without ever guaranteeing its trigger — is correct and well-evidenced (the control-arm table with the within-run |
✅ 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
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
No new clones introduced by this PR. Powered by astubbs/duplicate-code-cross-check |
✅ SpotBugs ReportNo bugs found (new bugs only — baseline from base branch excluded). |
|
Claude finished @astubbs's task in 2m 30s —— View job Claude's review of PR #220 (re-run after synchronize)Todo list:
What changed since the previous reviewThe previous review (posted on this same comment) covered everything through commit
Since the
No inconsistencies found between the new doc's narrative and the code/history it cites. The trimmed ledger entry reads cleanly and doesn't leave dangling context — a reader who only has the ledger entry gets the classification and a pointer to the mechanism, not a partial/stale duplicate. AssessmentNothing here changes my prior conclusion: the fix is sound, well-evidenced, and this doc-only follow-up commit accurately documents it without introducing any new claims that don't hold up against the code. No functional or correctness issues found in this delta. |
astubbs
added a commit
that referenced
this pull request
Aug 7, 2026
A fix that removes today's instance invites tomorrow's. Adds the sweep as a merge-prep step, timed deliberately: at merge prep, once the defect CLASS is understood - doing it mid-diagnosis only widens the investigation. The reporting requirement is the part that makes it useful. "None found" is worth nothing unless it says where you looked, and ruling a suspect out is a real result, not an absence of one. Run for #220. The class was "a test awaiting a consequence whose trigger it cannot force"; the greppable proxies were sleep-as-synchronisation in integration tests and awaits on a failure outcome. Results: - DrainCloseTest - closest relative. Two bare sleeps sequence a close-drain race, and because closeDrainFirst is commanded, the await's isClosedOrFailed disjunct is guaranteed to fire, so the await is pure synchronisation and the real check is the assertEquals after it. - RetriesTest - candidate. throwOnHeader/checking flipped on 3s/2s sleeps against a running consumer; the premise holds only if it got there in time. - TransactionTimeoutsTest.produceTimeout - explicitly NOT an instance, though it is the nearest sibling and still open in the flake ledger. It latches its trigger (the injected sendOffsetsToTransaction counts down while already holding the commit write lock, then sleeps 5s) and the worker attempts the read lock at latch+1s against a 2s deadline. Its tightness label stands. - Dismissed: chaos and probe pacing loops, OffsetCommittingSanityTest's explicit JUST_SLEEP mode, RebalanceEoSDeadlockTest's injected delay (that is the mechanism), TransactionMarkersTest's deliberately blocked record. Most isClosedOrFailed uses are failFast guards - opposite polarity, safe. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
added a commit
that referenced
this pull request
Aug 7, 2026
…st the tree Two things, both from re-running the capture workflow over work it had already documented by hand. CONCEPTS.md, seeded from the transactional-commit area this learning touched - the produce/commit lock pair and the rules about when each side may be taken: dirty, eager processing during commit, the bounded commit-lock acquisition. Plus the parallel-consumption nouns those rules only make sense against (control loop, broker poller, shard, in-flight work) and the test-reliability vocabulary this area keeps needing. Entries stand alone deliberately - no file paths, class names, PR links or current config values - so the file cannot rot as the code moves. Scoped to the area actually investigated; the rest of the project is explicitly not covered yet. The flagged-ambiguity tail records the distinction this investigation turned on: "stall", "load-tightness flake" and "unforceable trigger" had been used loosely for the same red test and are not the same thing. The write-up gained three corrections found by re-verifying every claim against the tree rather than trusting the earlier pass: - status overstated the fix as SOLVED; #220 is open, not merged - "setDirty() has exactly one caller" was imprecise - there are two overloads, and only the no-arg mark-true path has the single caller - a citation abbreviated a plan filename to something that resolves to nothing And three findings from prior sessions: this test was already a CI repeat offender in late July for an entirely different mechanism (cross-class broker contention tripping its intentional timeouts, fixed by forked-per-broker isolation, not by loosening them), which makes a third distinct way it goes red; a keyword search for "commitTimeout" mostly returns the offsetCommitTimeout config field, which is how "single sighting" survived scrutiny; and #29 claims the fix for the production commit-lock timeout this doc cites as a different bug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Claude finished @astubbs's task in 2m 17s —— View job Claude's review of PR #220 (re-run after synchronize)Todo list:
What changed since the last reviewThe last review on this comment covered everything through So the test-fix mechanics I already verified in the two prior review passes (gate logic,
One thing I could not verify in this sandbox: the specific PR/issue numbers newly cited in the frontmatter and body ( The new AssessmentNo functional or correctness issues found in this delta or in the PR overall. The doc additions since the last review are accurate against the current code (verified via line-by-line citation checks, not just read for plausibility), and no test or production code changed, so the earlier verdict — the fix is sound and well-evidenced — stands unchanged. |
astubbs
changed the title
astubbs
changed the title
commitTimeout waited 35s for PC to die from a commit-lock timeout, but nothing made that timeout reachable. It fires only while the slow record holds the produce read lock AND the controller is inside acquireCommitLock, and two independent things prevented that overlap: 1. The sleep started when the record acquired the produce lock, so the trigger had only `sleep - commitLockAcquisitionTimeout` of margin - and none when the attempt arrived a commit-interval later. 2. maybeAcquireCommitLock is gated on wm.isDirty(), whose sole mark-true setter is PartitionState#onSuccess (onFailure is a no-op). If every other record of the batch succeeds before the slow one starts, nothing marks dirty during its hold, no commit is attempted at all, and no timeout can fire. requestCommitAsap() cannot override this - isDirty is AND-ed into the gate. Either way PC stays healthy and logs nothing, and the await burns its full 35s. Widening a timeout cannot fix case 2: there is no deadline to widen. So the test now creates the overlap, the way produceTimeout already does on the other side of this lock. A latch on preAcquireOffsetsToCommit - wired through PCModule, not around it - times the sleep from the commit attempt, and one record's success is deferred into the lock hold so a commit is always attempted. The 35s await and isClosedOrFailed are untouched; no assertion is weakened. Confirmed by controlled experiment rather than by the fix appearing to work: stock, before 0/12 fail commitLockAcquisitionTimeout 2.5s vs 2s sleep 6/6 fail commitInterval 3s (hypothesis refuted) 0/6 fail latch removed + 1500ms delayed commit attempt 4/4 fail latch present + same 1500ms delay 0/4 fail fixed, stock, heavy load 0/8 fail guard deliberately broken 2/2 fail, on the guard The 6/6 arm carries its own control: param [2] (50s sleep) fired correctly in the same JVM at the same instant, ruling out machine speed by construction. Also corrects the method javadoc, which documented the small multiplier as 5 when SMALL_TIMEOUT_MULTIPLIER is 2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The write-up for the commitTimeout flake, in the directory the drain zombie and the nudge race went to once they were understood. The lesson generalises past this test: before awaiting a consequence, check the test can FORCE the trigger in every interleaving. Here it could not, in two ways - too little margin, and an isDirty gate that can suppress the commit attempt entirely, leaving no deadline to widen. The valuable section is the seven plausible readings that were wrong: the stall-by-analogy framing, the missing-ERROR inference (FakeRuntimeException is a PCRetriableException and logs at DEBUG), the refuted commitInterval hypothesis, brute-force soaking, the vacuous "probe clean" verdict, a CI history search that cannot see this class because a retried run reports success, and blaming the PR it surfaced on - excluded by tree hash, which is a technique worth copying. Records the other-instances sweep, led by its negative result: produceTimeout is the nearest sibling and is explicitly NOT an instance, so nobody fixes it the way this one was fixed. Cross-links the vacuous-await doc as a SIBLING rule - that hazard is an await satisfied too early, this one is an await whose trigger never fires - and says what the prior-art search did not cover. Code-behaviour claims carry file:line and were verified against the tree. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
It reads exactly like a member - an await that expired under load - and it is not one. The 35s await is not the margin; it is the deadline for a consequence. The margin belongs to the trigger, and neither thing that stopped it firing is tightness. Points at the solution doc for the mechanism rather than restating it, so the two copies cannot drift. The point of the entry is the warning it leaves for the members still listed, produceTimeout especially: before filing any of them as a tight assertion, check whether the awaited thing can be triggered at all in every interleaving. A test that waits on a consequence it cannot force is not tight, it is unsound, and raising its timeout will never fix it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…estion Two paths release the same ProducingLock on the same worker thread - WorkContainer#onPostAddToMailBox and cleanUpContext's finally - and nothing clears PollContextInternal#producingLock between them. A second unlock() by a thread holding zero read locks should throw IllegalMonitorStateException. Raised and deliberately not chased in docs/plans/2026-08-03-001-investigate-transactional-commit-flake.md, where the attempt to measure it was void: surefire:test does not reprocess test resources, so the debug logging never reached target/test-classes. Measured here with -am verify, which does: 340 acquires, 340 releases, 0 IllegalMonitorStateException. Exactly 1:1, so no double release actually occurs and the "is it silently throwing" half is closed. What stays open is which path is skipped and why, and whether that holds for the error, retry and stale-work branches, which were not separately measured. Worth someone's time because this lock enforces the EOS invariant and has already produced two problems. A release path that is correct by accident rather than by construction is how the next one arrives. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…fork Three failures found while capturing the learning above, all of which return a clean or empty result indistinguishable from a genuine one. A tool that crashes gets fixed; a tool that returns "nothing found" gets believed. - A worktree's basename is not the repo name. Anything keyed on `basename $(git rev-parse --show-toplevel)` gets the worktree directory, so a session search matched zero sessions where the correct name matched 37 - then recorded "no relevant prior sessions" and carried on. Use --git-common-dir, which every worktree of a repo agrees on, and take its dirname. - A worktree has no repo-root config, so a health check resolving a config path against --show-toplevel reports it missing when it lives in the main checkout. - Bare `gh` in a fork resolves upstream. `gh pr view` without -R returned "no pull requests found" for a branch with an open PR. That one is worse than a missing result: a verifier treating not-found as unverified will downgrade a correct claim, turning a right doc wrong. Relevant here specifically because AGENTS.md mandates worktrees for all work and this is a fork with a live upstream, so both conditions hold on every run. Writing it exercised its own thesis: the worktree-safe command in the first draft was wrong - basename of the common dir is `.git`, so the sed stripped it to an empty string - caught by running it rather than reasoning about it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Shared domain vocabulary, seeded from the area this investigation touched: the produce/commit lock pair and the rules for when each side may be taken - dirty, eager processing during commit, the bounded commit-lock acquisition - plus the parallel-consumption nouns those rules only make sense against, and the test-reliability vocabulary this area keeps needing. Entries stand alone deliberately: no file paths, class names, PR links or current config values, so the file cannot rot as the code moves. Scoped to the area actually investigated; the rest of the project is explicitly not covered yet. The flagged-ambiguity tail records the distinction this whole investigation turned on - "stall", "load-tightness flake" and "unforceable trigger" had been used loosely for the same red test and are three different things. Indexed in the "Where things live" table so it is discoverable by the same route as every other doc here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…th a control arm Two rules, both learned by getting them wrong on the commitTimeout flake. Before forming a hypothesis, check prior plans, solved-problem write-ups, the in-flight ledger, PRs and the issue tracker - fork and upstream - and report what each returned, including "nothing". Grep the mechanism, not the symptom: the failing test's name is the weakest search term there is. Searching for "commitTimeout" found nothing; searching for producerTransactionLock finds the only prior investigation into that exact lock, which already had both its ordering invariant and the method for settling this class of question. `--state open` is a collision check, not a prior-art search: the PR that solved something in your file is by definition merged, and the issue is usually closed. Searching only the open list finds nothing and feels like diligence. Search merged PRs BY TOUCHED FILE - a PR that fixed the same class rarely describes the same symptom. The control-arm method is promoted out of a dated plan doc, because AGENTS.md itself says a plan goes stale once its work lands and this must not go with it: confirm a cause by changing one term and showing the outcome flips, state the prediction first, and report refuted predictions as prominently as confirmed ones. It carries the two build traps that silently void an experiment - `-pl` without `-am`, and surefire:test not reprocessing test resources - plus the rule that a guard shipped with a fix must be verified by negative control. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…rs could fire AGENTS.md told agents that `probe clean` means the fault is likely in the test rather than in consumer-group progress. That is only true when the probe could have detected something. Lag stagnation needs LAG_STAGNATION_MIN_LAG of real lag sustained past LAG_STAGNATION_BOUND, and rebalance dwell needs REBALANCE_DWELL_BOUND. A test with a handful of records, or one that fails inside a window shorter than those bounds, cannot trip either - so its autopsy prints `probe clean` and the accompanying sentence carries no evidence at all. Not hypothetical: the commitTimeout autopsy read `probe clean` on a 15-record test that failed in 35s, against thresholds of 50 records and 150s, and the handoff written from it treated that as a finding. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A fix that removes today's instance invites tomorrow's. Adds the sweep as a merge-prep step, timed deliberately: once the defect CLASS is understood, since doing it mid-diagnosis only widens the investigation. The reporting requirement is what makes it useful. "None found" is worth nothing unless it says where you looked, and ruling a suspect out is a real result. Run for this branch. The class was "a test awaiting a consequence whose trigger it cannot force"; the proxies were sleep-as-synchronisation in integration tests and awaits on a failure outcome. It found DrainCloseTest and RetriesTest as relatives, dismissed the chaos and probe pacing loops with reasons, and - most usefully - confirmed TransactionTimeoutsTest.produceTimeout is NOT an instance despite being the nearest sibling, because it latches its trigger and keeps a real margin. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
astubbs
force-pushed
the
test/commit-timeout-deterministic-trigger
branch
from
August 7, 2026 04:04
4acbcde to
a3c591e
Compare
|
Claude finished @astubbs's task in 1m 11s —— View job Claude's review of PR #220 (re-run after synchronize)Todo list:
What changed since the last review
Confirmed still-standing verdict (unchanged from the last 3 passes)Since there's nothing new, I re-checked rather than re-derived: the diff
Minor nits (non-blocking, carried over — no need to act on these given the approval)
AssessmentNo functional or correctness issues found. The fix is sound and well-evidenced, the docs are accurate against the code they cite, and this is unchanged from the prior review passes. |
4 tasks done
astubbs
added a commit
that referenced
this pull request
Aug 11, 2026
Review finding on #224. The earlier sweep on this branch checked whether the AGENTS.md headings a citation named still existed, not whether the content the citation claimed was still under them. #272 kept the headings and moved the content, so that test could not detect the defect it was looking for, and two instances survived in the same file the sweep had already edited for exactly this reason. - The related-PRs entry said #220's probe correction is "in AGENTS.md". Historically true, so this records the relocation rather than rewriting it: the thresholds are now in docs/testing.md and the settling method in docs/investigating.md. - "Its §11 control-arm method is now promoted into AGENTS.md" was simply false. AGENTS.md:151-152 explicitly hands hypothesis-settling off, and the method is at docs/investigating.md:13-18. Repointed there. Re-swept the branch on the corrected test - every doc-location citation in every file this PR touches, checked against where the content actually lives now, not against whether the heading survived. The remaining one, "AGENTS.md (Testing) - carries the no-retry rule and the quarantine alternative", still holds: root Testing states both. The two inflight entries cite no doc locations at all. Both new relative links resolve. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XXViiRK9G6aHhYFoL3RTst
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
1 participant
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.
Description
TransactionTimeoutsTest.commitTimeout[1]failed once on CI (job 92733771394) with a 35sConditionTimeoutExceptionwaiting forisClosedOrFailed. The run reports success because attempt 2 passed on the identical tree.PC was healthy the whole time. The test waits 35s for a consequence whose trigger it never guaranteed. The trigger is a
TimeoutExceptionfromProducerManager.acquireCommitLock, and it fires only while the slow record holds the produce lock and the controller is inside its commit-lock acquisition. Two independent things prevent that overlap, and neither is a stall or a tight assertion:sleep - commitLockAcquisitionTimeoutof margin - and none at all when the attempt arrived a commit-interval later. After the sleep ends there is no contention left, so there is exactly one chance and ~33s of dead time.maybeAcquireCommitLockis gated onwm.isDirty(), whose sole setter isPartitionState#onSuccess(onFailureis a no-op). If every other record of the batch succeeds before the slow one starts, nothing marks dirty during its hold, no commit is attempted, and no timeout can fire.requestCommitAsap()cannot override this -isDirtyis AND-ed into the gate.Widening a timeout cannot fix case 2: there is no deadline to widen.
docs/plans/2026-08-03-001§5 records timeouts already being widened once on this same lock, and the flake surviving.The fix
The test creates the overlap instead of hoping for it, the way
produceTimeoutalready does on the other side of this lock. A latch onpreAcquireOffsetsToCommit- wired throughPCModule, not around it - times the sleep from the commit attempt, and one record's success is deferred into the lock hold so a commit is always attempted. The 35s await andisClosedOrFailedare untouched, and no assertion is weakened.A new guard asserts the failure came from the attempt made while the lock was held, so a regression that stops the overlap happening fails in one line instead of returning as an occasional 35s flake.
Evidence
Controlled experiment rather than "the fix works":
commitLockAcquisitionTimeout2.5s vs 2s sleepcommitInterval3s - hypothesis refutedThe 6/6 arm carries its own control: param
[2](50s sleep) fired correctly in the same JVM at the same instant, so machine speed is ruled out by construction rather than by argument.Not established: which of the two paths CI actually hit. That job has no DEBUG logging and I will not guess between them - the fix covers both.
Docs
Three lessons, each in its own commit:
AGENTS.md). The original handoff searched for the failing test's name, found nothing, and classified by analogy; grepping the mechanism findsdocs/plans/2026-08-03-001, the only prior investigation into this exact lock, which already had both the ordering invariant and the method. The control-arm method is promoted out of that dated plan becauseAGENTS.mditself says a plan goes stale once its work lands. Carries the two build traps that silently void an experiment (-plwithout-am;surefire:testnot reprocessing test resources).AGENTS.md). It told agentsprobe cleanmeans the fault is in the test. Lag stagnation needs 50 records of lag sustained past 150s; rebalance dwell needs 15s. On a 15-record test that fails in 35s neither detector can fire, so the verdict is vacuous - and it was read as a finding here.docs/inflight/).commitTimeoutreclassified out of the load-tightness family, with the warning aimed atproduceTimeoutand the other members still listed.docs/solutions/test-flakiness/unforceable-trigger-commit-lock-timeout-2026-08-07.md). Promotes the analysis out of the ledger, the same path the drain zombie and the nudge race took once understood. Its most useful section is the seven plausible readings that were wrong and why. The ledger entry shrank to the classification and its warning, pointing here for the mechanism, so the two cannot drift.Cross-linked to
vacuous-await-condition-brokerpoller-backpressure-2026-07-31.mdas a sibling rule, not the same one: that hazard is an await satisfied too early by a vacuous condition (a false pass), this one is an await whose trigger never fires (a false fail). Neither fix would have solved the other, so they stay separate rather than being merged.Prior art, and how it was searched
The original investigation searched open PRs, which is a collision check - the PR that already solved something in your file is by definition merged. Searching merged PRs by the files they touch (not by title keywords - a PR that fixed the same class rarely describes the same symptom) found three that mattered and that no symptom search would surface: #110, the sibling flake on this same
producerTransactionLockfixed four days earlier and the origin of the control-arm method; #86, which introduced the ambient probe whose verdict this work had to qualify; and #98, the backpressure test that only passed by racing its own setup. That lesson is now a rule inAGENTS.md, alongside a pre-merge step to look for other instances of the defect class.That sweep ran. Its most useful result is a negative:
TransactionTimeoutsTest.produceTimeout- same file, same lock, still listed as an open flake - is not an instance, because it latches its trigger and keeps a real margin. The write-up says so explicitly, so nobody "fixes" it the waycommitTimeoutwas fixed. Two relatives that are also not this defect (DrainCloseTest,RetriesTest) are named, along with what was checked and dismissed.One open product-code question, recorded not fixed
Everything else here is test or docs. This is the exception, and the only item that touches main-code correctness, so it should not be skimmed past.
In transactional poll-and-produce, two paths release the same
ProducingLockon the same worker thread -WorkContainer#onPostAddToMailBoxandcleanUpContext'sfinally- and nothing clearsPollContextInternal#producingLockbetween them. A secondunlock()by a thread holding zero read locks should throwIllegalMonitorStateException. It doesn't, and nobody has established why.It was raised and deliberately not chased in
docs/plans/2026-08-03-001-investigate-transactional-commit-flake.md§11, where the attempt to measure it was void:surefire:testdoes not reprocess test resources, so the debug logging never reachedtarget/test-classes. Re-measured here with-am verify, which does: 340 acquires, 340 releases, 0IllegalMonitorStateException. Exactly 1:1, so no double release actually occurs and the "is it silently throwing" half is closed.What stays open: which path is skipped and why, and whether that holds for the error, retry and stale-work branches, which were not separately measured. Recorded in
docs/inflight/bug-producing-lock-double-release.mdrather than fixed here - this lock enforces the EOS invariant and has already produced two problems, so a release path that is correct by accident rather than by construction is how the next one arrives.Also in this PR
CONCEPTS.md(new) - shared domain vocabulary seeded from the area this work touched: the produce/commit lock pair and the rules for when each side may be taken, the parallel-consumption nouns those rules need, and the test-reliability vocabulary. Entries carry no file paths, class names, PR links or config values, so the file can't rot as the code moves. Its flagged-ambiguities tail records the distinction the whole investigation turned on - stall, load-tightness flake and unforceable trigger are three different things that present as the same red test. Indexed inAGENTS.md's "Where things live" table.docs/solutions/workflow-issues/compound-tooling-breaks-in-worktrees-and-forks-…(new) - three ways the knowledge-capture tooling silently misreads this repo, all of which return a clean-looking result: a worktree's basename is not the repo name (a session search matched 0 where the correct name matched 37), a worktree has no repo-root config, and bareghin a fork resolves upstream and reports "no pull requests found" for a branch that has one. That last one would downgrade a correct PR citation to "pending". Relevant here becauseAGENTS.mdmandates worktrees and this is a fork with a live upstream, so both conditions hold on every run.Merge strategy
Rebase-merge. The branch has been re-cut for this: the original history recorded the order things were learned, not separable units -
AGENTS.mdwas revised across five commits and the write-up across four, and two commit messages needed an "and also", which is the repo's own test for non-atomicity.It is now nine commits in which every file lands exactly once, bar
AGENTS.md- which appears four times as four distinct ideas (a taxonomy row, the investigation rules, a correction to the ambient-probe guidance, the pre-merge sweep), not four revisions of one. Each is independently revertable, which the old history was not: reverting the prior-art rule used to mean untangling a commit that also edited the write-up.The re-cut was verified content-identical to the pre-re-cut tip (
git diff 4acbcdeb HEADempty), so history changed and content did not.Squashing is the wrong call here: it would bury the only behaviour change - the test fix - under eight docs commits, and releases after 0.6.0.0 generate their notes from the commit log.
Rebased onto master after #219 landed (it had rewritten
AGENTS.mdand the flake ledger, which made this conflict; a conflicting PR has no merge ref, so only CodeQL could run).Checklist
AGENTS.md(investigation checklist, control-arm method, ambient-probe correction) anddocs/inflight/(reclassification + new open-question entry)TransactionTimeoutsTest.commitTimeoutmade deterministic, plus a guard verified by negative control