Skip to content

🐛 fix(github): recognise the hive's own unpark replies when it posts as an unconfigured login - #17512

Draft
hanthor wants to merge 1 commit into
v5from
fix/unpark-reply-own-login
Draft

hanthor wants to merge 1 commit into
v5from
fix/unpark-reply-own-login

Conversation

@hanthor

@hanthor hanthor commented Nov 10, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Bug: the un-park sweep posts the same reply to one /hive command on every sweep when the hive posts as a GitHub login its configured identity doesn't name.
  • Cause:
  • Fix:
    • New learnSelfLogin records the author GitHub returns when the client creates an un-park reply, a notice or a label-approval comment. That author is the account the token posts as.
    • isHiveAccount already accepts cachedSelfLogin(), so the hive's own markers count from the next sweep on, for every marker reader that goes through markerAuthorTrusted.
    • A marker pasted by any other account still does not count.

Evidence from a live hive (tuna-os)

The hive's own replies all carry the same marker, id=6582313529, which points at the one maintainer comment it keeps answering:

Issue Bot replies Per day since 11-02
tuna-os/Tavern#299 320 15-58
tuna-os/corral#607 325 16-58
tuna-os/corral#634 323 15-58
tuna-os/bluefin-cli#295 323 15-58

On v6 the repeated reply is the project.ai_author refusal (renderUnparkAIAuthorRefusal). The trust check that loops is the same on v5, where /hive help shows it (see test).

Related issues

Refs #11313

Testing

  • New regression test TestSweepIssueUnparkCommandsAnswersOnceWhenPostingAsUnconfiguredLogin:
    • The fake server reports a comment author (postAuthor) different from the configured AppLogin; the test runs three sweeps.
    • Fails on v5: command 11 answered 3 times over 3 sweeps, want once. Passes with the fix.
  • go test ./pkg/github/... -count=1 passes (pkg/github, automerge, requestwatch). The pasted-marker invariants in label_approval_test.go (A13) still pass.
  • go build ./..., go vet ./pkg/github/ and gofmt are clean. check-changelog-fragment.sh reports OK.

Limits

  • One extra reply per restart: the learned login lives in memory, so after a restart the first reply to an already-answered command can be posted once more before the hive learns its login again. Reading the slug from GET /app at boot would close that gap; I kept this PR minimal.
  • Stricter rule after the first post: a hive with no identity configured whose GET /user fails used to trust markers from any author. Once it has posted, it now trusts only its own markers, which is the [RFC] [v5] Prow compatible mode: let an organisation's acceptance label (triage/accepted) approve work for Hive #11313 intent.
  • No v6 changes: the fix should reach v6 through v6-topup, since v6 carries the same postUnparkReply and trust check.

Contributor checklist

  • Changelog: changelog.d/fixed-unpark-reply-own-login.md
  • Targets v5
  • Title uses the emoji convention
  • DCO sign-off
  • No secrets or runtime state

…as an unconfigured login

The un-park sweep only believes a hive:unpark-reply / hive:label-approval
marker when the comment's author is one of the hive's own accounts
(markerAuthorTrusted, #11313). An App installed under its
own slug with no github.app_slug set falls back to the public default
slug, so the hive's real <slug>[bot] login was never among those accounts:
every sweep saw the command as unanswered and replied again.

A live tuna-os hive posted 320+ identical replies on each of four issues
(40-58 a day) this way.

Record the author GitHub returns when the client creates an un-park reply,
notice or label-approval comment. That author is by definition the
account the token posts as, so isHiveAccount now matches it through the
existing cachedSelfLogin check. A marker pasted by any other account still
does not count.

Signed-off-by: James Reilly <jreilly1821@gmail.com>
@kubestellar-prow kubestellar-prow Bot added do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. dco-signoff: yes Indicates the PR's author has signed the DCO. labels Nov 10, 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 hanthor 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 10, 2026
@github-actions

github-actions Bot commented Nov 10, 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.

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

@github-actions

Copy link
Copy Markdown
Contributor

Mutation testing (advisory)

No surviving mutants in changed files (or the run hit its time budget before reporting any).

Advisory only; this job never blocks the PR (#17349).

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. do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. 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