Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion bin/gh-wrapper.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1710,7 +1710,10 @@ if [[ -n "$AGENT_NAME" ]]; then
esac
done
((n == 1)) || return 0
[[ "$val" == *"β€” hive:"* ]] && return 0
# Only a line that starts with `β€” hive:` is a signature, as the repos'
# agent audit reads it; a new body that merely mentions the prefix in its
# text (a PR about that audit) still gets the PR's line back.
[[ $'\n'"$val" == *$'\nβ€” hive:'* ]] && return 0
_extract_item
local view_args=(pr view)
[[ -n "$item_num" ]] && view_args+=("$item_num")
Expand Down
4 changes: 4 additions & 0 deletions bin/gh-wrapper.test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -1612,6 +1612,10 @@ ATTR_OWN_LINE=$'New description.\n\nβ€” hive: agent=quality backend=claude model
_attr_run 'test-bot[bot]' "$ATTR_OLD_BODY" /dev/null -- pr edit "$ATTR_PR" --repo test/repo --body "$ATTR_OWN_LINE"
_expect_edit_body "own PR, new body carries its own line: left as written" "$ATTR_OWN_LINE"

ATTR_PROSE='The audit wants a `β€” hive:` signature line on every PR.'
_attr_run 'test-bot[bot]' "$ATTR_OLD_BODY" /dev/null -- pr edit "$ATTR_PR" --repo test/repo --body "$ATTR_PROSE"
_expect_edit_body "own PR, new body only mentions the prefix in its text: the line is re-appended" "${ATTR_PROSE}"$'\n\n'"$ATTR_TRAILER"

_attr_run 'someone-else' "$ATTR_OLD_BODY" /dev/null -- pr edit "$ATTR_PR" --repo test/repo --body 'New description: 16 failures.'
_expect_edit_body "a PR the hive did not open: body left as written" 'New description: 16 failures.'

Expand Down
1 change: 1 addition & 0 deletions changelog.d/fixed-hive-line-mentioned-in-text.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
- **A PR whose description mentions `β€” hive:` in its text is signed now.** The PR-request watcher skipped the `β€” hive: agent=… backend=… model=…` line whenever the body contained `β€” hive:` anywhere, so a PR that described the repositories' monthly agent audit (Danathar/arch-bootc#473, Danathar/atomic-image-builder#640 and #678) was opened unsigned, and that audit, which reads only a line beginning with `β€” hive:`, then failed. The watcher, `ReconcilePRAttribution`, the issue and review relays that use the same trailer, and the `gh` wrapper's body-edit guard now treat only a line that starts with `β€” hive:` as a signature. See [hive-open-pr](src/docs/hive-open-pr.md).
16 changes: 9 additions & 7 deletions src/docs/hive-open-pr.md
Original file line number Diff line number Diff line change
Expand Up @@ -174,16 +174,18 @@ possible fields are `agent`, `backend`, `model`, `effort`, `<tool>=<version>`,
`session`, and `requested_by`. The trailer is gated by
`governor.attribution_trailer` (default ON), but the matching audit entry
(`agent_pr_created` in `audit.jsonl`, same key=value pairs β€” see
[audit-log.md](audit-log.md)) is written regardless of the toggle. The `β€” hive:`
prefix doubles as the stacking guard: a request the watcher retries after a
partial failure will not gain a second trailer. Do not write a `β€” hive:` line
into your own body β€” the watcher will treat it as an existing trailer and skip
its own.
[audit-log.md](audit-log.md)) is written regardless of the toggle. A line that
starts with `β€” hive:` doubles as the stacking guard: a request the watcher
retries after a partial failure will not gain a second trailer. Do not start a
line of your own body with `β€” hive:` β€” the watcher will treat it as an existing
trailer and skip its own. Mentioning the prefix inside a sentence is fine: only
a line that begins with it counts, which is also how the repositories' monthly
agent audit reads it.

