feat: adopt the backlog store tooling and close its three upstream-fixed rows (Q889) - #1595
Merged
Conversation
scripts/agent/pr-mergeability-watch.sh was written here, the session-orchestrator skill ported it to Python, and the port then fixed defects the shell original still carried. Nothing reported the divergence, which is the cost this repo pays for forking a tool upstream keeps developing. The Python is vendored byte-identical and is not edited here, so the next upstream fix lands as a clean overwrite. The .sh becomes a three-line exec supplying --gate 'make check' and forwarding the rest, which keeps every call site and doc reference that names the .sh path working and gives repo defaults exactly one home. The predecessor's tests could not be carried over: they stubbed the sleep binary, which time.sleep never calls, so every case would have slept for real. The suite now drives the module with an injected runner and sleeper. It also restores the one assertion upstream's suite lacks. BLOCKED means a failing check or an outstanding review rather than a conflict, so a watch that woke on it would fire for every red PR in a batch; upstream covers CLEAN and UNKNOWN but never BLOCKED. Shown failing against a build that treats BLOCKED as a conflict before being trusted to pass.
Closes Q694, Q814 and Q828, whose fixes were already written and tested upstream while this repo's shell copy carried the defects. Q694: the reads are --json parsed in-process, so a stub cannot bypass the parsing the way a post-filter stub did, which is how a malformed read reached production instead of the suite. Q814: the local merge probe runs before the eligibility checks, so an ordinary DIRTY wake still records its OIDs. The cheapest-first intent survives, since only the local half moved and the paginated timeline read stays behind the checks. Q828: an unrecognised record reports as version skew naming both versions, not as an empty verdict that reads like corruption. It also derives the driver-owned paths and driver names from .gitattributes rather than the two hand-kept arrays the shell carried under a comment asking they be kept in step. A stale list under-reports, and under-reporting is the verdict that lets an unattended enqueue through. Vendored byte-identical, with repo entry via a forwarding .sh, on the same reasoning as the mergeability watch: the fork is what let these three sit unfixed here after upstream closed them. One property did not come across. A probe that cannot run now records nothing, where the shell wrote an UNMEASURABLE verdict, so an absent record no longer distinguishes ran-and-could-not-measure from never-ran. --confirm still fails closed, so the loss is diagnostic; filed as karlkfi/claude-skills#129 rather than patched here, and parallel-dispatch.md now states what the record can and cannot tell you. The suite gains two controls upstream lacks. Q834's coverage there is carried by the fixture being parked on main rather than by a named case, so a mis-set fixture would pass for the wrong reason; the controls assert the checkout really is parked and really does merge clean, and pointing the probe at the parked head turns the conflict case red.
The rule exists because a relocated row and a deleted row are indistinguishable to a line-position merge, so a branch that reorders a row while main deletes it merges clean and silently resurrects it. Measured both arms in throwaway repos, git 2.55.0. Single table, with the relocation far enough from the deletion to clear the diff context: exit 0, clean merge, row back. Per-item store, a rank edit against a file deletion: exit 1, CONFLICT (modify/delete). One file per item makes the two a modify and a delete of one path, which git refuses rather than resolves, so the silent default the rule was built for is gone. What is left is a careless resolution of a loud conflict, which the existing hand-resolution reconciliation habit already covers. The first control run reached the opposite conclusion and was wrong: a four-row fixture put the relocation inside the deletion's diff context, so the table arm conflicted too and the comparison read as no difference. Disbelieving it took knowing the repo has met the silent case twice already (#766/#768, #805); reproducing it needed twenty rows and a relocation from position 18 to the top. Also corrects two things the survey disproved rather than the measurement: the claim allocator is not vendored either, because this repo's is upstream of the skill's (which cites this repo's 460+ live claims as its own proof point) and backloglint rule 12 already enforces what queue.py claims does.
Check what the local version guarded before adopting an external port. Both adoptions so far were better than the copy they replaced in every measurable way and still dropped something local: upstream's mergeability watch suite never exercised BLOCKED where ours did three times, and the re-enqueue port stopped recording a verdict for a probe that could not run where ours wrote UNMEASURABLE. Neither is visible from reading the port, only from diffing its guarantees against the local one's. queue.py and the rules checker are both still to adopt. A reading of an upstream file ages, so re-read before acting. The audit that opened this work was correct when taken and obsolete 35 minutes later, when an upstream merge retired a section it had deliberately kept. A reading is a snapshot of someone else's repository rather than a measurement that can be re-run, so nothing signals the decay. Five phases here rest on reading files this repo does not own.
Flake-watch rows migrate as items with status: deferred and a flake label, keeping the **Event:** recurs on main after the fix trigger. Deferred is the store's parked-awaiting-a-trigger state. Blocked means waiting on a dependency, and a flake row is waiting on a recurrence that is expressly not expected, so blocked would have added every one of them to the set a groom reads each pass for no signal.
queue.py and its rank vectors, byte-identical from the session-backlog skill, with both suites adapted to this repo's conventions and wired into SCRIPTS_TESTS. The store does not exist yet, so nothing calls the tool: this carries and gates it ahead of phase 2 rather than switching anything. Re-reading upstream before vendoring earned its place immediately. The copy sized earlier was 877 lines; the current one is 884, because b0330e0 fixed rank_between minting a key check_rank rejects on a head insert below the second-smallest integer. Head insertion is the flakes-first path this repo uses constantly, so the older copy would have carried that into a migration of 176 items whose order is the priority. One test case is skipped by design and says so. It exercises queue.py claims against the allocator, and this repo's alloc-queue-id.sh claims through gh api where the skill's uses git push, which is the path the skill itself names as proven here at 460+ live claims. A local bare remote serves only the push form, so the fixture reaches the wrong half of the mechanism and the allocator correctly emits nothing. Reporting a skip beats a vacuous pass; QUEUE_TEST_ALLOCATOR_FIXTURE=push runs it where a push-form allocator exists. That skip is also why the suite would not run under this repo's required errexit prologue: the allocator's non-zero exit inside a command substitution killed the script silently after 39 checks, which read as '-e is incompatible' until the call was guarded. One cause, two symptoms. 114 checks across the two suites, inside make scripts-test.
… (Q889)
Rules 8, 9 and 11 are each a function of what the branch changed rather
than of what the store holds, which is why queue.py lint has no
equivalent: it is a pure function of a directory.
8. A flake item may not vanish. A shipped mitigation retires it to the
ledger; deleting it throws away the memory that a fix was already
attempted, so a recurrence reads as a fresh find.
9. Deleting the last item targeting a plan obliges that plan's index row
to stop reading as open work.
11. Every label is declared in docs/queue/README.md, so a typo cannot
stick silently.
Rule 10 is deliberately absent, dropped on the measurement recorded in
the plan doc: one file per item turns a relocation against a deletion into
a modify/delete conflict git refuses, so the silent default it guarded is
gone. Rule 12 is queue.py claims.
The baseline is the merge base with origin/main rather than its tip. An
item main deleted while this branch was behind is absent from the tip and
present at the base, so a tip-keyed check would read it as one this branch
removed and demand the ledger for finished work.
Two refusals rather than verdicts: a read it cannot take exits 2, and an
absent store reports the count it checked rather than ok, since a clean
bill of health for a store never read is the failure this whole session
kept meeting.
10 checks, each rule in both directions with its own control: an ordinary
item must still delete freely, and a plan keeping another live item must
not fire. Without those the rules are equally consistent with refusing
every deletion and demanding a flip on every close.
docs/queue/** is deliberately absent from the workflow's path filters
until the phase-2 cutover; check-path-filters rejects a pattern matching
nothing, and the comment there says when to add it.
queue.py migrate was run against the live table and reconciled, then the output discarded rather than committed. Committing it would have shipped a tree whose own gate fails: rule 11 reads the label vocabulary from docs/queue/README.md, which does not exist yet, so the checker exits 2. What the run established: 173 items, ID sets reconciling in both directions against the table's 174 anchors, the one difference being Q248, which the migration correctly left behind because it is an item ID on a Progress row rather than a Queue row. Order is preserved end to end, the Queue's 106 rows being the store's first 106 in order and the remaining 67 matching Deferred plus Flake watch in order. That is the check worth running, because the order is the priority and a count cannot see it scrambled. Three probes were written before one measured the right thing. Comparing render --all against the Queue section interleaves deferred items by rank, and matching Q-IDs over the rendered table catches ones quoted inside Notes text, so Q811 cited in Q871's note read as a reordering.
status-lint-gate failed on #1595 with 'No rule to make target queue-rules-check' against a tree that carries the target, runs it locally, and passes gate-lists-check. Rather than chase that, the step now runs a script, which is what every other step in that job already does. The wrapper is not incidental. Every gate here is a scripts/ file, and the Makefile recipe, the workflow step and gate-list.sh's own derivation all key on that: pointing the recipe at 'python3 <file>' satisfies none of them, and gate-lists-check says so, reporting that the gate runs no scripts/ file and therefore gates nothing on a PR. It also brings the gate under the shell linter and the errexit prologue check. Which promptly earned itself: shellcheck rejected the wrapper's own header, because a comment line beginning '# shellcheck' parses as a directive.
The step was in status-lint-gate, which is the aggregate required-status job: it reports whether the other jobs passed and has no actions/checkout, so a step there sees no repository at all. That is one cause wearing two faces, and the first face sent me the wrong way. As `make queue-rules-check` it failed 'No rule to make target' on a tree that carries the target, runs it locally and passes gate-lists-check, which read as a make problem. Rewriting it as a script turned the same absence into exit 127, 'No such file or directory', which is the error that names it: there was no Makefile either. It now sits in lint-status beside the other three lint steps, which is where the checkout is. The scripts/ entry point added chasing the first face stays. gate-lists-check requires it independently: a recipe reading 'python3 <file>' runs no scripts/ file, so the gate reports it gates nothing on a PR. It also brought the wrapper under the shell linter, which caught its own header.
Re-ran queue.py migrate against the live table. The flake-watch handling needed no work: all 29 rows arrive as status: deferred carrying flake, which is decision 4 satisfied by migrate itself. What does need work is the title cap. 61 of the 173 items have titles over the store's 72-character cap, the longest at 130, and queue.py lint fails on every one, so the store cannot land until they are rewritten. This is not a migration defect. The single table capped the Notes cell at 250 characters and never capped the Item cell at all, so adopting the store imposes a constraint this backlog has never been held to. The cap is deliberate upstream: a title renders whole in every index row, in next's kickoff prompt, and in any session named after the item, so it is the one field with nowhere to overflow. Rewriting them is judgement rather than truncation. Cutting at 72 characters severs titles mid-identifier, and moving the tail into the body leaves a title that no longer says what the item is. Q490 is fixed by hand as the worked shape, 80 characters down to 64 with the spec name moved into the body where it costs nothing. Also: v2.0.0 is not a label. A regex over the **Labels:** line reads it as one because it is backticked link text inside 2.0-gate's parenthetical, so the derived vocabulary is 18 labels rather than 19.
The cap stays. Its reasons are this repo's reasons too: a title renders whole in every index row, in next's kickoff prompt, and in any session named after the item. Both alternatives cost more than they save. Raising it means either patching the vendored queue.py, forking the file phase 1 spent its whole length un-forking, or duplicating the check locally. An allowlist of 61 IDs is how a cap stops meaning anything. So phase 2 is two pieces of work and the titles are the larger: rewrite the 61 first, then migrate. The other order edits item files the migration is about to overwrite.
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.
What & why
Phase 1 of Q889: adopt the
session-backlogtooling and the two agent scripts, and gate everything, without touching the backlog format. The store itself is phase 2 and is deliberately not in this PR.Closes Q694, Q814 and Q828, whose fixes were already written and tested upstream while this repo's forked shell copies carried the defects.
The fork was the defect
pr-mergeability-watchandpr-requeue-eligiblewere written here, the skills ported them to Python, and the ports then fixed things our copies still had. Nothing reported the divergence, so three rows sat open with their fixes twenty feet away.Both are now vendored byte-identical and never edited here, entered through a forwarding
.shthat keeps every call site and doc reference working and gives repo defaults one home. That is what makes the next upstream fix a clean overwrite instead of a merge against local edits.The same discipline caught two gaps the ports had, both found by diffing the port's guarantees against the local one's rather than by reading the port:
BLOCKEDwas untested upstream. It means a failing check or an outstanding review, not a conflict, so a watch that woke on it would fire for every red PR in a batch. Our shell suite covered it three times. Restored, and shown failing against a build that treatsBLOCKEDas a conflict before being trusted. Filed as claude-skills#124.UNMEASURABLEwas dropped. A probe that cannot run now records nothing, where the shell wrote a third verdict, so an absent record no longer distinguishes ran-and-could-not-measure from never-ran.--confirmstill fails closed, so the loss is diagnostic. Taken as-is rather than patched, to keep the vendor clean; filed as claude-skills#129.parallel-dispatch.mdnow states what the record can and cannot tell you, replacing a sentence that merged in docs(development): cut the agent process playbooks to repo deltas (Q886, Q887) #1587 and became false.Two rules measured out of scope rather than built
Rule 10 is not ported. It guards a relocated row outvoting a deleted one, which a line-position merge cannot tell apart. Measured both arms, git 2.55.0: the single table merges clean at exit 0 and silently resurrects the row; one file per item gives
CONFLICT (modify/delete)at exit 1. The silent default is gone, and the residual belongs to the existing hand-resolution reconciliation habit rather than to a new gate.The first control run said the opposite — a four-row fixture put the relocation inside the deletion's diff context, so both arms conflicted and the comparison read "no difference". Disbelieving it came from knowing this repo has met the silent case twice (#766/#768, #805); reproducing it needed twenty rows.
The claim allocator is not vendored. Ours is upstream of the skill's, which cites this repo's 460+ live claims as its own proof point, and
backloglintrule 12 already enforces whatqueue.py claimsdoes.What landed
scripts/agent/pr-mergeability-watch.py+.shwrapper + suite (22 checks,BLOCKEDrestored)scripts/agent/pr-requeue-eligible.py+.shwrapper + suite (34 checks, two Q834 controls added — upstream covers it by fixture rather than by name, so a mis-set fixture would pass for the wrong reason)scripts/docs/queue.py+rank-vectors.tsv+ two suites (114 checks)scripts/docs/check-queue-rules.py+ suite (10 checks) — rules 8, 9 and 11, whichqueue.py lintcannot express because each is a function of what the branch changed rather than of what the store holdsqueue-rules-checkinCHECK_FAST_GATES, three suites inSCRIPTS_TESTS, a CI step instatus-lint.ymlRe-reading upstream before vendoring earned its place immediately: the
queue.pycopy sized earlier in the session was 877 lines, the current one 884, because a fix landed forrank_betweenminting a key its own checker rejects on a head insert — the flakes-first path this repo uses constantly, going into a migration of 176 items whose order is the priority.Phase 2 is measured but not landed
queue.py migratewas run against the live table and reconciled, then discarded rather than committed. Committing it would ship a tree whose own gate fails, since rule 11 reads a vocabulary file that does not exist yet.It established: 173 items, ID sets reconciling both ways against the table's 174 anchors, the one difference being
Q248— an item ID on a Progress row, which the migration correctly left behind and which the plan had already flagged as needing a decision. Order is preserved end to end: the Queue's 106 rows are the store's first 106 in order, and the remaining 67 match Deferred plus Flake watch in order.How it was tested
make checkgreen over the final tree, read by sub-gate name rather than exit code:gate-lists-check(35 fast gates, 86 suites),script-docs-check(223 scripts),path-filters-check,shellcheck,doc-links,lint-backlog,em-dash-check.180 new assertions across four suites, all inside
make scripts-test. Every new assertion was shown failing before it was trusted: theBLOCKEDcase against a build that wakes on it, the Q834 controls by pointing the probe at the parked head, and each of rules 8, 9 and 11 against its own violation with a control proving it does not simply refuse everything.Four gates rejected an earlier version of this change and each was right —
gate-lists-checktwice (a suite nothing runs, then a gate with no CI home),check-path-filters(adocs/queue/**pattern matching a directory that does not exist yet, so it would gate nothing), andshellcheck.Checklist
make checkis green (plus any heavier gate this change warranted).parallel-dispatch.mdandscripts/README.mdupdated where the ports changed what they describe.)