Skip to content

πŸ› Write-back containment: per-kind gate for every unsolicited GitHub write, with backfill guard - #15122

Merged
clubanderson merged 13 commits into
v5from
fix/writeback-containment
Oct 29, 2026
Merged

clubanderson merged 13 commits into
v5from
fix/writeback-containment

Conversation

@clubanderson

Copy link
Copy Markdown
Member

Fixes #15121

What

One central write-back gate (pkg/writeback) that every unsolicited GitHub comment/label/review from the platform sweeps consults before touching a PR or issue hive did not open. Per write-back kind: an ACMM floor and whether the review opt-in label (review.require_labels, fallback hive-review) is needed; per-repo mode override (off | issues-only | labels-only | full); backfill guard with persisted first_seen/writable_at so a newly-writable repo never back-fills onto pre-existing PRs.

Default matrix (overridable under writeback.kinds.<kind>):

Kind L4 L5 L6
sentinel dashboard only opt-in label all PRs
hold_gate_notice hive PRs only hive PRs only hive PRs only
stage_label, needs_decision_comment, supersession_notice, duplicate_fix_notice, reporter_trust_hold, covered_by_pr_label, issue_close, parent_issue_edit, gate_comment β€” opt-in label opt-in label
supersession_close, contributor_adopt, ci_rerun β€” β€” opt-in label
claim_comment opt-in label opt-in label opt-in label
welcome_comment β€” opt-in label all
issue_file yes yes yes

Gated in this PR

Sentinel sweep + webhook, hold guard (drift comment / re-hold / mergeability push β€” hive-authored only), PR stage labels + needs-decision comments, supersession sweep (notice + close), duplicate-fix sweep, reporter-trust reconcile. Below the gate: sentinel findings go to Security β†’ Sentinel findings and /api/sentinel; hold-gate notices are dashboard-only; PR stage tracking continues internally.

Dashboard / API

βš™ β†’ Features β†’ Autonomy & ACMM β†’ Write-back containment: the three knobs, per-repo mode, per-kind matrix, 24h suppressed counter with drill-down, backfill banner. /api/status.writeback, GET /api/sentinel, GET /api/writeback/suppressed (documented in dashboard/openapi.json).

Proof

  • TestWritebackA1ReplayTorchSpyreStaysSilent: L4, torch-spyre not opted in, 50 pre-existing workflow-touching PRs β†’ zero GitHub writes, 50 internal findings, exactly one writeback: skipped 50 pre-existing items on torch-spyre/torch-spyre (backfill off) line, banner present; second sweep still silent.
  • TestWritebackA1ReplayOptedInRepoStillGetsSentinel: spyre-inference PR labelled hive-review at L5 gets comment + label; unlabelled one does not; unlabeled_min_level override lowers it.
  • TestWritebackPerRepoIssuesOnlyBlocksSentinel, TestWritebackGatedPathsMatrix (table-driven over every kind Γ— L4/5/6 Γ— unlabeled/labeled/own), pkg/writeback unit tests (97% coverage), TestDirectWriteRatchet with baseline testdata/direct-writes-baseline.txt (leftovers listed in the issue).

Docs

operator-reference.md "What hive writes to GitHub, and when" (inventory table), acmm-policy-matrix.md "What hive will say on GitHub at each level", agent-configuration.md, labels-and-control-signals.md, github-write-surface.md, security-model.md, hive.yaml.example, changelog fragment.

@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Oct 28, 2026
@kubestellar-prow

Copy link
Copy Markdown
Contributor

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

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

@kubestellar-prow kubestellar-prow Bot added the size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. label Oct 28, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor

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

@github-actions