A later `gh pr edit <n> --body …` (or `--body-file`, or `-F -`) on a PR the hive
opened keeps the trailer. gh replaces the whole body, so the `gh` wrapper
re-appends the PR's current `β€” hive:` line to the new text when that text has
none of its own. A PR another account opened is edited exactly as written, and
re-appends the PR's current `β€” hive:` line to the new text when no line of
that text starts with `β€” hive:`. A PR another account opened is edited exactly as written, and
a PR whose body has no trailer gets none. An edit made any other way, such as
`gh api` against the pulls endpoint, bypasses the wrapper and can still drop
the line.
Expand Down
25 changes: 22 additions & 3 deletions src/pkg/github/attribution.go
Original file line number Diff line number Diff line change
Expand Up @@ -358,11 +358,13 @@ func ParseRunTrailers(body string) (runKey, planRef string) {
}

// AppendTrailer appends the visible trailer to body, blank-line separated.
// No-op when the trailer is empty or the body already carries one β€” a request
// the watcher retries after a partial failure must not stack trailers.
// No-op when the trailer is empty or the body already has a trailer line
// (HasAttributionLine) β€” a request the watcher retries after a partial
// failure must not stack trailers. A body that only mentions `β€” hive:` in its
// text still gets the line.
func AppendTrailer(body string, m InvocationMeta) string {
t := m.Trailer()
if t == "" || strings.Contains(body, AttributionTrailerPrefix) {
if t == "" || HasAttributionLine(body) {
return body
}
body = strings.TrimRight(body, "\n")
Expand All @@ -372,6 +374,23 @@ func AppendTrailer(body string, m InvocationMeta) string {
return body + "\n\n" + t
}

// HasAttributionLine reports whether body has a line that starts with the
// `β€” hive:` prefix: the signature line itself, read the way the repos' monthly
// agent audit reads it (a line whose first characters are `β€” hive:`). A body
// that only mentions `β€” hive:` in its text has none. Danathar/arch-bootc#473
// and Danathar/atomic-image-builder#640 and #678 described that audit, so
// their text named the prefix; AppendTrailer, which tested for the prefix
// anywhere, took that for a signature and opened them unsigned, and the audit
// then failed on them.
func HasAttributionLine(body string) bool {
for _, line := range strings.Split(body, "\n") {
if strings.HasPrefix(strings.TrimSuffix(line, "\r"), AttributionTrailerPrefix) {
return true
}
}
return false
}

// HasAttributionTrailer reports whether body carries the visible `β€” hive:`
// trailer, i.e. whether the artifact was hive-mediated: opened through the
// PR-request watcher, stamped by ReconcilePRAttribution, or opened by a
Expand Down
28 changes: 28 additions & 0 deletions src/pkg/github/attribution_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,34 @@ func TestAppendTrailer(t *testing.T) {
if got := AppendTrailer("body", InvocationMeta{}); got != "body" {
t.Errorf("empty meta must not touch the body, got %q", got)
}
// A body that only mentions the prefix in its text is not signed
// (Danathar/arch-bootc#473 described the audit that reads the line).
prose := "The audit checks for a `β€” hive:` signature line.\n"
if got, want := AppendTrailer(prose, m), strings.TrimRight(prose, "\n")+"\n\n"+m.Trailer(); got != want {
t.Errorf("prose mention: AppendTrailer = %q, want %q", got, want)
}
}

// HasAttributionLine is the repos' agent-audit reading: a line whose first
// characters are `β€” hive:`, not the prefix anywhere in the text.
func TestHasAttributionLine(t *testing.T) {
for _, tc := range []struct {
body string
want bool
}{
{"Fixes #1\n\nβ€” hive: agent=quality backend=copilot", true},
{"β€” hive: agent=scanner", true},
{"Fixes #1\r\n\r\nβ€” hive: agent=quality\r\n", true},
{"Fixes #1\n\nβ€” hive: agent=quality\n\n---\n🐝 **Hive Agent**: `quality` | **SHA:** `abc`", true},
{"The audit checks for a `β€” hive:` signature line.", false},
{"Quoted:\n> β€” hive: agent=quality", false},
{"Indented:\n β€” hive: agent=quality", false},
{"", false},
} {
if got := HasAttributionLine(tc.body); got != tc.want {
t.Errorf("HasAttributionLine(%q) = %v, want %v", tc.body, got, tc.want)
}
}
}

func TestAttributionAuditDetail(t *testing.T) {
Expand Down
Loading