Skip to content

fix: An epic-child PR's merge ref goes stale when the epic branch moves, and nothing retriggers it - #9379

Merged
usirin merged 3 commits into
mainfrom
build/8880-stale-child-merge-ref-afc92e57
Sep 17, 2026
Merged

usirin merged 3 commits into
mainfrom
build/8880-stale-child-merge-ref-afc92e57

Conversation

@usirin

@usirin usirin commented Sep 17, 2026 •

Copy link
Copy Markdown
Member

An epic run's child PR keeps reporting checks over the old base after the driver pushes the
assembly branch, and nothing in the pipeline retriggers it. This adds fabrika lane retrigger <epic>
and the operate step that runs it right after lane push.

What the platform actually does

The issue offered two readings and asked for the real one. I read it off a live pull request in this
repository rather than off the docs alone.

GitHub DOES rebuild refs/pull/<n>/merge when the base moves. PR #8063's head had not been
pushed since 2026-09-06. Fetching its merge ref on 2026-09-16 gave commit 470decb9, whose first
parent is 2c2a5dc3 — the tip of main that morning, 374 commits past the base.sha the PR payload
still reports (5c26458b). So the merge ref is current; the PR record's own base.sha is the thing
that is frozen.

GitHub schedules nothing for a base push. Every check run on that same head (e715358e) is
stamped 2026-09-06, with hundreds of trunk commits landed since. pull_request fires on opened,
synchronize and reopened, and a base push is none of them
(events that trigger workflows).

So the second reading holds: the ref rebuilds, the event never fires. That also rules out the cheap
lever the first reading would have allowed — a re-run replays the original event's GITHUB_SHA and
GITHUB_REF ("The workflow will also use the same GITHUB_SHA (commit SHA) and GITHUB_REF (git
ref) of the original event that triggered the workflow run",
re-run workflows and jobs),
which is the stale merge commit and therefore the same red.

The verb

lane retrigger <epic> sweeps every OPEN pull request based on epic/<n> and moves the head of each
one that is behind the branch, through PUT /pulls/{n}/update-branch — the base merged into the head
branch, which is a synchronize, so a run is scheduled against a merge ref computed now. Nothing is
closed, nothing is force-pushed, no commit is rewritten, and the preview stage stays up. Close/reopen
is named forbidden in the operate step, with #8881 as the reason.

It is idempotent by construction: staleness is read from the platform's own comparison against the
base as it stands, so a child whose head already carries the base tip is reported current and never
written to. A second call right after a first writes nothing. Each write carries expected_head_sha,
so a head a sibling moved first refuses the write rather than misaddressing it; that 422 is shared
with a real conflict, and the two are split by re-reading the head.

Verdicts on exit 0 are RETRIGGERED, CURRENT and NONE, above one row per child: #<pr> current,
or #<pr> <commits behind> behind, <head before> -> <head after>, where the count is the
comparison's own behind_by — what says whether the base drifted by one commit or a hundred.
Refusals: 8 an accepted update whose head did not move inside its window, or a read that failed
after this sweep had already moved a child's head (UNKNOWN, re-read before writing again), 11 a
list or a comparison unread while the sweep had written to nothing, which is the sweep that is safe
to re-run, 42 a child the assembly branch does not merge into, which is that child's repair round
rather than anything to retry here.

Live smoke against this repository: lane retrigger 8716 answers RETRIGGER-VERDICT: NONE at exit
0, reading one page and writing nothing.

Start with packages/fabrika-cli/src/lane/retrigger-verb.ts, then the operate step beside the
lane push fence.