github-actions Bot commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

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
#15186 πŸ› Activity Monitor: one coherent post-decision state for Let hive decide rows; 060 reviewed in CI / Review CONFLICT src/docs/api-reference.md
#15134 πŸ› Delegated decision on a capability gap: model the blocker, stop the un-park/re-park loop, name the missing App permission CONFLICT src/docs/api-reference.md
#15104 πŸ› Activity Monitor: Rebase, then merge as maintainer; fixer owns parked conflicting PRs; dismiss self-inflicted changes_requested CONFLICT src/docs/api-reference.md
#15240 πŸ› Never adopt a person's PR: owner/human-authored PRs return to their author instead of the contributor-offline lane same files src/pkg/dashboard/server.go
src/pkg/dashboard/static/index.html
#15270 πŸ› fix(kick-logs): archive kick log when the turn ends, not only at the next rotation point same files src/pkg/dashboard/static/index.html
#15267 πŸ› tracker: closed superseded PRs no longer render as rows same files src/pkg/dashboard/static/index.html
#15256 πŸ› clanker PRs with a human changes-requested review show Returned to contributor, not Sentinel alert same files src/docs/api-reference.md
src/pkg/dashboard/static/index.html
#15253 πŸ› decision toast: issue decisions no longer read as 'approved for merge' same files src/pkg/dashboard/static/index.html
#15219 πŸ› fix: preserve human and owner PRs during delegated decisions same files src/internal/testutil/dashboard_import_allowlist.txt
src/pkg/dashboard/static/index.html

@hivecommons-hive hivecommons-hive Bot added author/human A person pushed this PR themselves dco All commits are DCO signed-off role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 28, 2026
@hivecommons-hive hivecommons-hive Bot added stage/071-needs-author Human-authored PR: CI red, changes requested or conflicting; diagnosis posted, the author must act and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 28, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor

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

Failing checks:

  • relative links and anchors resolve

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

Copy link
Copy Markdown
Contributor

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

Failing checks:

  • relative links and anchors resolve

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 hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/071-needs-author Human-authored PR: CI red, changes requested or conflicting; diagnosis posted, the author must act labels Oct 28, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Docs currency β€” read at head 5ff93f0. The matrix in acmm-policy-matrix.md, the knobs in operator-reference.md/hive.yaml.example, the state path/env var, the audit action and the log/banner strings all match pkg/writeback (kinds.go:128-147, config.go, gate.go:312-316). Two gaps:

  1. src/docs/api-reference.md has no rows for the two new endpoints. GET /api/writeback/suppressed and GET /api/sentinel are registered at src/pkg/dashboard/api.go:523-524, and the PR body says they are "documented in dashboard/openapi.json" (they are), but the PR only renumbered existing citations in api-reference.md; neither path appears there at head. Suggested rows after the /api/acmm-recommendation row (api-reference.md:293), matching the handlers' query params (api_writeback.go handleWritebackSuppressed/handleSentinel):
 | `GET` | `/api/acmm-recommendation` | Dashboard auth/session | Advisory level-up recommendation (`acmmadvisor.Recommendation`, JSON): never changes the applied level β€” see [ACMM advisor](acmm-advisor.md) | `pkg/dashboard/api.go:343` |
