[clubanderson]

Fixes #14519

What

hivecommons/docs#481 by a trusted registry contributor sat 17+ hours: after admission (contributor-lane:registry-trusted) the lane skipped with required-check-missing:Audit production dependencies because GitHub held the fork head's pull_request runs in action_required and nothing would ever start them. automerge_held_runs.go deliberately never nudges human-authored lanes — right for unknown contributors, wrong for one the operator already trusts to merge.

How

• New automerge_contributor_held_runs.go: in evaluateContributorLanePR, when claim.admittedByRegistry() (registry path only — not auto-promotion, claim marker or association) and the CI reason is required-check-missing:<ctx>, list the head's pull_request runs, pick the action_required ones whose head repository is the PR head (fork included), mark once-per-head in Engine.heldRunApprovals, and POST /repos/{o}/{r}/actions/runs/{id}/approve (raw request; go-github v72 has no binding).
• Refuses on sentinel / exclude / hold labels and on .github/workflows/** diffs (reuses contributorLaneDiffReason); the lane's own gates already return earlier for those, the helper re-checks defensively.
• One idempotent audit comment <!-- hive:contributor-held-runs-approved head=… author=… tier=… runs=… --> via the effects boundary; structured log msg="contributor-lane approved held runs" with repo/pr/head/author/tier/run_ids.
• Toggle auto_merge.contributor_lane.approve_held_runs_for_registry_trusted (default true) in config, cmd/hive policy plumbing, and dashboard /api/config/auto-merge GET/PUT.
• Header comment in automerge_held_runs.go and docs (hive-merge.md, operator-reference.md, github-app-setup.md) describe the exception and the App's Actions: Read and write requirement (403 → logged once per head, lane keeps waiting).

Tests

Table-driven against the contributor-lane httptest fixture (extended with branch-protection required contexts, actions/runs listing and the approve endpoint; posted PR comments now become visible to later GETs): registry-trusted fork/same-repo → approves once across two ticks + one comment; auto-promoted / claim-marker → no approval; sentinel label, exclude label, workflows path → refuse; toggle off → nothing; no held runs → nothing; 403 → no comment. Plus helper unit tests, config default test, dashboard GET/PUT field tests. No time.Sleep.

[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: v5

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

PR	Title	Relation	Files
#14502	🐛 fix: enable tool-less Copilot ioscan classifiers	same files	src/pkg/config/config.go

[hivecommons-hive]

@clubanderson is auto-promoted for contributor-lane merging on this PR: the last 5 merged PRs by this author in this repository have no revert or sentinel-alert evidence. CI, reviewer verdict, diff fences, soak, holds and branch protection still apply.

[hivecommons-hive]

@clubanderson this PR is held at stage/071-needs-author and the hive will not merge it yet.

Failing checks:

• test (dashboard 4/4)
• test (dashboard shuffle 2/3)

Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path.

[hivecommons-hive]

Reviewed (correctness) — approving; one advisory, no blocker.

Side effect in the relay's read-only evaluation (low, in-scope). evaluateContributorLanePR documents relay=true as "the relay's read-only evaluation" (src/pkg/github/automerge/automerge_contributor_lane.go:1149-1152), and the behind/update-branch and effects-reconcile steps honour that (:1223, :1241). The new call at :1256-1258 does not — a merge-request relay evaluation (automerge_relay_gate.go:81) will now approve runs and post the audit comment. Once-per-head bookkeeping keeps this from double-firing against the sweep, so the impact is only the broken invariant, but gating on !relay keeps the contract intact and is a one-liner:

		if !relay && strings.HasPrefix(reason, requiredCheckMissingPrefix) {

Nit (not blocking): on a 403 the loop in automerge_contributor_held_runs.go:88-95 keeps trying each held run and warns per run; #14519 asks for "logged once per head". Breaking out of the loop on isGitHubStatus(err, http.StatusForbidden) would match that wording. The approve forbidden test currently asserts both runs are attempted, so adjust wantApproved there if you take this.

Verified: registry-only gating via claim.admittedByRegistry() (lane.go:531), label/workflow-path fences re-checked defensively (held_runs.go:52-58), fork head matched by head_repository.full_name (held_runs.go:120-131), comment marker dedupe via paginated listIssueComments (lane.go:1089). Table tests cover fork/same-repo/auto-promoted/marker/sentinel/exclude/workflows/toggle/no-runs/403 across two ticks.

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

[hivecommons-hive]

Reviewed — no findings from this perspective.

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

[hivecommons-hive]

Logic reads correct to me; CI is red for a reason this PR owns — the new config leaf is missing from the Settings UI parity check.

Finding (medium) — test (dashboard 4/4) / dashboard shuffle 2/3 / coverage

TestSettingsDialogReferencesEveryHiveYAMLLeaf (src/pkg/dashboard/settings_parity_test.go:33) fails on every shard that runs it:

hive.yaml settings are not referenced by the Settings UI and are not allowlisted:
        auto_merge.contributor_lane.approve_held_runs_for_registry_trusted

The PR adds the leaf in src/pkg/config/config.go:7747 and plumbs it through /api/config/auto-merge GET/PUT (src/pkg/dashboard/api_config_automerge.go:172,326,428), but src/pkg/dashboard/static/index.html never mentions it — the sibling registry_tiers_sufficient is wired at index.html:49639, 49848, 51365, 52161, and this toggle has no equivalent. Either add the Settings toggle alongside those four sites (the API side is already done, so this is the consistent fix) or add the path to settings_parity_allowlist.txt. coverage fails only because that shard failed ("Coverage requires successful test shards (... sliced=failure ...)"), so fixing the parity test should turn the run green.

What I verified in the tree

• approveHeldRunsForRegistryTrusted (automerge_contributor_held_runs.go:47-105) is only reached from evaluateContributorLanePR after prefilterContributorLanePR, contributorLaneDiffReason (workflow-path fence returns at automerge_contributor_lane.go:1210), reporter-trust hold and changes-requested gates — so the in-helper re-checks are genuinely defensive, as the body says.
• Admission is gated on claim.admittedByRegistry() only; claim-marker/auto-promotion/association paths return nil at :50.
• heldWorkflowRunsForHead (:124-136) filters on action_required + head-repo full name, so a fork head is matched by the fork's name — matches the stated intent and differs deliberately from heldSameRepoWorkflowRuns (automerge_held_runs.go:561).
• The once-per-head mark is set before the API calls (:84-86), so a 403 does not retry every tick; the audit comment is suppressed by marker scan + effects claim (:195-218).

Open question (low, not a defect)

evaluateContributorLanePR is documented as read-only when relay=true (automerge_contributor_lane.go:1147-1150), but the new call at :1256-1258 runs regardless of relay, so a merge-request relay evaluation can now approve runs and post a comment. The sweep would do the same within a tick and the once-per-head mark prevents duplication, so I don't think it's wrong — just worth a one-line note in that doc comment, or a !relay guard if you'd rather keep the relay side-effect-free.

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

[hivecommons-hive]

Logic looks correct to me; the red CI is a verified, self-contained omission — the new config leaf is missing from the Settings UI / parity allowlist.

Finding (medium) — TestSettingsDialogReferencesEveryHiveYAMLLeaf fails on this head

Both red shards (test (dashboard 4/4), test (dashboard shuffle 2/3), which also fail coverage and test) are the same failure:

settings_parity_test.go:33: hive.yaml settings are not referenced by the Settings UI and are not allowlisted:
    auto_merge.contributor_lane.approve_held_runs_for_registry_trusted

Mechanism: src/pkg/config/config.go:7747 adds the approve_held_runs_for_registry_trusted leaf, and src/pkg/dashboard/api_config_automerge.go:172,326,428 plumb it through GET/PUT — but src/pkg/dashboard/settings_parity_test.go:27 requires every Config leaf to be referenced in static/index.html or listed in settings_parity_allowlist.txt. Neither mentions the new key (the contributor-lane help text at static/index.html:49848 lists the other eight keys and has no checkbox for this one). Fix is either a toggle in the contributor-lane Settings card (the PUT handler already accepts the field) or one line in settings_parity_allowlist.txt; the former matches the PR body's "dashboard GET/PUT" intent.

What I verified in the tree (no defects)

• automerge_contributor_held_runs.go:48-62: gated on policy.ApproveHeldRunsForRegistryTrusted, claim.admittedByRegistry() and the required-check-missing: prefix. admittedRegistryTier (automerge_contributor_lane.go:579-597) returns "" unless the tier is in policy.RequireTiers, so "registry-admitted" really does mean "a tier the lane would merge for" — the header comment's claim holds.
• Call site automerge_contributor_lane.go:1256: files is already fetched and contributorLaneDiffReason(files) has already returned at :1206, so the .github/workflows/** refusal at :59 is a true defensive re-check, and the sentinel/exclude/hold label checks mirror the sweep's labelBlockReason (automerge_sweep.go:1919).
• heldWorkflowRunsForHead uses the same action_required conclusion match as heldSameRepoWorkflowRuns (automerge_held_runs.go:564), differing only in accepting the fork's full name — which is the point of the change.
• Once-per-head bookkeeping reuses Engine.heldRunApprovals with its own registry-held-approve: prefix, matching botTakeoverKey/rebaseNudgeKey; marking before the API call means a 403 (missing Actions: write) is logged once and never retried for that head until a new push — consistent with what github-app-setup.md now says.
• Security read: the approve call runs under the App token only for authors the operator explicitly placed at a mergeable tier, never for claim-marker/auto-promotion/association admissions, and never when the diff touches workflows. The toggle defaults on; an operator who does not want fork runs approved unattended must set it false — the docs say so.

Not a defect, just a note: the audit comment body embeds author straight from pr.GetUser(); it's a GitHub login so it's safe, but it is interpolated into a <!-- … --> marker line.

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

[hivecommons-hive]

Style / maintainability review (read at head f35e65b).

1. heldWorkflowRunsForHead is a verbatim copy of heldSameRepoWorkflowRuns — src/pkg/github/automerge/automerge_contributor_held_runs.go:132-144 vs automerge_held_runs.go:561-573. Same loop, same action_required + HeadRepository.FullName EqualFold filter; only the doc comment differs. #14519 explicitly asked to "reuse the detection … in automerge_held_runs.go". Two identical filters will drift (e.g. if the bot path ever starts tolerating a nil head repository). Suggest dropping the new function and calling the existing one with the fork-aware head name:

--- a/src/pkg/github/automerge/automerge_contributor_held_runs.go
+++ b/src/pkg/github/automerge/automerge_contributor_held_runs.go
@@
-	held := heldWorkflowRunsForHead(runs.WorkflowRuns, contributorLaneHeadRepo(pr, owner, repo))
+	// Unlike the bot path, the expected head repository may be a fork: the
+	// author's registry tier, not the head's origin, admits the approval.
+	held := heldSameRepoWorkflowRuns(runs.WorkflowRuns, contributorLaneHeadRepo(pr, owner, repo))
@@
-// heldWorkflowRunsForHead returns the runs waiting on approval whose head
-// repository is headRepo. Unlike heldSameRepoWorkflowRuns the expected head
-// repository may be a fork: the PR author's registry tier, not the head's
-// origin, is what admits the approval.
-func heldWorkflowRunsForHead(runs []*gh.WorkflowRun, headRepo string) []*gh.WorkflowRun {
-	var held []*gh.WorkflowRun
-	for _, run := range runs {
-		if run == nil || !strings.EqualFold(run.GetConclusion(), workflowRunConclusionActionRequired) {
-			continue
-		}
-		if run.GetHeadRepository() == nil || !strings.EqualFold(run.GetHeadRepository().GetFullName(), headRepo) {
-			continue
-		}
-		held = append(held, run)
-	}
-	return held
-}

and in automerge_held_runs.go:559-560 widen the doc comment ("whose head repository is fullName — the base repo for the bot lanes, the PR head (fork included) for the contributor lane"). TestHeldWorkflowRunsForHeadMatchesForkHead (automerge_contributor_held_runs_test.go:220) then targets heldSameRepoWorkflowRuns, which is still worth keeping since the fork-name case is new coverage.

2. Doc sentence now contradicts the paragraph it ends — src/docs/github-app-setup.md:46. The paragraph opens by describing the new direct fork-head approval, then closes with "Hive never approves fork heads or PRs carrying sentinel-alert." A reader skimming the permissions table takes the last sentence as the rule.

Outside that contributor-lane exception, Hive never approves fork heads, and it never approves PRs carrying `sentinel-alert`.

Advisory (no verdict effect): the requiredCheckMissingPrefix guard is checked both at the call site (automerge_contributor_lane.go:1256) and inside the helper (automerge_contributor_held_runs.go:53); and the helper's sentinel/exclude/workflow-path re-checks (:56-62) are unreachable from the lane (it returns at automerge_contributor_lane.go:495 / :1209 first), so the three "refuses" table cases exercise the lane's gates, not the helper's. Fine as defence-in-depth per the PR body; just noting the tests don't cover those branches.

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

[hivecommons-hive]

@clubanderson this PR is held at stage/071-needs-author and the hive will not merge it yet.

Why:

• the review swarm requested changes

Next: the hive does not push to your branch. Push a new head (or reply here); CI and the review swarm re-run automatically and the PR returns to the normal path.

[hivecommons-hive]

Evidence bundle `#14520f35e65b` sha256:b1f20c58e44d0b51e64eb1d9845f2aeaa61a97254f85b3d77a4ba3db3fd235d3 — download from the dashboard or `hivectl review evidence #14520`.

[hivecommons-hive]

merged by Hive trusted-author auto-merge: author @clubanderson holds role owner and GitHub permission admin on repo; CI green at f35e65b.

[github-actions]

Thank you for your contribution! Your PR has been merged.

We'd love to hear how your experience was: share feedback