Repository navigation
ci(hooks): fetch every remote at session start, and refuse a merge onto a stale branch - #372
Conversation
…to a stale branch A remote-tracking ref is a cache, and a stale one does not error - it answers. `origin/master` being fresh says nothing about `origin/<your-branch>`, which in a repo where several agents run at once moves without you: another session, another machine, or a sweep that touches every open branch in one pass. Measured, on 2026-08-26. A session picked up #205, fetched `origin/master`, and merged it. It never fetched the branch's own ref, two weeks stale since the package-rename sweep pushed five commits to it - so it re-did the rename from scratch, moved 239 files, resolved 43 merge conflicts on top of the duplicate, and found out at `git push`, which rejected the non-fast-forward. The recovery was to discard all of it and rebuild on the published tip. Nothing was looking, at any point, and every command in between reported success. The hook has two arms against that one failure. At `SessionStart` it runs `git fetch --all --prune`, throttled on a stamp file so a session that stops and resumes does not become a fetch loop, so every ref the session goes on to read is real. On `git merge` or `git rebase` it refuses while `origin/<branch>` holds commits the checkout does not, and names them. It denies where the two push hooks only report, and the line is the one `docs/agent-harness.md` already draws: a situation with no wrong answer gets `additionalContext`. Merging onto a ref known to be behind its published tip is not that - the result cannot be pushed without discarding someone else's commits, so there is no outcome the agent wanted. `BRANCH_FRESHNESS_OVERRIDE=1` is the escape hatch, read from the payload rather than the environment because a hook does not inherit an env prefix the agent puts on its own command - testing it the obvious way would look right and never fire. `git push` is deliberately not an arm: git refuses that case itself, legibly, and that refusal is what ended the incident. Ten self-test cases, and the negative controls are the ones that matter: a hook that exits 0 on everything passes an allow-only suite perfectly, which is exactly the shape this one would fail into, since every guard in it exits silent when it cannot answer. So the fixtures build a real origin and clone, assert ALLOW while up to date, publish a commit behind the clone's back, and assert DENY on the same commands - plus a `git commit -m "explain the merge"` that must not fire on the word. The registry self-test caught the doc going stale, as designed. Its number-word table stopped at twelve, one short of the count the doc now has to state, which turned a correct doc update into a failure reading as the doc being wrong; it runs to twenty now, with a note saying why it should not stop at the current count. AGENTS.md gains the rule in one line plus the enforcer's name, because Codex and anything else that reads it get no hooks at all.
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
No new clones introduced by this PR. ✅ jscpd (language-agnostic)
|
🧪🔒 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 |
|
…erride was a substring
Three independent review passes over the first version, every finding below
reproduced against fixtures rather than read off the page. Four were
correctness, and the first two would have made the guard something you switch
off within a day.
IT DENIED THE RECONCILIATION IT ASKED FOR. The deny message says "reconcile with
the published tip", and the guard then refused `git merge origin/<this-branch>`,
the rebase form, and `--ff-only`. The only way to obey the message was to invoke
the override, whose documented meaning is that those commits are to be
abandoned - the opposite. Worse, on `master` the branch's own counterpart IS
`origin/master`, so the commonest command in this repo was denied every time
master advanced. The header claimed that case was excluded; it was not. Merging
or rebasing onto the branch's own tip - by `origin/<branch>`, `@{upstream}` or
`FETCH_HEAD` - is now exempt, which fixes both.
IT TRAPPED A CONFLICTED REBASE. `--abort`, `--continue`, `--skip` and
`--quit` were all denied, so a sibling session pushing mid-rebase left the agent
unable either to finish or to abandon. check-history-rewrite.sh already exempts
exactly these; this had departed from it for no reason.
THE OVERRIDE WAS A RAW-PAYLOAD SUBSTRING. `git commit -m "note
BRANCH_FRESHNESS_OVERRIDE=1" && git merge ...` bypassed the guard, as did the
string merely appearing in the agent-written `description` field - on a guard
whose deny message teaches the agent that exact string. It is now a token of the
parsed command, and an exported variable is honoured too, the way
check-shallow-history.sh honours both forms of its own.
A BROKEN INTERPRETER WENT SILENTLY DEAD. There was no interpreter guard at all,
so a python3 that fails takes out both arms with nothing on screen - the
looks-identical-to-healthy class this hook exists to police. The guard added
first was `command -v python3`, and the new self-test case defeated it
immediately: that is an existence test, and an interpreter that is present and
exits 127 passes it. It now runs `python3 -c ''`, the same probe-do-not-assume
reasoning hook-common.sh already applies to `stat`. Once the branch is PROVEN
behind, a failure to emit the verdict exits 2 rather than degrading into an
allow.
THE THROTTLE STAMP COLLIDED ACROSS CLONES, measured independently by two
reviewers. `git rev-parse --git-common-dir` answers a relative `.git` from a main
checkout and an absolute path from a linked worktree, so every clone on the
machine keyed to the same stamp: one clone's session start suppressed another's
fetch for the whole window, leaving it reading exactly the stale refs this hook
exists to refresh - the guard's own failure mode. The key is resolved absolute.
IT WAS THE MOST EXPENSIVE HOOK ON THE BASH PATH. Measured at several times the
cost of its siblings, because it paid a python3 and a git process before
reaching the bail its own comment said came first. The pre-filter is hoisted and
widened to cover both arms; it may only skip work, never decide, so a payload
merely containing the word falls through to the same logic. Both fetches are now
bounded against a network stall - through git's transport knobs, since
`timeout(1)` is GNU-only and absent here.
Three helpers move into lib/hook-common.sh rather than being copied: the
tokeniser now emits each git invocation's ARGUMENTS as well as its subcommand
(the exemptions above need the flags and refs it was discarding, and
re-tokenising in the caller is what that file exists to prevent);
`hook_git_runs_any` answers "merge or rebase" with one interpreter spawn instead
of two; and `hook_throttle_expired` owns the now/mtime/shape-guard/compare
triad, of which this was becoming the third copy. `hook_stamp_path` is
re-documented as taking a key rather than a branch, since it now has a caller
that keys by path - with the absolute-path trap written next to it.
The self-test grows from ten cases to twenty-seven. Four are regressions that
fail against the version this replaces, and the SessionStart arm - which
produces no output ever, and is therefore the half most able to stop working
invisibly - is driven for the first time: it asserts the tracking ref actually
moves, and that a second start inside the floor does not fetch. Every ALLOW case
is paired with a DENY proving the same path can still fire, because an
allow-only suite is passed perfectly by a hook that does nothing.
|
@claude review this A local Where to push hardest:
Known and deliberate, so please do not re-report unless you think the reasoning is wrong: One thing found and not fixed here, for a separate decision: |
|
Claude finished @astubbs's task in 4m 38s —— View job Review:
|
…t one Both items the review raised, plus the imprecision it suggested documenting. A REDIRECT TARGET COULD SPOOF AN EXEMPTION. `punctuation_chars=True` makes shlex emit `> >> <` as tokens of their own, and the argument walk stopped only at the command SEPARATORS - so `git merge origin/master > out.log` walked its arguments as origin/master, >, out.log. The new hook is the one consumer that reads those arguments, and it tests them for membership of a control-flag set and a remedy-ref set, so a redirect target spelled `--abort` or `origin/<branch>` marked an unrelated merge exempt and the guard stood down. The stop condition is now "every character of this token is shell operator punctuation" rather than a hand-written list of five. That catches the whole family at once, including operators nobody enumerated, and it cannot swallow a real argument: refs, paths and flags all carry at least one non-operator character. `hook_git_subcommands` is unaffected either way - it cuts field one, which shlex guarantees is a clean token. Two fixtures, and they are negative controls in the strict sense: reverting the stop-list alone, with the fixtures left in place, turns both red, and restoring it turns both green. Each names a redirect target that is EXACTLY a control flag or the remedy ref, because that identity is the only way the confusion becomes observable. THE ONE PLACE THAT SWALLOWED ITS OWN STDERR. The exempt/guarded parse discarded stderr while every sibling probe in the same file surfaces it - in a file whose whole argument is that a broken guard must say so. Failing toward ALLOW there is correct and stays (the question is asked before the branch is proven behind, so it is the unprovable tier, not the proven one); being quiet about it was not. `-s ours` IS RECORDED AS AN IMPRECISION, not fixed. `git merge -s ours origin/master origin/<branch>` carries the exemption token and is allowed, and the strategy then discards the content of both parents. It does not reproduce the incident - the merge still records the remedy commits as ancestors, so the push is a fast-forward and nothing is lost at the remote - but the branch's content drops exactly what the exemption existed to admit. An agent does not reach for `-s ours` unprompted, and a strategy-aware check is more machinery than the case earns, so it joins the header's imprecision list where the next person to trip it will find it. The review also confirmed three things this branch had reasoned about but not proven: the octopus `git merge origin/master origin/<branch>` is correctly exempt (the merge does incorporate the remedy commits, so HEAD is not missing them); failing toward ALLOW on the exempt parse sits in the right tier of the documented two-tier policy; and the SessionStart fixture cannot pass for any reason other than the fetch having run, because the publish goes through a separate clone and touches nothing in the checkout under test.
|
Both actionable items fixed in bd17390, and the third recorded rather than fixed. Taking them in the order you raised them. 1. The arg-walk stop-list ( Two fixtures, and they are negative controls in the strict sense: reverting only the stop-list while leaving the fixtures in place turns both red, and restoring it turns both green. Each names a redirect target that is exactly a control flag or the remedy ref, because that identity is the only way the confusion becomes observable: 2. The swallowed stderr on the exempt/guarded parse. Fixed — 3. On the three you confirmed — the octopus exemption, the fail-toward-ALLOW placement, and the SessionStart fixture — thank you for tracing those rather than accepting them. The SessionStart one in particular was a claim I had made and not proven; your trace (the publish goes through a separate clone and never touches the tracking ref in the checkout under test, so only the fetch can move it) is the argument that was missing. On the One correction to the record, since you noted the sandbox stopped you executing the tokeniser: the claims above were run, not traced. The suite is |
…check #372 adds `check-branch-behind-its-own-remote.sh`, registered on both `SessionStart` and `PreToolUse`. Two conflicts, and both are the same underlying thing: two branches extending one arithmetic check without knowing about each other. The counts. Master says twelve scripts across fourteen registrations, this branch says twelve across thirteen; both were right before the merge and neither is right after it, because each side added a script and the registrations the other never saw. Recomputed from the merged `settings.json` rather than picked between: thirteen scripts across fifteen registrations, which the self-test now agrees with. The word map. Both sides independently hit the same wall - a count past `twelve` reads as a mismatch rather than as a word the map does not know - and both extended it to twenty, so the map itself merged clean and only the comment explaining it conflicted. Master's phrasing is kept because it states the general failure; this branch's contribution was the literal symptom, which is worth keeping because it does not look like a missing entry: "says thirteen registrations; settings.json has 13" names a disagreement between two numbers that are equal. `bin/test-check-agent-hooks.sh` passes, including master's new arms for the fetch throttle, and the `PostToolUse` bullet this branch added still reads correctly - #372's registrations are on `SessionStart` and `PreToolUse`, so `after-push-check-ci.sh` is still the only one on its event. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
…he sweep tool a mode that could have caught it (astubbs#258) * docs(upstream-map): track the 28 issues closed by the 2023 admin sweep Upstream ran two bulk clearouts before going quiet, and neither was a triage: 2023-06-15 `eddyv` closed 35 unmerged PRs with "Closing - Stale." (34 of them ours), and 2023-07-07 `johnbyrnejb` closed 28 issues with "Closing Issue" - every one marked COMPLETED rather than "not planned". That state reason is why the cohort was invisible. GitHub renders `completed` as resolved, so the issues read as done at a glance, and `upstream-sweep.sh` searches `updated:>=last_swept`, so anything last touched in 2023 can never appear in a sweep. None of the 28 were in this manifest; the mirroring that produced the other 78 fork mirrors only ever sampled *open* upstream issues. All 28 were re-read in full - bodies plus all 58 comments - and each verified against fork source rather than trusted from its thread. That mattered: four issue bodies point at PRs that "fix" them (astubbs#372->astubbs#390, astubbs#319->astubbs#270, astubbs#203->astubbs#345, astubbs#191->astubbs#346) and every one of those PRs was itself swept unmerged. Result: 2 already fixed (astubbs#41 offset-scan removal, astubbs#319 shutdown CME), 6 partly addressed, 20 fully open. Grouped into work items rather than 28 near-duplicate entries, since several want designing together - per-topic handlers with separate consume/produce types, seek with subscription changes, retry expiry with stall detection. astubbs#57 folded into the existing log-noise entry. Swept PR head commits were checked and are all still reachable, so the drafts are recoverable; entries cite branch and SHA, not just the PR number. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * docs(upstream-map): mirror the whole 2023 cohort, not the parts we rate The point of mirroring is a clean, known cutoff - "every issue upstream closed administratively is accounted for here" - not a curated pick of the good ones. A set filtered by our own value judgement is not a cutoff: the next person cannot tell "not mirrored because it was junk" from "not mirrored because it was missed", and that ambiguity is worth far more than the cost of carrying a few items nobody will ever action. Drops the decline recommendations from astubbs#53, astubbs#199, astubbs#246 and astubbs#57 in favour of neutral statements of the trade-off, and renames sweep-2023-decline-candidates to sweep-2023-small-items (status wontfix -> none). The facts that informed those recommendations are kept - they are useful to whoever picks the issue up - but they are now framed as input to that person's decision rather than as our verdict delivered in advance. Fork issues can always be closed later on their merits, which leaves a visible record; pruning before mirroring does not. Records the principle in the section header so a later pass does not re-prune. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * docs(upstream-map): link the created fork mirrors back to their entries The 28 mirrors are live as astubbs#227-254 (label `upstream-admin-closed`), with confluentinc#41 -> astubbs#233 and confluentinc#319 -> astubbs#252 created and immediately closed as completed, carrying the code evidence that they are genuinely fixed. Records the fork numbers on each entry so the mapping is queryable rather than re-derived, which is the reason this file exists. Adds `fork.fork_issues` (the plural of the existing `fork_issue`) because several entries deliberately group upstream items that want designing together, and documents it in the schema block. Verified: all 28 mirrors are referenced, none missing, no strays. Also snapshots each upstream title verbatim into its mirror body header, dated 2026-08-07 - the fork titles may drift as these are worked, and the original wording is what makes an old thread findable. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * fix(tooling): give upstream-sweep a mode that can see closures it never saw The default sweep is a window search - `updated:>=$since` - so it structurally cannot see an item whose last activity predates the window, and `last_swept` only moves forward, so the blind spot grows. That is why upstream's 2023 administrative closures went unnoticed for three years: 28 issues closed with "Closing Issue" and 35 unmerged PRs closed as "Closing - Stale.", all marked COMPLETED, none of it triage, none of it visible to us. Adds `--audit`: no time window, asks "which closed upstream items are neither tracked in the manifest nor mirrored in the fork?", and flags days where an implausible number of things closed at once. Run against the real repo it rediscovers both sweeps from scratch and reports zero unaccounted, which is also an end-to-end check that the new fork_issues linkage is correct. Bots are excluded from the PR analysis. Dependabot self-closes superseded bumps in batches that look exactly like a sweep - 2022-08-16, 2022-10-20, 2023-11-03 and 2024-01-25 are all dependabot - and unfiltered they buried the two real sweeps in noise. Filtering them surfaced two human PRs that had been sitting inside those batches (confluentinc#508, confluentinc#650), now recorded for review. The report says outright that a bulk day is not proof of a sweep, because a release triage looks identical from here, and that stateReason COMPLETED is not evidence of a fix - that assumption is what made this cohort invisible. Known remaining hole, recorded not fixed: a PR closed alone on a quiet day is still invisible, since detection keys on bulk. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * fix(tooling): teach the audit about Discussions, a content type we never checked Discussions were invisible to everything: not in the manifest, not mirrored, not in any sweep mode. 74 of them upstream, and nothing we run would ever have mentioned one. They were NOT swept - no bulk closure, and the largest day (2024-04-02, six threads) is all answered=true housekeeping. So the failure mode is different from the 2023 cohort: not administrative closure, just questions nobody answered, in a place no issue search reaches. Handling differs accordingly - these want answering or converting, not mirroring wholesale. The audit now lists zero-reply discussions, excluding release-announcement threads by title: those legitimately have no replies, and counting them as neglect would put 9 false positives at the top of the report. Two finds justify the check on their own. Discussion 542 (zero replies) is a field report of a transactional consumer stuck in rebalance because it cannot acquire the produce lock, with the reporter suspecting the revoke-time flush - the same lock lifecycle as bug-producing-lock-double-release, and they raised the timeout without effect, which is evidence about the mechanism rather than the duration. Discussion 883 (zero replies) is the same complaint as fork mirror astubbs#187 but with a runnable reproducer, which astubbs#187 lacks. Upstream-Issue: confluentinc#154 Forwarded: no Applied-Upstream: no * docs(inflight): record the open obligation to account for every upstream item Two notes, both about what is NOT done. Discussions are not a queue to be converted. There is no pipeline turning threads into issues, and most should never become one. The rule is a judgement: if reading a discussion makes us think there is an issue, we raise one - a normal fork issue on its own merits, citing the discussion as where it came from. It exists because we believe the problem is real, not because a thread existed. That also keeps `upstream-mirror` meaning exactly "an upstream issue we carry", which is what makes the 2023 cutoff verifiable. The wider obligation is now written down: every upstream issue, PR and discussion must be accounted for - carried, declined, or genuinely resolved upstream - not sampled. The 2023 cohort was found only because it was a *bulk* event; `--audit` keys on bulk closures and zero-reply threads, and both are proxies that miss whole shapes of problem (a PR closed alone on a quiet day, a discussion with one dismissive reply, an issue marked COMPLETED with no linked PR). The audit narrows the field; only reading discharges the obligation. Also records what has been ruled out, so it is not re-investigated: wiki disabled, no advisories, all open-milestone issues already mirrored, the orphan branches accounted for (v0.6.x-dev is the lambda-actor work already captured via swept PRs; 0.5.3.x's regression fix IS on master as a908e16), and "upstream pushed today" being a pushed_at artefact rather than new activity. Left open: project boards need a read:project scope we do not have, and 169 forks are unexamined. * fix(upstream-map): use a group the schema actually allows Two new entries used `group: batching-ordering`, which is not in the GROUPS allow-list in scripts/upstream-map.py, so `upstream-map.py validate` failed on them. Caught in review on astubbs#258. My mistake was validating the wrong way: I checked the file with an ad-hoc yaml.safe_load plus a duplicate-id check, which passes happily on a group the schema rejects, instead of running the project's own validator that AGENTS.md tells contributors to run before committing manifest changes. Reassigned both to `features` rather than widening GROUPS. The rest of this cohort's feature entries already use `features`, so this keeps the cohort internally consistent, and the ordering/batching distinction is not lost - it lives where it belongs, on the issues themselves via the `area/batching-ordering` label (astubbs#244, astubbs#236). Adding a vocabulary term to a shared schema as a side effect of a mirroring PR is the larger, less reversible change. Also drops the redundant 345 from sweep-2023-broker-disconnect-commit's `related` list, where it already appears in `prs`. Verified: `upstream-map.py validate` reports OK on 27 entries, and `refs` and `table` both still render. * docs(upstream): graduate the durable parts of the sweep record out of inflight The three inflight notes stay - they hold undischarged work - but they were also carrying permanent content, and inflight is transient by its own charter. Moved to durable homes, each citing the manifest rather than restating it: - docs/upstream.md gains the upstream-admin-closed cohort note (with the "do not trust 2023-era closure states" rule), the discussions non-mirroring policy decided 2026-08-07, documentation of --audit and its known blind spots, and the ruled-out upstream surfaces (wiki, advisories, milestones, orphan branches, pushed_at). - docs/solutions/ gains the transferable lesson: a closure state is a rendering choice not a triage, "fixed by #N" must be checked against the merge bit, windowed watchers need a no-window audit, and bots must be filtered before hunting bulk events. - upstream-pr-analysis.adoc is corrected: it claimed issues were never bulk-closed, which the 2023-07-07 sweep disproves, and the PR sweep is now confirmed rather than suspected. The inflight notes now hold only open work, pointing at the new homes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * docs(upstream): name the analysis doc's role (the plan) beside the manifest (the state) docs/upstream.md's opening enumerated everything it owns except the editorial analysis, and neither source-of-truth reference was a clickable link. Now the two roles are named explicitly: the .adoc is the plan, the manifest is the state tracker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * fix(upstream): make the audit's discussion query actually paginate, and correct the sweep cohort Address PR review feedback (astubbs#258). - upstream-sweep.sh: `gh --paginate` only substitutes a variable named `endCursor`; calling it `$cursor` meant the cursor was never fed back and gh re-requested page 1 forever (reproduced: 469 identical pages at first:2). It only looked healthy because 74 discussions fit in one page of 100. The failure was also swallowed by `2>/dev/null || true`, which would have printed a clean "(none)" for a broken query - the one answer this mode must never invent. Fail loudly instead. - upstream-map.yaml: the sweep cohort listed 36 PRs against a documented count of 35. Upstream closed exactly 35 unmerged PRs on 2023-06-15; confluentinc#66 (closed 2022-10-19) was carried in by mistake. Listing a ref here marks it accounted for, so it was hiding an unrelated PR from every future audit. Recorded PR 258 and moved the entry to `pr-open` per the lifecycle rule. - next-upstream-coverage-completeness.md: dropped the cached counts - inflight notes must not record what a command can answer, and the "~100 partially mirrored" line already contradicted docs/upstream.md, which says all 78 open upstream issues are mirrored with backlinks. Kept only what the audit cannot say: what nobody has read yet. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(agents): let the manifest cache frozen upstream-issue facts, and say why The rule said "the manifest tracks upstream PRs only", so mirroring the 2023 sweep cohort into it read as a violation. The rule is what is out of date. An archived upstream's closed issue numbers and closure events are frozen. Caching them locally is a read-path optimisation, not a second tracker: grepping one file is instant, while the same answer from the mirrors costs dozens of API round-trips and burns rate limit shared across agents. The usual objection to duplication - the copy silently diverges - needs a source that can still move, and this one cannot. So the boundary is redrawn where it actually bites: the mirror owns an issue's *live state* and remains the only place you update it; the manifest may cache what is frozen (number, cohort, owning mirror). Cache what is frozen; never mirror what is moving. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(upstream): put the manifest-caching rule in the doc that owns it The previous commit wrote the rule into AGENTS.md, which is the one place it does not belong. AGENTS.md's own preamble calls itself a router - "if it only matters once you are already in a topic... it goes in that topic's doc" - and docs/upstream.md line 7 already claims ownership explicitly: "AGENTS.md carries only the pointer and the one-line rule that manifest upkeep is the agent's job." The rule was in fact already stated twice before this branch (AGENTS.md:442 and docs/upstream.md:23), so growing the AGENTS.md copy deepened a fragmentation that was already there. Now stated once, where an agent editing the manifest will actually be looking, with AGENTS.md left holding the pointer and the one binding rule whose failure is silent (keep the fork side in sync). Net effect on the router: one line shorter than master, despite covering more. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * docs(agents): make a pointer say who owns the rule, not just what is nearby Diagnosed from a live miss. A reviewer cited AGENTS.md's "the manifest tracks upstream PRs only", so that section was read and treated as authoritative - while docs/upstream.md had been stating the same rule, and claiming ownership of it, the whole time. The duplicate was then grown in the router rather than fixed at source. Nothing in the reading path revealed the mistake. AGENTS.md stated a complete-looking rule and followed it with a pointer listing four adjacent mechanics ("the manifest schema, the mirrors, the commit trailers and the upstream sweep"), which reads as *further detail*, not as *this is a summary and that doc wins*. The contract was written down - docs/upstream.md:7 - but only on the side nobody enters from, since AGENTS.md is what loads every session. The anti-duplication rule already existed here ("Never state a fact twice"). What was missing is the half that makes it discoverable: a cross-reference must name the owner and say the owner wins, so a reader can tell a stub from the whole rule before editing the copy. Applied to the rule itself and to the upstream pointer that failed. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 participant
Description
A remote-tracking ref is a cache, and a stale one does not error — it answers.
origin/masterbeing fresh says nothing aboutorigin/<your-branch>, which in a repo where several agents run at once moves without you: another session, another machine, or a sweep that touches every open branch in one pass.Measured, on 2026-08-26. A session picked up #205, fetched
origin/master, and merged it. It never fetched the branch's own ref, two weeks stale since the package-rename sweep had pushed to it — so it re-did the rename from scratch, resolved the resulting merge conflicts on top of the duplicate, and found out atgit push, which rejected the non-fast-forward. The recovery was to discard all of it and rebuild on the published tip. Every command in between reported success.What lands
.claude/hooks/check-branch-behind-its-own-remote.sh, two arms against that one failure:SessionStart—git fetch --all --prune, throttled on a stamp file, so every ref the session goes on to read is real.PreToolUseongit merge/git rebase— refuses whileorigin/<branch>holds commits the checkout does not, and names them.It is the mirror image of
remind-master-drift-on-push.sh: that one watches the base moving under you, this one watches your own branch moving under you.Three helpers move into
lib/hook-common.shrather than being copied: the tokeniser now emits each git invocation's arguments as well as its subcommand (the exemptions below need the flags and refs it was discarding);hook_git_runs_anyanswers "merge or rebase" in one interpreter spawn; andhook_throttle_expiredowns the now/mtime/shape-guard/compare triad this was becoming the third copy of.Why it denies, and what it must never deny
docs/agent-harness.mdalready draws the line: a situation with no wrong answer getsadditionalContext, a violation gets adeny. Merging onto a ref known to be behind its published tip is not a judgement call — the result cannot be pushed without discarding someone else's commits.But a guard that blocks its own remedy gets switched off, so two things are exempt:
origin/<this-branch>,@{upstream},FETCH_HEAD. Without this, obeying the deny message required invoking the override, whose documented meaning is the opposite; and onmasterit deniedgit merge origin/masterevery time master advanced.--abort,--continue,--skip,--quit.BRANCH_FRESHNESS_OVERRIDE=1is the escape hatch, honoured as an exported variable and as a token of the parsed command — never as a raw-payload substring, which any prose naming the variable could satisfy, on a guard whose own message teaches that string.git pushis deliberately not an arm: git refuses that case itself, legibly, and that refusal is what ended the incident.Four known imprecisions are listed in the header rather than left to be rediscovered, including that
-s ourssatisfies the remedy exemption while discarding the content it was meant to admit.How it was checked
Three local reviewers (reuse, quality, efficiency) and one on the PR. Between them they found that the guard denied its own remedy, trapped a conflicted rebase, took a substring for its override, went silently dead on a broken interpreter, shared one throttle stamp across every clone on the machine, cost several times its siblings on the hot Bash path, and let a redirect target spoof an exemption. All fixed here.
The self-test drives both arms, including the
SessionStarthalf that produces no output and is therefore the part most able to stop working invisibly — it asserts the tracking ref actually moves, and that a second start inside the throttle does not fetch. Every ALLOW case is paired with a DENY proving the same path can still fire, because an allow-only suite is passed perfectly by a hook that does nothing. The regression cases were verified as true negative controls: reverting the fix alone, fixtures in place, turns them red.One fix caught another: the interpreter guard was first written as
command -v python3, and the new self-test case defeated it immediately — that is an existence test, and an interpreter present but exiting non-zero passes it. It now runspython3 -c ''.AGENTS.mdgains the rule in one line plus its enforcer's name, because Codex and anything else reading it get no hooks at all.Left for a separate decision
command -v <tool>as an existence-rather-than-works check is not unique to this hook. The same form is ininject-branch-context.sh,pre-commit-gate.sh,check-history-rewrite.sh,check-merge-outstanding-work.sh,check-shallow-history.sh,remind-inflight-on-push.shandbin/check-pr-analysis-surfaces.sh— and the identical class is already written up fornodeinbin/test-check-file-refs.shandbin/test-check-issue-refs.sh. The two DENY guards are where a silently-dead guard actually costs something. Out of scope here on purpose.Checklist
docs/agent-harness.mdgains the hook's entry, its overrides and its registration counts;AGENTS.mdgains the one-line rulebin/test-check-agent-hooks.sh, negative controls includedce-simplifylocally (three reviewers; their findings are the second commit) and asked for@claude review thison the PR rather than a localce-code-review