+| `GET` | `/api/writeback/suppressed` | Dashboard auth/session | Unsolicited GitHub writes the write-back containment gate (`pkg/writeback`) refused in the last 24 h, newest first, with what would have been written; `?repo=owner/name`, `?kind=<write-back kind>`, `?limit=N` (default 200). Drill-down for βš™ β†’ Features β†’ Write-back containment; see [operator-reference.md](operator-reference.md#what-hive-writes-to-github-and-when) | `pkg/dashboard/api.go:523`, `pkg/dashboard/api_writeback.go` |
+| `GET` | `/api/sentinel` | Dashboard auth/session | Sentinel findings for this process lifetime (max 1000, newest first, one per `repo#number@head`), including ones the write-back gate kept off GitHub (`suppressed: true`); `?repo=owner/name`, `?suppressed=true|false` | `pkg/dashboard/api.go:524`, `pkg/dashboard/api_writeback.go` |
  1. dashboard/openapi.json:33534 names a kind that does not exist. The kind filter example says sentinel_comment, but the kind is sentinel (src/pkg/writeback/kinds.go:35); ?kind=sentinel_comment matches nothing.
            "description": "Restrict to one write-back kind (e.g. sentinel, stage_label)"

β€” hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intent alignment (read at head 5ff93f0) β€” the six paths #15121's "Fix" section names (sentinel sweep, hold guard, stage labels on PRs, supersession, duplicate-fix, reporter-trust) are all gated and wired at boot (src/cmd/hive/github_client_config.go applyGitHubClientConfigHooks β†’ client.SetWritebackGate). One claim in the body/docs is not in the diff:

needs_decision_comment is declared but never consulted. writeback.KindNeedsDecisionComment exists (src/pkg/writeback/kinds.go, DefaultKinds: L5 + label) and the PR body ("PR stage labels + needs-decision comments"), src/docs/acmm-policy-matrix.md (stage_label, needs_decision_comment on foreign PRs β†’ opt-in label) and src/docs/operator-reference.md (inventory row: hive:stage-needs-decision comment β†’ L5 + opt-in label) all say it is gated. But the only writer of that comment, Engine.postHumanEntry in src/pkg/github/automerge/automerge_stage_human.go:362 (c.gh.Issues.CreateComment with stagelabels.DecisionComment(route)), is untouched and is still listed as a raw leftover in src/pkg/writeback/testdata/direct-writes-baseline.txt (raw pkg/github/automerge/automerge_stage_human.go 1). Same for the stall note at automerge_stage_labels.go:1117. The stage labels on that PR are kept off by stageWriter.Apply (src/pkg/github/stage_writer.go, sets out.Suppressed), but the caller at automerge_stage_labels.go:1128 ignores out.Suppressed and posts the 085 comment anyway β€” so a torch-spyre contributor PR routed to 085 at L4 still gets a hive comment.

Smallest fix that reuses what this PR already added β€” honour Outcome.Suppressed in the automerge caller:

--- a/src/pkg/github/automerge/automerge_stage_labels.go
+++ b/src/pkg/github/automerge/automerge_stage_labels.go
@@ -1111,6 +1111,13 @@
 		if out.Changed() {
 			c.info("stage labels reconciled", ...)
 		}
+		if out.Suppressed {
+			// Write-back gate kept stage labels off this foreign PR
+			// (writeback.KindStageLabel); the stall note and 085/090 entry
+			// comments (KindNeedsDecisionComment) must stay off it too.
+			c.info("stage labels: entry/stall comments kept off GitHub by write-back gate", "repo", obs.Display, "pr", obs.Number)
+			continue
+		}
 		if want.Stalled && want.Stage.StallNote() {

Note Suppressed is only set when a label write was attempted, so a visit with no label delta still needs a direct WritebackAllowed(..., KindNeedsDecisionComment, body) in postHumanEntry; if you'd rather defer that to the follow-up PR, trim the claim from the body, acmm-policy-matrix.md and operator-reference.md instead. Either way, lower the baseline line when the comment moves behind the gate.

β€” hive: agent=adjudicator-3 backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style / maintainability review (head 5ff93f0; read pkg/writeback, pkg/github/writeback_gate.go and the gated sweeps).

1. min_level "default" is displayed as 5 but never applied β€” a no-op dashboard save silently tightens three kinds (src/pkg/writeback/config.go MinLevelOrDefault/Policy; src/pkg/dashboard/api_governor_features_platform.go "writebackMinLevel": cfg.Writeback.MinLevelOrDefault())
Policy() only raises per-kind floors when Unsolicited.MinLevel != nil (comment: "only an explicit writeback.unsolicited.min_level raises the per-kind matrix"), yet MinLevelOrDefault() reports DefaultMinLevel = 5 when unset and the Features form renders that as the current value. The matrix next to it shows claim_comment L4, hold_gate_notice L4, verdict_label L3 β€” contradicting the "5" above it. Worse, saving the form unchanged PUTs writebackMinLevel: 5, which becomes an explicit floor and lifts claim_comment/verdict_label to L5; PUTting 0 clears it and the form shows 5 again, so the round trip is not idempotent. The DefaultMinLevel doc ("the floor under which no unsolicited write of any kind lands") also describes behaviour the code does not implement. Suggest making "unset" observable as 0/none and keeping DefaultMinLevel as matrix documentation only:

 // MinLevelOrDefault resolves unsolicited.min_level.
+// 0 means no global floor: the per-kind matrix alone applies.
 func (c Config) MinLevelOrDefault() int {
 	if c.Unsolicited.MinLevel == nil {
-		return DefaultMinLevel
+		return 0
 	}
 	return *c.Unsolicited.MinLevel
 }

and in index.html renderWritebackContainmentSection drop the || 5 fallback (defMin) so the input is empty/placeholder "matrix" when no floor is set. Adjust TestWritebackConfigDefaults (writeback_test.go, the MinLevelOrDefault() != writeback.DefaultMinLevel assertion) accordingly.

2. stageLabeler.writeAllowed comment contradicts the code (src/pkg/github/stage_labeler.go)
Doc says "A PR the labeler never read (no facts) is gated with what it knows: number only", but the body returns true (ungated) on !ok. Either gate it (writeback.Item{Number: number, IsPR: true} fails closed via ReasonNeedsLabel) or fix the comment; the current text will mislead the next person migrating a kind.

// (hivecommons/hive, torch-spyre 2026-10-28). A PR the labeler never read
// (no facts) cannot be told from an issue, so it is not gated here.

3. sentinelWritebackItem is now the generic go-github β†’ writeback.Item adapter (src/pkg/github/sentinel_sweep.go; callers in pr_supersession_sweep.go, pr_duplicate_fix_sweep.go, pr_reporter_trust.go)
Three non-sentinel sweeps call a helper named for sentinel, while its typed twin WritebackItemForPR(pr PullRequest) lives in writeback_gate.go. Rename to writebackItemForGHPR and move it beside WritebackItemForPR so the two adapters are discoverable together.

4. Ratchet exemption is self-certifying (src/pkg/writeback/ratchet_test.go countLines)
Any line containing the string writeback:gated is skipped, with no check that a gate call exists. The issue's contract is "a ratchet test fails on new direct write calls outside it"; a copy-pasted trailer defeats it silently. Cheap tightening: when a file contains writeback:gated, require it to also contain WritebackAllowed( / writeAllowed( / .Allowed(, else t.Errorf("%s: writeback:gated annotation without a gate call", rel). Advisory.

5. RepoModeFor is order-dependent with overlapping keys (src/pkg/writeback/config.go)
Iterating c.PerRepo (a map) and returning the first suffix match means torch-spyre: off and torch-spyre/torch-spyre: full resolve nondeterministically. Prefer an exact-key lookup first, then a sorted suffix scan.

β€” hive: agent=adjudicator-4 backend=copilot model=claude-fable-5.1 copilot=1.0.88

…ts card

Adds a per-repo Write-back mode selector (default / off / issues-only /
labels-only / full) and a 'writes: <mode>' chip to each repo header on the
Projects card. Both the selector and the Settings textarea read and write
the same writeback.per_repo map via /api/config/governor/features and
re-render from /api/status on save. Adds renderer and API round-trip tests.

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
…helper and style tokens

- direct-write ratchet: baseline automerge_adoption_close_first.go and the
  extra api_repo_item_decision.go wrap that landed on v5 today
- dashboard import ratchet: allow pkg/writeback
- Projects-card chip/select: spacing and font-size tokens instead of raw px/rem
- suppressed-writes drill-down: formatDashboardDateTime instead of toLocaleString

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
…atchet baseline

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
…ck.Config no longer recurses into config.Config

TestSettingsDialogReferencesEveryHiveYAMLLeaf walked config.Config.Writeback
(a writeback.Config) by bare type name, matched the root Config, and recursed
with an ever-growing prefix until the test binary was OOM-killed β€” which is
what took the CI runners down on the dashboard and rest lanes. Qualified
types now resolve through the declaring file's imports (module-relative
path), never by bare name; pkg/writeback is collected so its leaves are
enumerated, the Settings matrix/per-repo controls carry their setting keys,
and the two read-only policy knobs are allow-listed.

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
… the decision-reroute wrapper writes

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
… qualified-type test

Signed-off-by: Andrew Anderson <andy@clubanderson.com>
Signed-off-by: Andrew Anderson <andy@clubanderson.com>
@hivecommons-hive hivecommons-hive Bot added stage/071-needs-author Human-authored PR: CI red, changes requested or conflicting; diagnosis posted, the author must act and removed stage/050-review-pending Review swarm requested; awaiting a verdict for the current head labels Oct 29, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor

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

Why:

  • the branch conflicts with the base and needs a rebase

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.

@github-actions github-actions Bot removed the conflicts-with-open-pr Open PR has a merge conflict with another open PR label Oct 29, 2026

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Delta re-review (docs-currency) β€” read head 7e8b633 vs prior 5ff93f0. The PR-own delta is a rebase onto v5 plus citation refreshes (71b31e2, d43516e, 7e8b633); the refreshed citations check out (e.g. src/docs/api-reference.md:288 β†’ pkg/dashboard/api.go:338, :244 β†’ api.go:514). Both previous docs-currency findings are still present:

  1. src/docs/api-reference.md has no rows for the two new routes. src/pkg/dashboard/api.go:524-525 registers GET /api/writeback/suppressed and GET /api/sentinel; the page header (api-reference.md:3) requires a row "in the same change that adds or renames a route", and grep -i writeback on the file at head returns nothing. Suggested rows, after the PUT /api/config/governor/compliance row at api-reference.md:182:
| `GET` | `/api/writeback/suppressed` | Dashboard auth/session | Write-back containment drill-down: the unsolicited GitHub writes the central gate (`pkg/writeback`) suppressed in the last 24h, newest first. Query `repo` (`owner/name`), `kind` (a `pkg/writeback` kind, e.g. `sentinel`, `stage_label`) and `limit` (1–2000, default 200; `400` otherwise). Returns `{enabled, items[], total_24h, status}`. | `pkg/dashboard/api.go:524`, `pkg/dashboard/api_writeback.go` |
| `GET` | `/api/sentinel` | Dashboard auth/session | Sentinel findings the hive holds in memory, whether or not the write-back gate let them reach GitHub. Query `repo` (`owner/name`) and `suppressed=true\|false` (`400` for any other value). Returns `{findings[], total, suppressed, writeback}`. | `pkg/dashboard/api.go:525`, `pkg/dashboard/api_writeback.go` |
  1. dashboard/openapi.json:33551 still documents a kind that does not exist. The kind filter example says sentinel_comment, but src/pkg/writeback/kinds.go:35 defines KindSentinel Kind = "sentinel" and the handler compares the raw string (api_writeback.go:72: string(it.Kind) != kind), so ?kind=sentinel_comment silently matches nothing.
            "description": "Restrict to one write-back kind (e.g. sentinel, stage_label)"

β€” hive: agent=ci-maintainer backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Intent alignment β€” delta re-review (5ff93f0 β†’ 7e8b633)

Delta is a rebase onto v5 plus three PR-authored changes, all tightly coupled to the issue: internal/settingsparity/settingsparity.go:17-60,178-215 (qualified-type resolution so config.Config.Writeback resolves to writeback.Config, with the regression test in settingsparity_test.go), pkg/dashboard/settings_parity_allowlist.txt:512-513, and the ratchet baseline absorbing v5-landed adoption writers (pkg/writeback/testdata/direct-writes-baseline.txt:7,10,68-69). The PR's own 45 files stay inside the #15121 contract; no scope creep.

Previous findings at the new head:

  • Still present β€” src/docs/api-reference.md has no rows for GET /api/writeback/suppressed and GET /api/sentinel (registered at pkg/dashboard/api.go:524-525); the three "refresh citations" commits only re-numbered existing rows. Add near line 244:
    | `GET` | `/api/writeback/suppressed` | Dashboard auth/session | Last-24h ring of unsolicited GitHub writes the write-back gate held back. Query `repo` (`owner/name`), `kind` (a `writeback` kind such as `sentinel`, `stage_label`), `limit` | `pkg/dashboard/api.go:524`, `pkg/dashboard/api_writeback.go:50` |
    | `GET` | `/api/sentinel` | Dashboard auth/session | Sentinel findings whether or not they reached GitHub (Security β†’ Sentinel findings). Query `repo`, `suppressed=true\|false` | `pkg/dashboard/api.go:525`, `pkg/dashboard/api_writeback.go:149` |
    
  • Still present β€” dashboard/openapi.json:33551 example names kind sentinel_comment; no such kind exists (pkg/writeback/kinds.go:35 is sentinel).
              "description": "Restrict to one write-back kind (e.g. sentinel, stage_label)"
    
  • Still present (unchanged files) β€” pkg/writeback/config.go:108-118 RepoModeFor map-iteration order; pkg/github/stage_labeler.go:50-62 doc vs return true on unread PRs; pkg/writeback/ratchet_test.go:91-93 string-only writeback:gated exemption; writeback.unsolicited.min_level default round-trip; sentinelWritebackItem naming.

Advisory: baselining automerge_adoption_close_first.go (7 raw writes, closes a contributor's original PR) is consistent with the issue's declared follow-up for kind contributor_adopt, but it is another unsolicited foreign-PR write left outside the gate at merge.

β€” hive: agent=adjudicator-3 backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Hive filed review findings that do not block this PR as backlog items:

Cap: 10 backlog item(s) per PR per day.

Signed-off-by: Andrew Anderson <andy@clubanderson.com>

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Style β€” delta re-review at 7e8b633 (previous review at 5ff93f0). The delta is the rebase on v5, the settingsparity qualified-type resolver (src/internal/settingsparity/settingsparity.go:17-60, 177-209), three direct-writes-baseline.txt bumps, CSS tokens and data-setting-key hooks in index.html. The new settingsparity code reads cleanly and its test (settingsparity_test.go:184-230) pins the regression. None of the previously flagged files changed, so those findings stand:

Still present

  1. src/pkg/github/stage_labeler.go:54-61 β€” doc says a PR the labeler never read "is gated with what it knows: number only", but if !ok || !item.IsPR { return true } lets it through ungated. Make the comment say what the code does (the item type is unknown without facts, so this is the honest statement):
// (hivecommons/hive, torch-spyre 2026-10-28). An item the labeler never
// read (no facts) cannot be told from an issue and is not gated here.
  1. src/pkg/dashboard/api_governor_features_platform.go:416 reports writebackMinLevel: cfg.Writeback.MinLevelOrDefault() (5 when unset), while the write path at :223-233 stores any non-zero value as an explicit pointer, and src/pkg/writeback/config.go:145-147 only applies the floor when the pointer is set. Saving the Settings tab unchanged therefore flips every non-own-only kind below 5 up to 5. Report the raw value and expose the default separately so an untouched form round-trips:
		"writebackMinLevel":      cfg.Writeback.Unsolicited.MinLevel,
		"writebackMinLevelDefault": writeback.DefaultMinLevel,
  1. src/pkg/writeback/config.go:110-115 β€” RepoModeFor ranges over the PerRepo map and returns the first suffix match; with both foo/bar and baz/bar keyed, bar resolves nondeterministically. Sort the keys (as Validate already does at :178-182) or prefer an exact match before suffix matches.

  2. src/pkg/writeback/ratchet_test.go:89-99 β€” countLines exempts any line containing writeback:gated/writeback:solicited without checking that a gate call is on it; an annotation alone retires a raw write from the baseline. Low; noted as previously.

  3. src/pkg/github/sentinel_sweep.go β€” sentinelWritebackItem is still the shared go-github adapter used by supersession, duplicate-fix and reporter-trust (pr_supersession_sweep.go, pr_duplicate_fix_sweep.go, pr_reporter_trust.go); the name should say what it is. Low.

Docs-currency items (api-reference rows for GET /api/writeback/suppressed / GET /api/sentinel, sentinel_comment in dashboard/openapi.json) are also unchanged at this head β€” see that perspective.

β€” hive: agent=adjudicator-4 backend=copilot model=claude-fable-5.1 copilot=1.0.88

@clubanderson
clubanderson force-pushed the fix/writeback-containment branch from 7e8b633 to 1e55375 Compare October 29, 2026 03:17

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Security β€” delta re-review at 7e8b633 (rebase onto v5; PR-own delta = commits 5edda00…7e8b633: ratchet baseline, settingsparity, index.html tokens/date formatting, api-reference). Nothing in the delta widens a write path; the baseline growth (automerge_adoption_close_first.go 7, automerge_contributor_adopt.go 13β†’15, api_repo_item_decision_reroute.go 5 in src/pkg/writeback/testdata/direct-writes-baseline.txt) covers v5 code in kinds #15121 explicitly defers.

1. Docs state containment guarantees the gate does not enforce (low, in-scope). src/docs/acmm-policy-matrix.md:145, src/docs/operator-reference.md:951 and :957, and src/docs/github-write-surface.md present contributor_adopt, ci_rerun, claim_comment, issue_close, parent_issue_edit, gate_comment, welcome_comment, covered_by_pr_label, verdict_label as enforced defaults ("contributor_adopt β†’ L6 + label"). At this head the only production WritebackAllowed/Allowed call sites are for sentinel (sentinel_sweep.go), hold_gate_notice (cmd/hive/main_helpers.go), stage_label (stage_labeler.go:50-66), supersession_notice/supersession_close (pr_supersession_sweep.go), duplicate_fix_notice, reporter_trust_hold. KindContributorAdopt/KindCIRerun (src/pkg/writeback/kinds.go:66,86,135,142) have policies but no caller: automerge_contributor_adopt.go:923 β†’ automerge_adoption_close_first.go:117-124 still comments on, labels and closes a contributor's PR via raw go-github calls, and shared_ci_rerun.go has no gate call. #15121 itself lists these as "Follow-up PRs migrate kinds … onto the gate", so the code is on contract β€” the docs are not. An operator at L5 reading the matrix would believe hive cannot adopt/close an unlabeled contributor PR on that repo; it can. Please mark the deferred rows, e.g. in acmm-policy-matrix.md:145:

| `supersession_close` | β€” | β€” | β€” | opt-in label |
| `contributor_adopt`, `ci_rerun`, `claim_comment`, `issue_close`, `parent_issue_edit`, `gate_comment`, `welcome_comment`, `covered_by_pr_label`, `verdict_label` | *policy defined, not yet enforced* β€” these paths still write directly (baseline in `pkg/writeback/testdata/direct-writes-baseline.txt`); migration is the #15121 follow-up | | | |

and the matching rows in operator-reference.md:951,957.

Previous findings at this head: openapi sentinel_comment β€” fixed (string no longer present); api-reference rows for GET /api/writeback/suppressed / GET /api/sentinel (api.go:524-525) β€” still missing despite the citation-refresh commits; config.go min_level/RepoModeFor map iteration (src/pkg/writeback/config.go:108-118), stage_labeler.go:50-57 doc-vs-code, ratchet annotation exemption (ratchet_test.go:91) β€” untouched by the delta, still present.

β€” hive: agent=adjudicator-2 backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correctness β€” delta re-review at 7e8b633 (previous head 5ff93f0)

1. settingsparity bare-identifier lookup can resolve a pkg/config struct to a same-named pkg/writeback struct β€” src/internal/settingsparity/settingsparity.go:188-191 (resolve, *ast.Ident case) and :251 (c.byName[...] = sd, last declaration wins; pkg/writeback is collected last at :49).

The new resolver fixes qualified types (writeback.Config no longer collides with config.Config), but a bare identifier still goes through the cross-package byName map, and this PR adds pkg/writeback to that map. Any struct name shared by pkg/config and pkg/writeback now silently yields the writeback struct's fields for the config-side leaf. Reproduced by compiling this file into a scratch module and feeding ConfigLeaves a fixture where both packages declare Policy: the config-side Repos map[string]Policy produced repos.<key>.own_only (writeback's field) instead of repos.<key>.x_config. The new test TestConfigLeavesQualifiedTypeDoesNotCollideWithRootConfig (settingsparity_test.go:185) only covers the qualified case, so this would not be caught. Consequence: the parity gate reports phantom/missing leaves with no error β€” exactly the silent drift the allowlist is meant to catch. I could not read pkg/writeback/config.go at this head to confirm an actual collision exists today, so this is a latent hazard rather than an observed CI break; it is cheap to close.

Suggested fix β€” prefer the declaring directory for bare identifiers, then fall back to the cross-package map (keeps the crux/config alias re-export behaviour):

 type structDecl struct {
 	st      *ast.StructType
 	pkgName string
+	dir     string            // "<dir relative to src>" of the declaring file
 	imports map[string]string
 }
@@ func (c *collected) resolve(expr ast.Expr, imports map[string]string) (*structDecl, string, bool) {
-func (c *collected) resolve(expr ast.Expr, imports map[string]string) (*structDecl, string, bool) {
+func (c *collected) resolve(expr ast.Expr, from *structDecl) (*structDecl, string, bool) {
+	imports := map[string]string{}
+	if from != nil {
+		imports = from.imports
+	}
 	switch t := expr.(type) {
 	case *ast.Ident:
+		if from != nil && from.dir != "" {
+			if decl, ok := c.byDir[from.dir+"."+t.Name]; ok {
+				return decl, t.Name, true
+			}
+		}
 		decl, ok := c.byName[t.Name]
 		return decl, t.Name, ok
@@ in collect()
-				sd := &structDecl{st: st, pkgName: file.Name.Name, imports: imports}
+				sd := &structDecl{st: st, pkgName: file.Name.Name, dir: filepath.ToSlash(rel), imports: imports}

(and thread the current *structDecl instead of imports through walkStruct/walkNamed/inlineStruct; anonymous structs inherit the enclosing decl). Add a test with Policy declared in both fixture packages asserting repos.<key>.x_config.

Limits of this re-review. GitHub reads hit the hourly API cap mid-review. I diffed 28 of the PR's 46 files between the two heads locally (incl. settingsparity*, pkg/config/writeback*, dashboard/api_writeback*, github/client.go, allowlists); the pkg/config/writeback.go and dashboard/api_writeback.go content is unchanged since 5ff93f0. I could not fetch src/pkg/writeback/*, github/stage_labeler.go, github/sentinel_sweep.go, github/writeback_gate.go, docs/api-reference.md or dashboard/openapi.json at 7e8b633, so the seven findings from the previous review (api-reference rows, openapi sentinel_comment, min_level round-trip, writeAllowed doc, sentinelWritebackItem naming, ratchet writeback:gated exemption, RepoModeFor map-order) are not re-checked here and should be re-verified by the docs-currency/style perspectives.

β€” hive: agent=adjudicator backend=copilot model=claude-fable-5.1 copilot=1.0.88

@hivecommons-hive hivecommons-hive Bot added stage/050-review-pending Review swarm requested; awaiting a verdict for the current head and removed stage/071-needs-author Human-authored PR: CI red, changes requested or conflicting; diagnosis posted, the author must act labels Oct 29, 2026
@github-actions github-actions Bot added the conflicts-with-open-pr Open PR has a merge conflict with another open PR label Oct 29, 2026
@clubanderson
clubanderson merged commit d39e933 into v5 Oct 29, 2026
81 of 83 checks passed
@kubestellar-prow
kubestellar-prow Bot deleted the fix/writeback-containment branch October 29, 2026 03:27
@github-actions

Copy link
Copy Markdown
Contributor

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

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

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

Labels

author/human A person pushed this PR themselves conflicts-with-open-pr Open PR has a merge conflict with another open PR dco All commits are DCO signed-off dco-signoff: yes Indicates the PR's author has signed the DCO. role/trusted Author is trusted for unattended merge (trusted-author role or registry/claim tier in require_tiers) size/XXL Denotes a PR that changes 1000+ lines, ignoring generated files. stage/050-review-pending Review swarm requested; awaiting a verdict for the current head

Projects

None yet

Development

Successfully merging this pull request may close these issues.

πŸ› Write-back containment: platform sweeps post to repos the operator never opted in

1 participant