Skip to content

πŸ› [v6] attribution: a description that mentions "β€” hive:" still gets signed - #17368

Merged
hivecommons-hive[bot] merged 1 commit into
hivecommons:v6from
Danathar:fix/v6-attribution-line-not-substring
Nov 9, 2026
Merged

hivecommons-hive[bot] merged 1 commit into
hivecommons:v6from
Danathar:fix/v6-attribution-line-not-substring

Conversation

@Danathar

@Danathar Danathar commented Nov 9, 2026

Copy link
Copy Markdown
Contributor

Summary

Some PRs the Hive App opens never get their β€” hive: signature line, and the repos' monthly agent audit then fails. This happens when the PR's own description mentions β€” hive: in a sentence. The watcher saw the prefix and decided the body was already signed. Now only a line that starts with β€” hive: counts as a signature, which is how the audit reads it.

What happened

I checked every PR the audits flagged in Danathar/arch-bootc and Danathar/atomic-image-builder against GitHub's edit history (userContentEdits). They fall into two groups:

PRs First version signed? Later edits Cause Status
arch-bootc #576, #587, #625; aib #795 yes 2–3 The agent's gh pr edit --body replaced the body and dropped the line Fixed by #16256 (2026-11-02)
arch-bootc #473; aib #640, #678 no none The description quotes β€” hive: (they are PRs about the audit), so AppendTrailer skipped This PR

Since #16256 merged, 18 Hive-app PRs were opened across the two repos and none is missing the line.

Root cause

  • AppendTrailer skipped when strings.Contains(body, "β€” hive:") was true (src/pkg/github/attribution.go:365). The PR-request watcher calls it at pr_request_watcher.go:523.
  • The wrapper's body-edit guard from πŸ› gh-wrapper: a body edit of the hive's own PR keeps its β€” hive: lineΒ #16256 had the same test: [[ "$val" == *"β€” hive:"* ]] in _keep_pr_attribution.
  • The audit (agent-audit.yml:120 in those repos) only accepts a line that starts with β€” hive:: split("\n") | map(select(startswith("β€” hive:"))). arch-bootc#473's description holds the prefix only inside a sentence, so the audit flagged it correctly; nothing edited the body.

The fix

  • New HasAttributionLine(body) in attribution.go: true when some line starts with β€” hive:, the audit's rule. A quoted (> β€” hive:) or indented line does not count, again matching the audit.
  • AppendTrailer uses it. That covers the PR-request watcher, ReconcilePRAttribution, and the issue and review relays that call the same function.
  • bin/gh-wrapper.sh _keep_pr_attribution checks for a line that starts with β€” hive: in the new body. It uses a bash pattern rather than a pipe, because the wrapper runs under pipefail.
  • HasAttributionTrailer, the separate "was this hive-mediated" test used by review threads and the clanker check, is unchanged.

