[Danathar]

Summary

A fork PR whose only red check was a maintainer-run fork guard (here the fork-gated Canary) was sent back to its author on its first red head. The author was nudged to fix Canary and their relay was handed rework, although the check itself says the author cannot fix it. This happened because the hive only knew about the fork guard from the fork-guard index (not yet read for that head) or from the stage record (only set once a PR has been parked on a guard). With this PR the stage pass reads such a head's own check annotations once, using the same rule the index uses. A head red only on fork guards goes straight to stage/085-needs-decision for a maintainer, and the guard is recorded for later heads. A fork head with any other red check still goes to its author, as before.

What went wrong

Evidence (projectbluefin hive log, build 467557e; GitHub timeline and check runs of projectbluefin/utah-packages#972):

• 14:41:50Z Karibhou opened fix: make dashboard and main page fully dynamic #972 from the fork Karibhou/utah-packages (head d24a12f).
• 14:42:24Z Canary failed on d24a12f. Its annotation reads: "a fork pull request cannot publish the canary image, and no maintainer-run Canary has passed on d24a12f… This is expected, not something the author can fix: a maintainer reviews the change, then runs the 'Canary for a fork pull request' workflow".
• 14:42:56Z the PR went to stage/072 for a body issue (closing-contradiction:#42), and Karibhou fixed that at 14:44:43Z.
• 14:45:32Z stage labels: contributor nudged ... pr 972 ... reason required-check-failing:Canary. The hive rewrote its nudge comment (edited 14:45:31Z) to "Failing checks: Canary" with "Queued in your relay fix queue". The same nudge refreshed the fix already in flight for that PR so that it named Canary, and after the relay refused that task (see 🐛 [v6] relay: a fix for your own PR is taken from any hive you subscribe to, and a polling-slot refusal no longer counts as a failed attempt #17405), it went out again as fix-karibhou-2 at 14:46:11Z.
• 14:47:06Z Karibhou replied that the Canary failure is the fork gate.
• 14:50:57Z the PR moved to stage/085-needs-decision (fork-guard:Canary), once the index read the annotation.

The hive already has this rule. hgithub.ForkGuardChecksFor parks a fork PR at 085 when every failing check's annotation says it refuses forks. But the stage pass only learns the result in two ways:

• The fork-guard index. It is filled by a CI-enriched enumeration that has read the annotations for that exact head, so it lags a new head by minutes and is empty after a restart.
• The stage record. foldForkGuard and carriedForkGuard carry a guard across heads only after the PR has once been parked on it (fork-guard: reason or ForkGuardChecks).

A fork PR's first red head has neither. #972's record said closing-contradiction, so foldForkGuard returned early and the nudge went out. (#17380 fixed the same gap for branch updates. The stage path's record fallback was already there, but it cannot help a PR that was never parked on a guard.)

What changes

• foldForkGuard: when the PR is from a fork, its head is red, and neither the index nor the record names a guard, it calls the new foldUnclassifiedForkGuard.
• That function reads the head's check digest (the cached one the record path already uses). For each red check it reads the latest run on the head and that run's annotations, and applies ForkGuardChecksFor. It stops at the first red check that is not a fork guard.
  • When every red check is a fork guard, the head's observations carry the guard, so the stage is 085 with no nudge and no adoption, and NoteForkGuard records it for later heads.
• The result is remembered per head and red set in the stage tracker, so each head is read once. A head found red on a real failure is not read again, and a failed read, or a red check that is being re-run, is retried on the next pass.
• The latest-run-per-check lookup is moved out of forkGuardHeldOnHead into latestCheckRunsOnHead, and both callers use it.

Cost: at most one check-run list plus one annotation read per red check, once per fork head that nothing has classified yet. A head with a real failure costs one annotation read.

Out of scope: Canary itself and merge policy. If the annotations cannot be read, the head keeps today's behaviour (nudge) until the index catches up.

Files

• src/pkg/github/automerge/automerge_fork_guard_carry.go: foldUnclassifiedForkGuard, headForkGuard, forkGuardFromAnnotations, latestCheckRunsOnHead
• src/pkg/github/automerge/automerge_stage_sharedci.go: foldForkGuard calls it when nothing knows a guard
• src/pkg/github/automerge/automerge_stage_labels.go: the tracker's per-head cache
• src/docs/labels-and-control-signals/stage-labels.md (fork-guard row), changelog.d/fixed-fork-guard-first-head.md, and two line citations in ADR-0025 that had already drifted
• Test: src/pkg/github/automerge/automerge_fork_guard_first_head_test.go (new)

Tests

No tests were run locally (repo policy); CI is the check.

TestForkGuardFirstHeadReadFromAnnotations replays #972 with an empty index and no record guard. The mock serves the real Canary annotation text from check run 129228619506.

• First pass: the head is red only on Canary. It must land at 085 with reason fork-guard:Canary, with no nudge comment and no Nudge to the relay sink. On the v6 tip foldForkGuard returns early (no record guard), so the PR goes to 072 and is nudged, and the test fails there.
• Second pass: the PR stays at 085 and the Canary annotations have been read exactly once.
• A fork head red on Unit tests: it still goes to 072 with one nudge, and its annotations are read once across two passes.

Refs projectbluefin/utah-packages#972
Refs #17380

🤖 Generated with Claude Code

https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1

[kubestellar-prow]

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by:
Once this PR has been reviewed and has the lgtm label, please assign clubanderson for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

• OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

[github-actions]

Open PR overlap check

Base branch: v6

Conflicts are checked with every PR merged onto the current tip of v6, so a PR that only needs a rebase is not reported as conflicting with its siblings.

No conflicting open PRs or same-file overlaps were found.

[hivecommons-hive]

Read the diff against v6 7423d91 together with foldForkGuard (automerge_stage_sharedci.go:88-148), headChecks (automerge_stage_human.go:89-104), ForkGuardChecksFor (pkg/github/fork_guard.go:43-63) and Store.NoteForkGuard (stagelabels/store.go:2587). The change does what the body says: a fork head that is red and has no guard in the index or the record now gets classified from its own annotations with the same rule the index uses, and the forkGuardHeldOnHead split into latestCheckRunsOnHead preserves the original held/not-held logic exactly. Looks correct to me.

One low-severity edge worth a look, not a blocker:

• automerge_fork_guard_carry.go — knownNotForkGuard keys on (key, head) only, while headForkGuard keys its cache on (key, head, red-set). headChecks refreshes the digest every humanChecksTTL (10m) on the same head, so the red set can change without a push: a fork head red on Unit tests (flake) + Canary is classified not-a-guard, then the flake is re-run green and only Canary is red. foldUnclassifiedForkGuard returns at the knownNotForkGuard check before headForkGuard ever sees the new red set, so the PR stays at 072 with the author nudged until the index path catches up. Keying knownNotForkGuard on the same redKey (or dropping it and letting the headForkGuard cache hit do the short-circuit — it already returns nil, false without a read) would close it. Pre-existing index fallback still applies, hence low.

Also noted: Store.NoteForkGuard is a no-op when the record does not exist yet (store.go:2591), so on a brand-new PR's first pass the guard lives only on the observations; the second pass writes it. The test covers that second pass, so this is fine as written.

— hive: agent=reviewer backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Read the diff against the v6 tree (foldForkGuard, ForkGuardChecksFor, Store.NoteForkGuard). The main mechanism is sound: moving the !ok || len(guard)==0 early-return in automerge_stage_sharedci.go below the red-check loop means nothing is read for a green head, and foldUnclassifiedForkGuard applies the same hgithub.ForkGuardChecksFor rule the index uses (fork_guard.go:43), on the latest run per check (latestCheckRunsOnHead, shared with forkGuardHeldOnHead and behaviourally identical there). An in-progress re-run returns errForkGuardUnsettled and is not cached, so it is re-read next pass. NoteForkGuard is a no-op when the record does not exist yet (stagelabels/store.go:2591), but the observation still carries the guard for that pass and the tracker cache answers the next pass, so the record picks it up once it exists.

One low finding — the negative cache ignores the red set (automerge_fork_guard_carry.go:202, :227-231):

• headForkGuard (:244) keys its cache on head and sorted red set, and the comment says "once per head and red set".
• But knownNotForkGuard (:231) short-circuits on head alone whenever the stored guard is empty, before headForkGuard runs.
• Mechanism: a fork head first seen red on {Canary, Unit tests} (say a flaky Unit tests) stores guard=nil. A maintainer re-runs Unit tests, it goes green, and the head is now red only on Canary — exactly the #972 shape — yet line 202 returns before re-classifying. The same applies to the commit-status case (:274, nil, nil is cached as a negative).
• Consequence: that head keeps the author-facing route (072 + nudge + relay rework) until the fork-guard index catches up — i.e. the pre-PR behaviour, for that head only. Not a regression against the tip; it is a hole in the new coverage. Dropping the knownNotForkGuard pre-check (or making it compare entry.red too) would make the cache match its own docstring at the cost of one annotation read per red-set change.

Not blocking in my view; flagging so the author can decide whether a red set that shrinks on the same head is worth the extra read.

— hive: agent=reviewer-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Test-coverage read of automerge_fork_guard_first_head_test.go @348be98 (quality lane). The test pins the two pure cases well (guard-only head → 085, no nudge, one annotation read; real-failure head → 072, one nudge, one read). Three branches of the new code have no test that would fail if they regressed:

1. errForkGuardUnsettled is not remembered (headForkGuard → forkGuardFromAnnotations when the latest Canary run is in_progress). A case with "status":"in_progress" on the first pass and completed/failure + guard annotation on the second should assert: pass 1 nudges nobody and caches nothing, pass 2 lands at 085 with a second annotation read. Today a regression that caches the unsettled read as guard=nil would reproduce the fix: make dashboard and main page fully dynamic #972 nudge for the head's entire life, and nothing fails.
2. Mixed red set → author. {Canary (guard), Unit tests (real)} on one fork head must go to 072 with one nudge, and forkGuardFromAnnotations should stop after the first non-guard check (one annotation read, not two — the "costs one annotation read" claim in its comment is otherwise unverified).
3. The reviewer's shrinking-red-set finding is directly testable: {Canary, Unit tests} red → Unit tests re-run green → head red only on Canary. With the current knownNotForkGuard pre-check that test fails (stays 072), so it is the regression test to land alongside whichever fix is chosen.

All three fit the existing runs/annotations maps and pass() helper in the new test; none needs new fixtures. Not blocking — flagging so the branches ship pinned.

---

🐝 Hive Agent: quality | Instance: hosted-available-oke-11-placeholder-r05x | SHA: unknown

— hive: agent=quality backend=copilot model=claude-fable-5.1 copilot=1.0.88

[hivecommons-hive]

Hive status — updated in place; edits do not notify. Full history is in the Activity Monitor.

Evidence bundle `#17406348be98` sha256:9f798704c079fefd982167f961e1dda76a9415fd41a40583525bf320092da4f3 — download from the dashboard or `hivectl review evidence #17406`.

Review findings from ci-maintainer (folded here because this head already carries its hive review):

Style re-review of the delta 8ae93bc…348be98 (relay fix-pr-from-any-hub + hub Return path). Delta reads cleanly; one advisory, non-blocking.

Hub keys retry policy off a free-text reason string — src/pkg/dashboard/contribute_fixqueue.go:577-585. relayDidNotTakeFix matches the exact sentence "Hub is not the active polling slot" emitted at bin/contributor-relay.js:10009. The protocol already carries a structured discriminator for this (failure_kind: 'environment' at contributor-relay.js:5055, 9984, 9992), and relayFailureReason (contribute_fixqueue.go:589) already reads msg.FailureKind. A future reword of the relay's log/reason sentence would silently re-enable attempt counting. The string match is still needed for pre-#972 relays, but new relays can tag the refusal so the hub has a stable key:

        sendTo(hub, { type: 'task_failed', seq: nextSeq(), task_id: msg.task_id, reason: 'Hub is not the active polling slot', failure_kind: 'not_taken' });

and on the hub side accept either:

 func relayDidNotTakeFix(msg WSMessage) bool {
-	return strings.EqualFold(strings.TrimSpace(msg.Reason), relayRefusedNotActiveSlot)
+	return strings.EqualFold(strings.TrimSpace(msg.FailureKind), "not_taken") ||
+		strings.EqualFold(strings.TrimSpace(msg.Reason), relayRefusedNotActiveSlot)
 }

Minor: the Return doc comment (src/pkg/fixqueue/fixqueue.go:373-374) says "busy, or polling another hive", but the only caller routes the polling-slot refusal; a busy relay defers a fix-pr (contributor-relay.js:10015) rather than refusing it, and 'Already has active task' (:10021) still goes through Requeue. Either trim the doc to what is wired or route that reason to Return too.

Previous finding automerge_fork_guard_carry.go:231 (negative fork-guard cache keyed on head only): file untouched in this delta, still present, previously judged non-blocking.

[hivecommons-hive]

merged by Hive trusted-author auto-merge: author @Danathar holds role owner and GitHub permission maintain on repo; CI green at 348be98.