Deviations

  • Declined guidance — Said: the acceptance criterion asks that close/reopen be named
    forbidden in operate/SKILL.md with Closing and reopening a PR tears down its preview stage mid-deploy and fails the deploy on a deleted D1 #8881 as the reason. Did: the skill names close/reopen
    forbidden and states the reason in words (it tears the PR's preview stage down mid-deploy); the
    Closing and reopening a PR tears down its preview stage mid-deploy and fails the deploy on a deleted D1 #8881 citation lives in the lane retrigger row of packages/fabrika-cli/docs/verb-reference.md
    and in this body. Why: portability-guard reds a repository-specific issue reference inside
    the shipped claude-plugins/fabrika/ corpus, and its plugin-operate row's ceiling only shrinks,
    so the number cannot land in that file. Disposition: stated here; the reason and the citation
    are both reachable, in two files instead of one.
  • Out-of-scope change — Said: the issue names the retrigger. Did: also added three
    adapters to src/io/pulls.ts (openPullsForBase, compareStanding, updatePullBranch).
    Why: the verb reads and writes over the GitHub API and this package's source may not invoke
    gh, so those reads have nowhere else to live. Disposition: stated here.

Fixes #8880

@github-actions

github-actions Bot commented Sep 17, 2026 •

Copy link
Copy Markdown
Contributor

No preview deploy

  • No preview deploy for this PR — its diff touches no deploy-relevant path, so no preview stack was minted and e2e is not applicable. (eafac25)
  • web — Stage pr-9379 torn down.

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

governance: PASS @ 5114ac1 content:e3ced352a6cd — no contradiction, no weakening

Scope

governance scope derives required at 5114ac1 over one governed root: claude-plugins/ (1 file,
skills/operate/SKILL.md). The other five changed files sit outside the declared root set. self false,
so the §4 fence did not apply and this verdict is judged by the head's rules.

This is the governance-namespace derivation, not a control-plane classification -- whether this diff
needs a code-owner approval is a separate question CODEOWNERS answers.

Corpus half -- does this contradict standing law

No decision record is in the diff, so there was no subject to rank and sweep --record was not run.
The corpus half below is a hand read.

Questions this change answers, and what standing law says about each:

  1. May a pipeline verb write to a child pull request's head branch on its own?
    No standing record forbids it. ADR 0228 (Extracted skill scripts relay verb decisions, never derive them, status amended-in-part by 0229, live) is the record that bears here, and it points
    the same way the diff went: the retrigger is a tested verb under packages/fabrika-cli/src/lane/
    and the operate step relays its answer rather than hand-rolling gh api in skill prose. No
    contradiction.

  2. Does moving a child's head collide with the epic-run records?
    The sweep is keyed on base=epic/<n>, so it reaches children of an assembly branch and nothing
    else. ADR 0382 (an epic tail PR must close its epic) governs the tail, which is based on trunk
    and therefore outside this sweep; ADR 0340 (an epic child's review-ui is the tail's by construction) is untouched -- the verb posts no verdict and reads no namespace. ADR 0290
    (retire epic conduction onto lane machines) is the shape this step lands inside, and a new lane
    verb beside lane push is consistent with it, not against it. No contradiction.

  3. Does it weaken the rule that a queued branch is not force-pushed?
    No. PUT /pulls/{n}/update-branch merges the base into the head branch -- no force-push, no
    rewrite, nothing closed -- and the sweep's base filter cannot reach a trunk-based queued tail.

  4. Does the diff license a destructive fallback?
    The opposite. The operate hunk is purely additive and one of its additions is a new prohibition
    (Never close and reopen a PR to force this), with the reason stated in words. That is a
    tightening of what a driver may do, not a loosening.

Gate half -- does this quietly weaken a guard

governance guards at 5114ac1: no-anchors-in-reach, 0 anchored invariants, 4 files compared
block-by-block against the base. Read by hand on top of that: the operate/SKILL.md hunk contains
only added lines -- no guard sentence was removed, narrowed or reworded, and no existing fence,
refusal or exit code was relaxed. The command.ts change registers a new leaf and touches no
existing one; pulls.ts adds three adapters below the existing exports and modifies none. No gate
invariant is in this diff's reach, and nothing in reach was softened.

Verdict

PASS -- no contradiction with standing law and no weakening of a guard.

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

review-code: FAIL @ 5114ac1 content:e3ced352a6cd — two repairs, both small

Code-class slice: packages/fabrika-cli/src/io/pulls.ts, src/lane/command.ts,
src/lane/retrigger-verb.ts, src/lane/retrigger-verb.unit.test.ts.

The five issue criteria are all met and CI is green. This FAIL is two repairs inside the verb's own
contract, appended as criteria 6 and 7 on #8880 — both one-line fixes, neither touching the design.

CI at head

review ci at 5114ac1: settled / green — 45 check runs, 40 success, 5 skipped, 28 of the 40
workflows this repo authors produced a run at that head. Nothing was re-run locally.

Per criterion

  • [PASS] The platform behaviour is verified against a real PR whose base moved, and which reading
    holds is recorded.
    The PR body records it against a live pull request, with what was read: a
    merge ref fetched 2026-09-16 over a head last pushed 2026-09-06, its first parent that morning's
    trunk tip, 374 commits past the base.sha the payload still reports; and every check run on that
    head stamped the push date. The second reading holds — the ref rebuilds, the event never fires —
    and both platform claims carry doc citations (pull_request activity types; GITHUB_SHA /
    GITHUB_REF replay on a re-run). Both are what the code turns on, so the behaviour claim traces to
    the read rather than to a name.

  • [PASS] A documented, non-destructive retrigger. PUT /pulls/{n}/update-branch merges the base
    into the head branch (io/pulls.ts:180-208). I read the request shape: one PUT carrying
    {expected_head_sha} and no other write anywhere in the path — nothing closed, nothing
    force-pushed, no commit rewritten. The close/reopen prohibition is graded in review-skill, where
    its file lives.

  • [PASS] A verb under packages/fabrika-cli/src/lane/, not hand-rolled gh api in skill prose.
    src/lane/retrigger-verb.ts, registered as a leafCommand at src/lane/command.ts:225-248 and
    wired into laneCommand at line 1608. No gh invocation in the path; the three reads and the one
    write are adapters in io/pulls.ts over restCall / pagedWithLinkProof — the shape ADR 0228
    asks for.

  • [PASS] Idempotent. retriggerOne asks compareStanding(repo, base, child.headSha) first and
    returns Current with no write on identical or ahead, the two statuses that mean the head
    already carries the base tip; the write is reachable only through behind and diverged. A test
    asserts the absence of the PUT rather than just the verdict, and a second pins the comparison URL
    to the branch ref rather than the PR's frozen base.sha.

Findings — both blocking this round

  1. BaseStanding.behindBy is hard-required and never read. compareStanding refuses with
    "GitHub answered 200 but its comparison declares no behind_by" when the field is absent, and the
    only consumer, retriggerOne, branches on status alone. A response missing behind_by fails the
    whole sweep over a value nothing uses. Either give it a consumer — the row could read
    #<pr> 3 behind -> <sha>, which is information the driver would use — or drop the field and its
    refusal. (An epic-child PR's merge ref goes stale when the epic branch moves, and nothing retriggers it #8880 criterion 6.)

  2. Exit 11's stated contract is false on a partially written sweep. Both the CLI long
    description (command.ts:246) and the verb-reference.md row say 11 is "the pull request list
    or a comparison could not be read before anything was written — UNKNOWN, never an empty
    sweep". The code reaches 11 through unknown.wrote === false, which is a per-child fact: with
    two children, a successful update of the first followed by a failed comparison on the second exits
    11 with a write behind it. The moved row still prints on stderr, so nothing is lost silently —
    what is wrong is the promise, on the one surface a driver consults to decide whether it is safe to
    re-run. Either scope the wording to the child, or track whether the sweep has written anything yet
    and route a later read failure to 8. (An epic-child PR's merge ref goes stale when the epic branch moves, and nothing retriggers it #8880 criterion 7.)

Standing checks — all clean

  • Test honesty. No pre-existing test or assertion changed or deleted; the diff adds one file.
    Nine cases cover both verdict paths, both refusal codes and the 422 split, each asserting an
    observable (exit code, exact stdout, request line, request body) rather than the implementation
    against itself.
  • Staleness traps. Handled rather than trapped: every write carries expected_head_sha, so a
    head a sibling moved is refused rather than misaddressed, and the 422 shared by a real conflict and
    a moved head is split by re-reading the head (retrigger-verb.ts:422-440). The 202 is treated as a
    receipt and proven by watching the head move, never by the status; awaitMovedHead reuses
    pollWaits' schedule rather than writing a second one.
  • Comment discipline. The module docblock states the platform constraint the code cannot show and
    carries the @ruling tag naming the hosted issue, which is this package's citation form. Each
    interface and function docblock states a why the signature does not — why openPullsForBase reads
    open only, why compareStanding is separate from compareFiles, why Accepted is not a landing.
    No narration of control flow.
  • Release containment. A new CLI verb a driver types: no default-on surface, nothing scheduled,
    no automatic caller. The absence of a caller is the containment and the diff matches it.
  • Sweep order. Every child is attempted before a refusal is composed, so one bad child does not
    strand the rest unread, and Unknown outranks Conflicted — the right precedence, since an
    unknown write needs a re-read before anything else is touched.

Observation, not a finding

Moving a child's head is what schedules the run, and it also changes that child's <base>..<tip>
digest — so a child already carrying a range-scoped review verdict would read Stale after a sweep.
It cannot fire under the current shape (an epic run opens one PR and its children open none, which
the skill text says plainly), so I am recording it rather than blocking on it.

Deviations

review deviations reports the section present with two entries, and both match the substance I
read.

Entry Claim Read
Declined guidance — the #8881 citation lives in verb-reference.md and the PR body rather than in operate/SKILL.md the guard reds a hosted issue reference in the shipped skill corpus Verified: guard portability-guard check is clean at this head with no ceiling raised, and the skill rubric independently names a hosted issue URL in skill text as a finding. The constraint is real and the reason still lands in the skill in words. Graded in review-skill.
Out-of-scope change — three adapters added to src/io/pulls.ts the verb reads and writes over the API and this package may not invoke gh Verified: all three are new exports appended below the existing ones, no existing export is modified, and each is the minimum the verb needs.

deviation-disclosure: PASS — nothing undisclosed that this gate could see. Neither finding above
is a deviation; both are defects inside what the PR set out to build.

Verdict

FAIL — criteria 6 and 7 on #8880.

Verdict-written: 2026-09-17T01:22:18Z

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

review-doc: FAIL @ 5114ac1 content:e3ced352a6cd — one inaccurate exit-code promise

Doc-class slice: one file, packages/fabrika-cli/docs/verb-reference.md — a single added row for
lane retrigger.

Hygiene checklist

  • [PASS] Right surface. The verb reference is where this package documents what each verb does,
    and the row sits in the lane table in registration order, between lane assembly / push and
    lane assembly-pr. No why-narrative landed in a code-shape doc.
  • [PASS] One Diataxis mode. Reference throughout: what the verb does, what it refuses, what it
    prints. It carries a paragraph of platform explanation, which on any other surface would be a mode
    mix — here it is the constraint that makes the refusals readable, and every neighbouring row in the
    file is written the same way. No steps, no tutorial voice.
  • [PASS] Supersession. Nothing is replaced and no second row answers the same question. The row
    states its boundary against the two verbs a reader would confuse it with: lane push moves the
    branch, this schedules the checks.
  • [PASS] Status sanity. No frontmatter; the body speaks in the present tense about a verb that
    exists at this head.
  • [PASS] Claims trace. The four falsifiable platform claims each carry their ground: the merge
    ref recomputes (a named live pull request whose merge ref carried that morning's trunk tip while
    its head had not been pushed for eleven days), a base push emits no pull_request event (the three
    activity types), a re-run replays the original GITHUB_SHA / GITHUB_REF, and update-branch
    merges the base into the head branch. None is stated from intuition.
  • [PASS] Portability. guard portability-guard check is clean at this head — 1424 files, 206
    references under a declared ceiling and none above it. No ceiling was raised: the guard's
    configuration is not in this diff at all. The row's Closing and reopening a PR tears down its preview stage mid-deploy and fails the deploy on a deleted D1 #8881 link sits under an existing ceiling for
    this file.
  • [PASS] Prose craft. Applying writing-for-agents to the added row: the leading sentence
    front-loads what the verb produces before how; the platform paragraph is cached rather than
    duplicated lookup — it is the gotcha no config confesses; no negation-steered rule (the close/reopen
    ban is paired with the positive target in the same clause); no no-op sentence I could find. The row
    is dense, which is this file's established shape rather than sprawl.

Finding — blocking

The row's exit 11 is a promise the verb does not keep. It reads: "11 the list or a comparison
unread before anything was written, never an empty sweep". The verb reaches 11 through a per-child
wrote: false flag, so a sweep that updates the first child and then fails to compare the second
exits 11 with a write already behind it. This is the surface a driver consults to decide whether a
retry is safe, so the inaccuracy costs exactly where it is read. The same sentence appears in the CLI
long description and is graded there too; the fix is one wording change on both, or the behaviour
change described in review-code. (#8880 criterion 7.)

Observation, not a finding

The platform explanation — merge ref rebuilds, base push emits no event, re-run replays the stale sha
— now lives in three homes: this row, the operate skill step, and the retrigger-verb.ts docblock.
writing-for-agents would call that one meaning in three places. I am not raising it, because each
of the three serves a different reader and the self-contained row is this file's convention
throughout; collapsing it would be a change to the file's shape, not a repair to this diff.

Deviations

Covered in full in the review-code verdict at this head; both entries match what I read, and
neither concerns the doc-class slice beyond the #8881 citation landing in this file, which I verified
against the portability guard.

Verdict

FAIL — criterion 7 on #8880.

Verdict-written: 2026-09-17T01:22:50Z

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

review-skill: FAIL @ 5114ac1 content:e3ced352a6cd — no finding in this slice; the round is FAIL

Skill-class slice: one file, claude-plugins/fabrika/skills/operate/SKILL.md, read whole rather
than by hunk. The change is one added block in the epic-run push section: a paragraph, a fence, and
a prohibition.

Read this polarity correctly. I found nothing in the skill-class slice that refuses. Every check
below passes. This round's two blocking findings are in the code and doc namespaces, and the round
carries the acceptance criteria they appended (#8880 criteria 6 and 7) — review post binds polarity
to the round rather than to the namespace, so a clean namespace in a FAIL round posts FAIL. A repair
round on this PR has nothing to change in this file.

1 — Behavioral correctness

  • The fence is node packages/fabrika-cli/src/bin.ts lane retrigger $lane_key. $lane_key is
    declared in this skill's own arguments: frontmatter, so the harness substitutes it before the body
    reaches the agent and the isolation verifier never sees it — not a finding under skill-conventions
    §4. No other expansion, no default expansion, no .. climb.
  • The verb exists at this head with that spelling and that arity: lane retrigger <epic>, one integer
    positional plus an optional --repo. On an epic lane $lane_key is the epic issue number — the same
    value epic/$lane_key is built from four paragraphs above — so the positional is the right one.
  • Every state word the block prints is one the verb actually emits: RETRIGGER-VERDICT: NONE, exit
    42, exit 8. I checked each against retrigger-verb.ts and the registered description.
  • No contradiction anywhere in the file. The block adds a step and retires no rule; I read the
    surrounding section and the rest of the document for a line the addition leaves standing against
    itself and found none. The lane push paragraphs above are untouched and still say what they said.
  • The block is honest about its own ordinary answer — "RETRIGGER-VERDICT: NONE says no open PR sits
    on the branch at all, which is the ordinary answer under this shape — the run opens one PR and its
    children open none". A step that is usually a no-op and says so beats a step that quietly is one.

2 — Trigger and description quality

Unchanged. The diff edits no frontmatter and adds no skill, so there is no routing surface to judge.

3 — Cross-skill conflict and shadowing

Unchanged, and the block draws its own boundaries rather than absorbing a sibling's lane: publishing
the branch stays lane push's, a conflicted child is named as that child's repair round rather than
anything this step retries, and moving a stalled PR stays elsewhere.

4 — fabrika conventions

  • Two-layer split, correctly cut. The deterministic part — sweep the open PRs, compare each head
    to the branch, write only the behind ones, guard the write — is entirely in the verb. The skill
    keeps the judgement a driver needs: when to run it, what the answers mean, what not to do instead.
    This is the shape the issue's fourth criterion asked for and the shape ADR 0228 asks for.
  • Single-home facts. The exit-code semantics are cited, not restated as a second contract; the
    Closing and reopening a PR tears down its preview stage mid-deploy and fails the deploy on a deleted D1 #8881 report is reached by a pointer to the verb's reference row rather than copied.
  • Portability. guard portability-guard check is clean at this head — 1424 files, 206 references
    under a declared ceiling, none above it — and the guard's configuration is not in this diff, so no
    ceiling was lifted to admit anything. No hosted issue number, decision-record number or
    decision-corpus path landed in this file. This is the deviation the PR discloses, and the constraint
    behind it is real: this rubric independently names a hosted issue URL in skill text as a finding, so
    putting Closing and reopening a PR tears down its preview stage mid-deploy and fails the deploy on a deleted D1 #8881 here would have been the finding the criterion's literal wording asked for. The reason
    a driver needs — a reopen tears the PR's preview stage down mid-deploy — is in the file, in words, at
    the point of use.
  • No contract read is instructed, whole or otherwise.

5 — Writing craft

writing-for-agents applied to the whole file, with attention on the added block:

  • Completion criterion. The step ends on a checkable bound — one of three named verdict tokens on
    the last stdout line, or one of two named exits. Not a fuzzy "confirm the checks ran".
  • Negation. "Never close and reopen a PR to force this" is a prohibition, which this reference
    admits only as a hard guardrail paired with the positive target. Both conditions hold: the issue's
    criterion asks for it by name, and the positive target is the fence immediately above. The paragraph
    then says why in one sentence rather than leaving the ban bare.
  • Cache, not duplication. The platform paragraph is exactly what this reference says to cache —
    the gotcha no config confesses — and the reader here cannot look it up.
  • No-ops. Checked sentence by sentence for a line the model would obey by default. The closest is
    "It is safe to run after every push", which does change behaviour: without it a driver hesitates
    over an idempotent call.
  • Co-location. The block sits against the push it follows, so the reason, the command and the
    forbidden alternative are read together.

Per criterion

  • [PASS] Close/reopen explicitly named as forbidden, with the reason. Named forbidden in bold,
    with the mid-deploy preview teardown as the reason and the report reachable through the verb's
    reference row. The Closing and reopening a PR tears down its preview stage mid-deploy and fails the deploy on a deleted D1 #8881 number itself is one file away, disclosed as a deviation with a constraint
    I verified; the substance the criterion protects — that a driver knows this is forbidden and why —
    is delivered in the file the driver reads.
  • [PASS] The step lands where the driver will hit it. Directly after the lane push fence and its
    refusal notes, before the PR-opening block. A driver walking this section in order meets it in the
    same breath as the push.

Verdict

FAIL as the round's polarity. No finding in the skill-class slice; nothing here to repair.

Verdict-written: 2026-09-17T01:24:25Z

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

Repair round 1 — both blocking findings addressed, plus the branch brought up to date with main.

Finding 1 / criterion 6 — BaseStanding.behindBy had no consumer. Given one, as the row the
reviewer suggested. Outcome.Moved now carries behindBy from the standing read, and a moved
child's row reads #8859 3 behind, aaaaaaaa -> bbbbbbbb instead of just the two shas. The count is
the comparison's own behind_by, so the refusal when GitHub omits it now guards a value a driver
reads. BaseStanding.behindBy's docblock names the consumer.

Finding 2 / criterion 7 — exit 11 promised an untouched sweep it did not keep. Fixed in the
behaviour rather than the wording, so the promise a driver depends on stands. Moved now carries
wrote beside Unknown's, and the sweep routes an unknown on outcomes.some(wroteTo) — whether
any child was written to — instead of on the unknown child's own flag. A comparison that fails on
the second child after the first child's head was moved is now exit 8, and 11 means what it
says: nothing was written, so the sweep is safe to re-run. A new test asserts exactly that ordering
(routes a read that fails after an earlier child was moved to the written-to code).

Both surfaces that state the exit codes were reconciled whole against the behaviour: the CLI long
description in lane/command.ts, the lane retrigger row in docs/verb-reference.md (which now
also carries the row grammar), the operate/SKILL.md step's exit-code sentence, and the PR body.
The row grammar changed, so every surface quoting it moved in this push.

Also merged origin/main into the branch (it was two commits behind) — no conflicts.

Green in this tree: build check --surface code and --surface prose both green at this head;
vitest run src/lane src/io 76 files / 1669 tests pass after the merge.

Reviewer, start at packages/fabrika-cli/src/lane/retrigger-verb.ts — the Outcome union and the
wroteTo helper are the whole of finding 2.

— at eafac25

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

governance: PASS @ eafac25 content:8c751bc8cfc9 — no contradiction, no weakening

Corpus half — does this contradict standing law

No decision record is in this diff, so there was no subject to rank and governance sweep was
not run. The corpus half is a hand read, recorded here as such.

Questions the change decides, written down before the read:

  1. May an operator move an OPEN child pull request's head automatically after its base branch moves?
  2. Is a mechanical retrigger a CLI verb, or gh api prose inside skill text?
  3. Is close/reopen forbidden as a way to rebuild a merge ref?
  4. Does scheduling checks this way weaken base-freshness anywhere?

Read against the standing corpus, ids resolved through adr resolve at the base:

  • 0132 — Adopt GitHub merge queue for base-freshness at merge (live, accepted). It owns
    base-freshness at the trunk merge, and its rejected alternative is branch-protection "require
    branches up to date", which would force an update-and-re-run on every open pull request at every
    base advance. This verb's base is an epic run's assembly branch, which no merge queue reaches, and
    it schedules checks rather than deciding a merge — nothing here changes what lands on trunk or how.
    It is operator-invoked, scoped to one assembly branch, and a no-op on a child that already carries
    the base, so it is not the treadmill 0132 argued against. No contradiction.
  • 0228 — Scripts relay, never derive (live, amended in part by 0229). The retrigger is
    implemented as a verb under packages/fabrika-cli/src/lane/ with a testable decision core, and the
    skill fence relays a single call rather than hand-rolling gh api. That is this record's shape, not
    a departure from it. No contradiction.
  • 0313 — A queue dwell is a wait, not a park (live, accepted). Read and found not in reach:
    the verb records nothing on a lane and parks nothing.

No standing record was found that this diff contradicts.

Gate half — does this quietly weaken a guard

  • governance guards at this head: no-anchors-in-reach, 0 anchored invariants in reach, 4 files
    compared block-by-block against the base.
  • The diff removes no line at all. Across all six files the change is purely additive (the one
    operate/SKILL.md hunk is 32 added lines against 6 context lines; verb-reference.md adds one row;
    pulls.ts and command.ts append; two files are new). There is no removed or softened line to
    point at, so there is no weakening to evidence.
  • guard portability-guard check run against this head's tree: clean, 1428 files scanned, 206
    references under a declared ceiling and none above it. No allow-list ceiling was raised to make
    room for the new references — the two hosted issue URLs the change adds sit outside the guard's
    walked roots (packages/fabrika-cli/docs/) or inside the admitted @ruling span, and the ticket
    numbers in the new test file are test-data string literals the guard already exempts.
  • The verb's own refusals add guards rather than removing any: expected_head_sha on every write, a
    fail-closed behind_by read, and an exit split that reports a sweep with a write behind it as
    UNKNOWN-with-a-write rather than as an untouched sweep.

No gate invariant is in this diff's reach, and nothing in it is removed or softened.

Self fence

governance scope printed self false — the diff does not edit this skill or its contract, so the
fence did not apply and the head's rules were used.

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

review-code: PASS @ eafac25 content:8c751bc8cfc9 — merge-ready

Round 2. Both round-1 findings are repaired at this head, and the repair carries its own regression
tests.

CI at head

review ci --wait settles green: 45 check runs, 40 success, 5 skipped, 0 failure, with 28 of the
40 workflows this repo authors producing a run at this head. Gate coverage is present, so the green
is a green something inspected. Typecheck, lint, unit tests and the scanners are read off that
enumeration and not re-run here.

Per criterion

  • 1 — the platform behaviour is verified against a real PR whose base moved, with what was read
    recorded.
    [PASS] The PR body's ## What the platform actually does names the live read: an open
    PR whose head had not moved since 2026-09-06, its merge ref fetched 2026-09-16 resolving to
    470decb9 whose first parent is 2c2a5dc3 (that morning's trunk tip), 374 commits past the
    base.sha the payload still reports (5c26458b); and every check run on that head stamped the day
    the head was pushed. The second reading is stated as the one that holds, and the lever the first
    reading would have allowed (a workflow re-run) is ruled out with the documented
    GITHUB_SHA/GITHUB_REF replay quoted. The same read is carried in retrigger-verb.ts's module
    docblock with both platform citations.
  • 2 — a documented non-destructive retrigger, close/reopen named forbidden. [PASS] lane retrigger moves the head through PUT /pulls/{n}/update-branch (io/pulls.ts,
    updatePullBranch) — a merge of the base into the head branch. Nothing is closed, force-pushed or
    rewritten, and the verb touches no working tree. operate/SKILL.md names close/reopen forbidden in
    its own bolded sentence with the reason in words. The #8881 citation sits in the verb-reference
    row and the PR body rather than in the skill; that is disclosed as a deviation and verified below.
  • 3 — the step lands where the driver hits it. [PASS] The paragraph sits in the epic-child
    section of operate/SKILL.md directly after the lane push fence and its isolation note, and
    before the paragraph that opens the run's PR.
  • 4 — a verb under packages/fabrika-cli/src/lane/, not gh api prose. [PASS]
    packages/fabrika-cli/src/lane/retrigger-verb.ts holds the decision core, lane/command.ts
    registers a thin adapter that only unwraps options and emits, and the skill fence is one call.
  • 5 — idempotent. [PASS] retrigger-verb.ts:165 returns Current without any write when the
    platform's comparison reads identical or ahead; the staleness question is asked of
    compareStanding's live base...head comparison rather than the PR record's frozen base.sha,
    which is what makes a second call a no-op. Two tests hold it: one asserts no PUT is issued for an
    ahead child, one asserts the compare URL is the live one.
  • 6 — behindBy is read by a consumer or dropped. [PASS] Repaired. row at
    retrigger-verb.ts:203-208 prints it on every Moved row (#<pr> <n> behind, <from> -> <to>), it
    is threaded through Moved from both the accepted path (:198) and the declined-but-moved path
    (:190), and the test asserts the exact stdout #8859 3 behind, aaaaaaaa -> bbbbbbbb. The
    fail-closed behind_by refusal in compareStanding now guards a field the verb actually uses, so
    the sweep it can fail is one that would otherwise print a number it never read.
  • 7 — exit 11 means what it says. [PASS] Repaired. retrigger-verb.ts:267 chooses
    outcomes.some(wroteTo) ? APPEND_UNKNOWN : LANE_UNREADABLE, and wroteTo (:99) is true only for
    a Moved or Unknown this sweep addressed a write to. The three no-write paths are correct: a
    failed comparison sets wrote: false before any PUT, a 422 Declined sets wrote: false
    because nothing landed, and an Unreadable update sets wrote: true because it may have. The
    regression test "routes a read that fails after an earlier child was moved to the written-to code"
    scripts exactly round 1's scenario — first child updated, second child's comparison 502s — and
    asserts APPEND_UNKNOWN and not LANE_UNREADABLE.

Standing checks

  • Test honesty. Nine tests, all new; no pre-existing test or assertion is touched anywhere in the
    six-file diff. Each asserts behaviour through the verb's public outcome (exit code, exact stdout,
    the requests and bodies the fake HTTP recorded), not the implementation against itself. The
    expected_head_sha test reads the actual request body.
  • Release containment. None needed and none claimed: the verb runs only when a driver types it,
    registers no new default-on surface and writes no lane record.
  • Comment discipline. The module docblock is long and load-bearing — it is the record of the
    platform read criterion 1 asked for, with two docs.github.com citations — and it carries the
    @ruling tag this package owes, so the collapse finding does not apply. The one inline comment
    (above the unknown branch at :261-264) states why the exit code answers over the whole sweep
    rather than over the child that failed, which is a constraint the expression cannot show.
  • Staleness traps. This verb is the answer to one rather than a carrier of one: nothing is
    cached across a boundary, every write is guarded by the head it was addressed against, and the 202
    receipt is explicitly not read as a landing — awaitMovedHead proves the merge by watching the
    head move and calls the spent window UNKNOWN rather than done.

Behaviour claims, traced

  • "Idempotent" — traced to :165, the early Current return before any write.
  • "Refused rather than misaddressed" — traced to the expected_head_sha body in updatePullBranch
    and the Declined arm's head re-read, which splits a conflict from a head a sibling moved.
  • "202 is a receipt, not a landing" — traced to awaitMovedHead, which polls the head and never
    reads the tag as success.
  • "Diverged children are updated too" — traced to :165 admitting only identical and ahead as
    current, with a test.

Deviations

Entry Substance Verdict
Declined guidance — the #8881 citation lives in verb-reference.md and the PR body, not in operate/SKILL.md the portability guard reds a hosted issue reference inside the shipped skill corpus and its floor only shrinks Verified. guard portability-guard check at this head's tree is clean (1428 files, 206 references under a declared ceiling, none above), and the two references the change adds sit outside the walked roots or inside the admitted @ruling span. The constraint is real, no ceiling was raised, and the reason still lands in the skill in words.
Out-of-scope change — three adapters added to src/io/pulls.ts the verb reads and writes over the API and this package may not invoke gh Verified and proportionate. All three are the verb's own reads and writes, each documented against the endpoint it calls, and openPullsForBase states why it narrows to open where its sibling pullsForBranch does not.

deviation-disclosure: PASS — nothing undisclosed that this gate could see. Both entries match
substance, not just a label, and I found no third deviation the section omits.

Observation, not a finding

The sweep reports the first Unknown in the refusal headline and every other child in the rows
below it, so a sweep holding both an unknown and a conflict exits on the unknown and the conflict is
visible only as a #<pr> conflicted row. That ordering is the right one — an unknown dominates,
because it is the one that says do not write again — and the conflicted child is not lost.

Verdict

PASS — all seven criteria met, CI green with gate coverage at this head, deviations disclosed and
verified.

Verdict-written: 2026-09-17T01:47:17Z

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

review-doc: PASS @ eafac25 content:8c751bc8cfc9 — merge-ready

Round 2. Doc-class slice: the one added row in packages/fabrika-cli/docs/verb-reference.md. Round
1's finding here was the exit-11 promise that the code did not keep; the row now describes the
behaviour the code has.

The round-1 finding, repaired

The row previously promised exit 11 meant "before anything was written" while the verb could reach
it after a child's head had already moved. Both halves changed together: the code now routes a
post-write unknown to 8 (verified in review-code), and the row states 11 as "the list or a
comparison unread while this sweep had written to no child, so the sweep is untouched and safe
to re-run — never an empty sweep". That is now an accurate promise, and it agrees with the CLI's own
--help description and with the operate/SKILL.md paragraph word for word in substance. Three
surfaces, one meaning.

Hygiene checklist

  • Right surface. [PASS] verb-reference.md is this package's one-row-per-verb reference, and a
    new verb owes a row there. The row sits between lane assembly/push and lane assembly-pr,
    which is where a reader following the epic-run sequence meets it. No why-narrative leaked into a
    code-shape doc and no shape note leaked into a decision record.
  • One Diataxis mode. [PASS] Pure reference, matching its neighbours: what the verb produces, the
    platform facts it rests on, the verdict tokens, the row format, the exit codes. It gives no
    procedure and asks the reader to do nothing, so it does not drift into how-to; the platform
    explanation it carries is the grounding a reference row in this file conventionally holds (compare
    the lane refresh and lane integrate rows above it), not a tutorial digression.
  • Supersession. [PASS] Nothing is replaced or contradicted. No other row and no other doc claims
    to answer "how do I retrigger a child's checks"; this is the first.
  • Status sanity. [N/A] The file carries no frontmatter or status line.
  • Claims trace. [PASS] Every falsifiable platform claim in the row is grounded. The merge-ref
    rebuild and the missing event are traced to the live read the PR body records with commit SHAs and
    dates. The event list (opened, synchronize, reopened) and the re-run replay of GITHUB_SHA
    and GITHUB_REF are the documented behaviours, cited in retrigger-verb.ts's docblock against
    docs.github.com. The expected_head_sha guard and the shared 422 are traced to the
    updatePullBranch docblock and the REST page it links.
  • No reference only this repo can resolve. [PASS] guard portability-guard check run against
    this head's tree: clean, 1428 files scanned, 206 references under a declared ceiling and none above
    it. No ceiling was raised. The row does carry #8063 and a #8881 link, and both sit in
    packages/fabrika-cli/docs/, outside the two roots the guard walks (claude-plugins/fabrika/ and
    packages/fabrika-cli/src/) — so they are outside the portability floor by placement, which is
    the same reasoning the deviation disclosure gives for keeping the #8881 citation out of the
    skill.
  • Prose craft (writing-for-agents). [PASS] The row is long, and long is this file's unit: every
    neighbouring row is one dense paragraph, and a shorter row here would answer fewer of the questions
    the file exists to answer. Read against the skill's levers — the opening states what the verb
    produces rather than what it is; the reason is given before the mechanism; the exit codes are each
    bound to a remedy rather than left as numbers; the idempotence claim names what makes it true
    rather than asserting it. Nothing needs re-reading to parse and no sentence restates a neighbour.

Deviations

Both disclosed entries are graded in review-code; neither is a doc-class deviation. The first one's
justification is the reason a doc reference lives in this file rather than in the skill, and I
verified it holds: the guard's walked roots do not include packages/fabrika-cli/docs/.

deviation-disclosure: PASS — nothing undisclosed that this gate could see in the doc slice.

Observation, not a finding

The exit-code semantics now live in three places: the CLI --help description in lane/command.ts,
this row, and the operate/SKILL.md paragraph. That is the established shape in this repo — every
verb row mirrors its CLI description, and the skill carries only the subset a driver hits — so it is
not new drift. It is still three copies that must move together, and this PR is the second time
in two rounds that they did.

Verdict

PASS — the round-1 inaccuracy is repaired, every hygiene line holds, and the portability guard is
clean at this head with no ceiling raised.

Verdict-written: 2026-09-17T01:47:54Z

@usirin

usirin commented Sep 17, 2026

Copy link
Copy Markdown
Member Author

review-skill: PASS @ eafac25 content:8c751bc8cfc9 — merge-ready

Round 2. Skill-class slice: the 32 added lines in claude-plugins/fabrika/skills/operate/SKILL.md,
read against the whole file rather than the hunk. Round 1 found nothing in this slice and posted FAIL
as the round's polarity; the repair did not touch this file's added text beyond the exit-11
sentence, and that sentence is now true.

1 — Behavioural correctness

  • Every step executable as written. The one fence is
    node packages/fabrika-cli/src/bin.ts lane retrigger $lane_key. $lane_key is declared in this
    skill's own arguments: frontmatter (line 4), so the harness substitutes it before the body reaches
    the agent and the isolation verifier never sees an expansion — not a finding under
    skill-conventions §4. No other $VAR, no default expansion, no .. climb.
  • The entrypoint spelling matches its neighbours and the file already explains it. The three
    fences above and below it — lane push, lane assembly-pr, lane assembly-body — use the same
    repo-relative spelling, and the parenthetical after the lane push fence states the equivalence
    once for all of them ("the same repo-relative path <fabrika> resolves to in a checkout of
    fabrika's own repo; in a repo that installs fabrika it is that install's absolute bin"). The single
    home is upstream of the new step and the new step does not restate it. Correct by the file's own
    convention.
  • Checkable completion criterion. The step ends on a bound the driver can read: one of
    RETRIGGER-VERDICT: RETRIGGERED, CURRENT or NONE on the last stdout line, with NONE named as
    the ordinary answer under the current epic shape so a driver does not read it as a failure. Every
    exit code the step names (42, 8, 11) is bound to a remedy rather than left as a number.
  • Verb and spelling exist in the reference beside it. lane retrigger is registered in
    lane/command.ts, its positional is the epic issue, and the verb-reference.md row carries the
    same verdict tokens and the same three exit codes. Flags: the fence passes none, and the verb's
    only flag (--repo) is optional with a documented default. No drift between the step and the
    contract.
  • No contradiction elsewhere in the file. I read operate/SKILL.md whole for a rule this change
    leaves standing against itself. Nothing else in it tells a driver to re-run a workflow, to close
    and reopen a PR, or that an assembly push needs no follow-up; the only other reopen in the file
    is unrelated (a frozen lane's door). The new paragraph is purely additive and the diff removes no
    line anywhere.
  • The exit-11 sentence, round 1's finding in the code slice, now reads true here too. "Exit
    11 is the sweep that wrote to nothing, which is the one you can simply re-run" matches the
    verb's repaired routing. A driver acting on this sentence will not re-run a sweep that already
    moved a head.

2 — Trigger and description quality

[N/A] No frontmatter changed and no new skill was added. operate's description is untouched.

3 — Cross-skill conflict and shadowing

[PASS] No new skill, so no new trigger to collide. I checked the nearest lane: heal-ci owns "this
PR is stuck, diagnose it and move it". This step is not a diagnosis — it is the known remedy for a
cause the driver just created one command earlier, scoped to the children of one assembly branch, and
the skill says so in that shape. No absorption either way.

4 — fabrika conventions

  • Two-layer split. [PASS] The deterministic work — list, compare, guard, write, prove — is the
    verb's; the prose carries only the judgement a driver needs (when to run it, why the cheap levers
    do not serve, what each verdict and exit means). This is the shape criterion 4 asked for and the
    shape ADR 0228 asks for.
  • Sizing. [PASS] 32 lines is the longest single addition in this section, and it earns them: the
    platform reasoning is what stops a driver reaching for close/reopen or a workflow re-run, which is
    the failure the step exists to retire. Nothing that belongs in the reference row is inlined — the
    row's full exit table and its row format stay there, and the skill names only the three exits a
    driver meets.
  • Single home. [PASS] The platform read is stated once in the skill in the form a driver needs,
    once in the reference row, and once in the verb's docblock as its governing record. The skill
    restates no sibling verb's behaviour.
  • Portability. [PASS] guard portability-guard check at this head's tree is clean — 1428 files,
    206 references under a declared ceiling, none above. The added skill text names no ticket, no
    decision record, no decision-corpus path and no hosted URL. Round 1 verified the constraint behind
    the disclosed deviation and I re-verified it: a #8881 in this file would red the guard, whose
    floor only shrinks, so the criterion's literal wording could not be met without lifting a ceiling —
    and lifting it would itself have been the finding. The reason close/reopen is forbidden lands in
    the skill in self-contained words ("it tears the pull request's preview stage down mid-deploy"),
    which is the substance the criterion protects.
  • Contract reads. [N/A] The change instructs no contract.md read.

5 — Writing craft (writing-for-agents)

[PASS] Read over the whole added block against the skill's levers:

  • The opening sentence states the fact and the action together and is the paragraph's own headline.
  • The mechanism is given before the fence, so the driver knows what the call buys.
  • The forbidden alternative is steered by its positive target: "Never close and reopen a PR to force
    this" is immediately followed by what to do instead and why the head has to move at all. The
    prohibition is not left bare.
  • Every claim about the platform is falsifiable and named (the event list, the re-run's pinned SHA
    and ref) rather than asserted as intuition.
  • The NONE sentence earns its place: it pre-empts a driver reading the ordinary answer as a
    failure, and it states the cost (one board read) so the step's price is visible.
  • No sentence needs re-reading to parse, and nothing restates the paragraphs around it.

Deviations

Both disclosed entries are graded in review-code. The first is the only one touching this slice,
and its substance is verified above.

deviation-disclosure: PASS — nothing undisclosed that this gate could see in the skill slice.

Observation, not a finding

"lane retrigger's own reference row carries the report that fallout was filed under" asks an
adopter's agent to follow a pointer into a file it may have but a ticket it cannot open. The reason
itself is already self-contained in the sentence before it, so the pointer costs nothing if ignored
and serves a reader inside this repo. Worth a trim some day; not a finding, because the guard is
clean and the load-bearing half of the sentence stands alone.

Verdict

PASS — no finding in the skill-class slice, the exit-11 sentence now matches the verb, and the
portability floor is unmoved.

Verdict-written: 2026-09-17T01:48:42Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

An epic-child PR's merge ref goes stale when the epic branch moves, and nothing retriggers it

1 participant