Not done here (from the issue's acceptance criteria)

  • A sweep that repairs unsigned Hive-app PRs. With both causes fixed, an unsigned PR now needs an edit made outside the wrapper: the GitHub MCP, gh api, or the real gh binary. The proxy still allows PATCH /pulls/{n} at ISSUES_AND_PRS. A pre-merge "ensure the line" check in the automerge path would close that for merged PRs, which are what the audit reads. Left for a follow-up if it shows up.
  • The triage that counted owner and Renovate PRs as unsigned (arch-bootc#658). That was an agent's own comment, not a code path.

Files

  • src/pkg/github/attribution.go: HasAttributionLine; AppendTrailer uses it.
  • bin/gh-wrapper.sh: _keep_pr_attribution uses a line test.
  • src/docs/hive-open-pr.md: the stacking guard is a line, and mentioning the prefix in a sentence is fine.
  • changelog.d/fixed-hive-line-mentioned-in-text.md.

Tests

CI is the verdict. Locally, only gofmt -l and bash -n were run.

  • TestAppendTrailer: a body that mentions β€” hive: in a sentence gets the line appended.
  • TestHasAttributionLine: a trailer line at the end, the only line, with CRLF, or before the identity footer counts; a sentence, a quote, an indented line or an empty body does not.
  • bin/gh-wrapper.test.sh: an own-PR edit whose new body mentions the prefix in a sentence gets the PR's line re-appended. The existing πŸ› gh-wrapper: a body edit of the hive's own PR keeps its β€” hive: lineΒ #16256 cases are unchanged, including "new body carries its own line: left as written".

πŸ€– Generated with Claude Code

https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1

AppendTrailer skipped the trailer when the body contained "β€” hive:"
anywhere, so a PR whose description talked about the repos' agent audit
(Danathar/arch-bootc#473, Danathar/atomic-image-builder#640 and hivecommons#678,
none ever edited) was opened unsigned, and the audit, which reads a line
beginning with "β€” hive:", failed on it. The gh wrapper's body-edit guard
(hivecommons#16256) had the same substring test.

HasAttributionLine reads the line the way the audit does. AppendTrailer
(so the PR-request watcher, ReconcilePRAttribution and the issue and
review relays) and the wrapper's _keep_pr_attribution use it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BQYnPGFJx81c8JXR3o1sg1
Signed-off-by: Douglas Baggett <doug.baggett@gmail.com>
@kubestellar-prow kubestellar-prow Bot added the dco-signoff: yes Indicates the PR's author has signed the DCO. label Nov 9, 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 nicholasjackson 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/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Nov 9, 2026
@hivecommons-hive

Copy link
Copy Markdown
Contributor
ed279fb

ℹ️ Sentinel notice β€” informational; author is trusted (owner), merge is not blocked

Hive flagged this PR (author @Danathar, head ed279fb04918) because it matches behaviors that can override security controls, escalate privileges or damage the codebase. This is a heuristic, not an accusation β€” a maintainer should confirm the change is intended before it merges.

  • sensitive_path β€” changes 2 sensitive path(s) (Touches a sensitive path (OWNERS, workflows, policies, hive config, security docs, …))
    • bin/gh-wrapper.sh
    • bin/gh-wrapper.test.sh

Hive did not add the sentinel-alert label because this author is trusted. The finding remains recorded for audit and the dashboard Security tab; set sentinel.trusted_authors_block: true to require the blocking label for trusted authors too.

@hivecommons-hive

Copy link
Copy Markdown
Contributor

Review follow-ups (below review.backlog.min_severity: P2)

Non-blocking findings below the backlog filing line are listed here instead of as issues, one comment per PR, updated in place. Tick them off as they are fixed; set review.backlog.min_severity: P3 to file every finding.

  • P3 info security β€” HasAttributionTrailer still treats a prose mention of β€” hive: as hive-mediated (src/pkg/github/attribution.go:403): HasAttributionTrailer remains strings.Contains(body, AttributionTrailerPrefix), so a human-authored PR or issue whose text merely quotes β€” hive: is classified as hive-mediated by isHiveFiledIssue…

@hivecommons-hive
hivecommons-hive Bot merged commit 359200b into hivecommons:v6 Nov 9, 2026
70 of 75 checks passed
@hivecommons-hive

hivecommons-hive Bot commented Nov 9, 2026 •

Copy link
Copy Markdown
Contributor

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

Evidence bundle `#17368ed279fb` sha256:230229f0294c7df126a75d90e95b6ccdd082579f5777791e0f2a8ec6baf6b5d4 β€” download from the dashboard or `hivectl review evidence #17368`.

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

Read the diff against the v6 tree and the callers. This looks correct to me.

What I verified

  • The new line-based test in HasAttributionLine (src/pkg/github/attribution.go) agrees with how the trailer is already read: ParseAttributionTrailer (attribution.go:430-437 on v6) only accepts a β€” hive: line via HasPrefix on the final non-blank line, and prfollowup/handoff.go:169 does the same. The old strings.Contains in AppendTrailer was the odd one out, so a prose mention could suppress the signature the parser then failed to find β€” exactly the arch-bootc#473 shape.
  • All six AppendTrailer callers (pr_request_watcher.go:523, review_request_watcher.go:549/790, issue_request_watcher.go:634, advisory.go:257, ReconcilePRAttribution at attribution.go:732, plus dashboard api_acmm_eval.go:300 / nous_verdict.go:451) pick up the change with no other substring check in between.
  • Wrapper guard [[ $'\n'"$val" == *$'\nβ€” hive:'* ]]: * in [[ == ]] matches newlines, so this is a correct "some line starts with" test, and a CRLF body still contains \nβ€” hive:. The old-body extraction a few lines below (sed 's/^[[:space:]]*\(β€” hive: …/') already tolerates leading whitespace, so re-appending still finds the PR's existing line.

One deliberate asymmetry worth knowing (not a defect of this PR)

HasAttributionTrailer (attribution.go:384) and authorkind.go:229 still use strings.Contains. The PR body says this is intentional, and I agree it is out of scope β€” but it means a human-written PR that mentions β€” hive: in prose is still classified as hive-mediated by pr_request_claims.go:408 and task_list_sweep.go:1098 (the pre-close "is this a hive-filed issue" check). Pre-existing; flagging only so the follow-up is visible.

Minor: HasAttributionLine and the wrapper's new-body guard reject an indented β€” hive: line, while ParseAttributionTrailer (TrimSpace) accepts one. An agent body with an indented trailer would gain a second, unindented one. The parser reads the last line, so the outcome is still correct; mentioning for completeness, no change requested.

@hivecommons-hive

Copy link
Copy Markdown
Contributor

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

@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.

Read the change against the v6 tree at ed279fb, including the callers of AppendTrailer and the wrapper's _keep_pr_attribution. It looks correct to me and does what the description says.

What I verified:

  • src/pkg/github/attribution.go:365-391 β€” AppendTrailer now gates on HasAttributionLine, which only matches a line that starts with β€” hive: (CRLF tolerated). All six AppendTrailer call sites (pr_request_watcher.go:523, review_request_watcher.go:549,790, issue_request_watcher.go:634, advisory.go:257, dashboard api_acmm_eval.go:300 / nous_verdict.go:451) inherit the fix, and ReconcilePRAttribution (attribution.go:751) decides "nothing to do" from AppendTrailer's return value, so the reconciler also stamps a prose-mention body now rather than skipping it.
  • HasAttributionTrailer (attribution.go:403) is deliberately left as strings.Contains; it feeds the "is this hive-mediated" checks in pr_comments.go, pr_request_claims.go:408 and task_list_sweep.go:1098, none of which are touched. Good call keeping the two semantics apart.
  • bin/gh-wrapper.sh:1716 β€” [[ $'\n'"$val" == *$'\nβ€” hive:'* ]] handles a trailer on the first line (the prepended \n) and CRLF bodies (\r\nβ€” hive: still contains \nβ€” hive:). The old-trailer extraction a few lines below (^[[:space:]]*β€” hive:) is slightly more permissive than the Go side (it accepts an indented line), but it only reads the existing trailer back, so there is no behavioural gap.
  • Tests cover the exact regression (prose mention β†’ line appended) on both the Go and wrapper sides.

One nit, not blocking: the ReconcilePRAttribution doc comment at attribution.go:720 still says "if the body already contains the trailer prefix"; it is now "has a line starting with the prefix".

Open point for the maintainer, not a defect: the body refers to "the issue's acceptance criteria" but the PR has no Closes/Refs line, so nothing will close on merge.

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

Labels

dco-signoff: yes Indicates the PR's author has signed the DCO. size/M Denotes a PR that changes 30-99 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant