sticky.Post: never edit a marker comment when the poster's identity cannot be resolved

[waynesun09]

Current behaviour

sticky.Post (internal/sticky/sticky.go) resolves the poster's login with forge.Client.GetAuthenticatedUser and then filters marker comments by author. If that lookup fails, it logs "Could not determine bot user, marker spoofing protection degraded" and continues with an empty login. FindMarkedComment then matches on the marker alone. The result is that the first comment carrying the marker gets edited, whoever wrote it.

Why this needs a follow-up

#7243 changes fullsend issues post-comment (postTrackerStickyComment) so that it never edits a comment it cannot verify as its own:

• An existing comment matches only when both the author (exact login) and the marker match.
• If the identity cannot be resolved after a short retry, it fails with an error and posts or edits nothing. Posting a new comment instead would orphan the earlier one.

sticky.Post (used by post-comment / post-review) is now looser than that path. When identity resolution fails, it falls back to editing an unverified comment.

Ask

Bring sticky.Post to the same rule:

• When the login cannot be resolved (after a short retry), do not edit any existing comment. Return an error naming the cause instead of falling back.
• Make FindMarkedComment with an empty login match nothing.
• Add tests for an unresolvable identity and a planted marker comment.

Out of scope for #7243.

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 12:54 AM UTC · Completed 12:58 AM UTC

Commit: 114478f · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $0.56

[fullsend-ai-triage]

Re-verified this directly against internal/sticky/sticky.go: Post() only logs when GetAuthenticatedUser fails and continues with an empty botUser, and FindMarkedComment's author filter is skipped entirely when botUser is empty (if botUser != "" { ... }), so it matches the first marker-bearing comment regardless of author. The gap as described is confirmed.

Correcting the prior triage note on this issue: it stated PR #7243's change "has nothing to do with comment-author matching" and that the reference to #7243 "doesn't hold up." Re-reading #7243's full PR description (still open) shows the opposite -- it explicitly adds exact-login + marker matching to the tracker-sticky path via a new --only-if-exists flag, and its own description states: "sticky.Post still falls back to marker-only matching when identity lookup fails; that is #7884." So this issue's framing is accurate and #7243's author scoped this exact follow-up here. This doesn't block the fix, though -- internal/sticky/sticky.go is an independent package from the tracker-sticky code #7243 touches, so this can proceed without waiting on #7243 to merge.

Also confirmed: TestFindMarkedComment_EmptyBotUser in internal/sticky/sticky_test.go currently asserts the opposite of the desired behaviour ("empty botUser should match any author") and will need to be inverted, not just supplemented, as part of this fix.

No open PR currently targets internal/sticky/sticky.go or FindMarkedComment.

Proposed test coverage:

Given: a comments list where one entry carries the marker, authored by an
  arbitrary non-bot user
When:  FindMarkedComment(comments, marker, "") is called
Then:  returns nil instead of matching the marked comment

Given: sticky.Post is called and GetAuthenticatedUser returns an error,
  while ListIssueComments already contains a marker comment
When:  Post runs
Then:  it returns an error naming the identity-resolution failure and calls
  neither CreateIssueComment nor UpdateIssueComment

Edge cases:
  - A genuine bot comment and a separately planted spoofed comment both
    present, resolution still fails -> must not edit either
  - DryRun with unresolved identity -> messaging must not imply an edit
    would occur

---

Next steps:

• /fs-code — agent creates a PR to implement this issue
• /fs-code <your instruction> — agent implements with your specific guidance

Previous run

Verified directly in internal/sticky/sticky.go: FindMarkedComment's author filter is only applied if botUser != "", so when identity resolution fails and botUser is empty, the filter is skipped entirely and the function matches the first comment carrying the marker regardless of author — confirming the gap as described.

One correction worth flagging: the reference to #7243 as the PR that already added exact-login + marker matching to postTrackerStickyComment doesn't hold up. #7243 is still open, and its actual change (flattening a rejected status string before it's logged, plus a readonly_repo flag for the review role) has nothing to do with comment-author matching. Reading postTrackerStickyComment/findMarkedTrackerComment in internal/cli/issues.go directly shows it still matches on the marker alone today, by design (its doc comment notes tracker.Client has no GetAuthenticatedUser). This doesn't change the validity of this issue — the sticky.Post gap is real and independently verifiable — but the stated rationale/precedent should be treated as unconfirmed.

Also worth noting for implementation: TestFindMarkedComment_EmptyBotUser currently asserts the opposite of the desired behavior ("empty botUser should match any author"). That test needs to be inverted, not just supplemented with new cases.

Proposed test coverage:

Given: a comments list where one entry carries the marker, authored by an
  arbitrary non-bot user
When:  FindMarkedComment(comments, marker, "") is called
Then:  returns nil (no match) instead of matching the marked comment

Given: sticky.Post is called and GetAuthenticatedUser returns an error,
  while ListIssueComments already contains a marker comment
When:  Post runs
Then:  it calls CreateIssueComment, never UpdateIssueComment, and logs that
  identity could not be resolved

Edge cases:
  - Both a genuine bot comment and a spoofed comment present, resolution
    still fails -> must not edit either
  - DryRun with unresolved identity -> message should say 'would create',
    not 'would update'

---

Labels: Matches this repo's convention for security-tagged defects (security + type/bug + bug + priority), based on recent issues like #7578 and #7725.

---

Next steps:

• /fs-code — agent creates a PR to implement this issue
• /fs-code <your instruction> — agent implements with your specific guidance

[fullsend-ai-triage]

🤖 Finished Triage · ✅ Success · Started 1:10 AM UTC · Completed 1:13 AM UTC

Commit: 114478f · View workflow run →

Runtime: claude · Model: sonnet → claude-sonnet-5 · Effort: high · Cost: $0.43