feat(#3697): add emoji reaction status notifications

[ralphbean]

Summary

Stacked on #5736 — this adds the "react to the issue with an emoji" alternative from #3697, as a supplement/alternative to status comments.

Blocked on: #5994 — perRepoConfig has no status_notifications field, and the live pkg/behaviourtest e2e suite only supports per-repo installs, so there is currently no test harness that can enable reactions to exercise the maintainer-mandated behavior test below.

• New AddIssueReaction/DeleteIssueReaction on forge.Client, implemented for GitHub (GitLab returns ErrNotSupported — no caller exercises that path yet)
• New AddIssueCommentReaction/DeleteIssueCommentReaction on forge.Client so reactions can target the triggering comment for slash-command-invoked runs, per Triage agent causes unnecessary notifications - should skip initial comment #3697's explicit requirement
• New status_notifications.reaction config block (start/completion, same enabled/on_failure/disabled values as comment), defaulting to disabled since it's an opt-in addition
• Notifier.PostStart adds a 👀 reaction when reaction.start: enabled; PostCompletionWithDetail swaps it for 👍 (success) or 😕 (failure/cancelled/skipped/unrecognized) depending on reaction.completion
• Reactions target the triggering comment (via --status-comment-id, wired through action.yml and all reusable workflow call sites) when the run was invoked by a slash command, and the issue/PR otherwise
• Reactions generate no GitHub notification, so unlike comments they don't need the on_failure start-suppression workaround
• Docs updated under "Status Notifications" in docs/guides/user/customizing-agents.md

Out of scope:

• ReconcileOrphaned does not yet reconcile orphaned reaction state on hard-killed runs — plumbing a reaction ID across process boundaries would need more design than is justified here.
• Reactions have no per-run identity (GitHub's reactions API is keyed by actor+subject+content), so concurrent same-role runs can collide on the same reaction. Documented as a known limitation with inline code comments; not fixable without GitHub API support.
• GitLab silently no-ops on reaction calls (ErrNotSupported) with no config-time validation warning users their reaction settings do nothing. Documented as a known limitation; follow-up needed (also applies to the new JIRA poll input driver).
• Live e2e behavior test for "slash command targets the comment, not the issue" — blocked on perRepoConfig has no status_notifications field — reactions/comments toggles are org-only #5994. Covered today by unit tests only (TestPostStart_ReactionTargetsTriggeringComment, TestPostCompletion_ReactionTargetsTriggeringComment in internal/statuscomment/statuscomment_test.go).

Test plan

• go build ./...
• go vet ./...
• gofmt -l clean
• go test ./... — all green except pre-existing, unrelated internal/runtime failures (verified present on base branch too)
• New unit tests for config parsing/validation, FakeClient, GitHub REST calls, and Notifier reaction lifecycle (start/completion/on_failure/cleanup, comment-scoped targeting)
• Vendor a binary built off this branch and try it against a real test repo
• Live e2e behavior test for comment-scoped reactions — blocked on perRepoConfig has no status_notifications field — reactions/comments toggles are org-only #5994

Assisted-by: Claude Opus 4.6 noreply@anthropic.com

[fullsend-ai-review]

🤖 Finished Review · ✅ Success · Started 9:34 PM UTC · Completed 9:53 PM UTC
Commit: 390f99e · View workflow run →

[qodo-code-review]

PR Summary by Qodo

Add opt-in emoji reactions for agent status notifications

✨ Enhancement 🧪 Tests 📝 Documentation 🕐 40+ Minutes

AI Description

• Add opt-in status notification reactions (👀 start, 👍/👎 completion) alongside comments.
• Extend forge.Client with AddIssueReaction/DeleteIssueReaction (GitHub implemented; GitLab returns
 ErrNotSupported).
• Add config parsing/validation, unit tests, and user docs for reaction notifications.

Diagram

graph TD
  A["Org config YAML"] --> B["internal/config"] --> C["internal/statuscomment Notifier"] --> D["forge.Client"]
  D --> E{{"GitHub REST API"}}
  D --> F["internal/forge/gitlab"]
  B --> G["Unit tests"]
  C --> G
  D --> G
  B --> H["Docs: customizing agents"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Use GitHub Checks/Commit Status instead of issue reactions

• ➕ More standard CI UX (green/red checks) and visible in PR header
• ➕ No need to manage reaction IDs or cleanup lifecycle
• ➖ Not an issue/PR-thread signal; less useful for issue-driven runs
• ➖ May require additional permissions/scopes and different plumbing than existing status notifications

2. Use labels for low-noise status signaling

• ➕ Works across forges more uniformly (GitHub/GitLab)
• ➕ No reaction ID tracking; easy to reconcile orphaned state
• ➖ Label churn can be noisy in audit/history and may require label management rights
• ➖ Harder to represent multiple concurrent runs or nuanced outcomes without many labels

3. Implement GitLab “award emoji” support now

• ➕ Feature parity across GitHub and GitLab
• ➕ Avoids adding an interface method that’s a no-op on one forge
• ➖ More API surface/risk without current callers exercising it
• ➖ Additional test/mocking burden and behavior differences to reconcile

Recommendation: The PR’s approach (opt-in reactions, fail-open on reaction errors, and GitLab returning ErrNotSupported) is a good incremental extension to existing status comments: it adds low-noise signaling without changing default behavior. Consider a follow-up to either (a) implement GitLab award emoji once there’s a concrete caller, or (b) add optional reconciliation if reaction IDs are persisted in the future.

Files changed (11) +595 / -29

Enhancement (5) +198 / -29

config.goAdd reaction notification config and shared validation helper +37/-9

Add reaction notification config and shared validation helper

• Extends StatusNotificationConfig with a Reaction block and introduces ReactionNotificationConfig. Refactors status notification validation into a shared helper with explicit allowed values for start/completion across comment and reaction settings.

internal/config/config.go

forge.goExtend forge.Client interface with issue reaction methods +14/-0

Extend forge.Client interface with issue reaction methods

• Adds AddIssueReaction and DeleteIssueReaction to the forge abstraction, documenting valid GitHub content values and the expected ErrNotSupported behavior on unsupported forges.

internal/forge/forge.go

github.goImplement GitHub issue reaction add/delete via REST +28/-0

Implement GitHub issue reaction add/delete via REST

• Implements AddIssueReaction (with content validation) and DeleteIssueReaction using GitHub’s /issues/:number/reactions endpoints, returning the reaction ID for later removal.

internal/forge/github/github.go

issue.goStub GitLab reaction APIs as unsupported +12/-0

Stub GitLab reaction APIs as unsupported

• Adds AddIssueReaction/DeleteIssueReaction methods that return forge.ErrNotSupported, with commentary pointing to GitLab’s award emoji concept but intentionally leaving it unimplemented.

internal/forge/gitlab/issue.go

statuscomment.goAdd reaction lifecycle to Notifier start/completion flow +107/-20

Add reaction lifecycle to Notifier start/completion flow

• Introduces opt-in reaction behavior: 👀 on start and 👍/👎 on completion based on status and completion mode. Adds shared failure classification, separate reaction enablement defaults (disabled), and fail-open behavior for reaction/token errors while preserving existing comment behavior and cleanup.

internal/statuscomment/statuscomment.go

Tests (4) +335 / -0

config_test.goAdd parsing/validation/marshal tests for reaction notifications +125/-0

Add parsing/validation/marshal tests for reaction notifications

• Introduces unit tests covering YAML parsing, valid/invalid reaction start/completion values, on_failure semantics, and marshaling output for the new reaction configuration block.

internal/config/config_test.go

fake_test.goAdd FakeClient tests for reaction APIs +32/-0

Add FakeClient tests for reaction APIs

• Adds tests verifying reaction IDs increment, calls are recorded correctly, delete tracking works, and error injection for AddIssueReaction is honored.

internal/forge/fake_test.go

github_comment_test.goAdd HTTP tests for GitHub reaction endpoints +44/-0

Add HTTP tests for GitHub reaction endpoints

• Adds httptest-based coverage ensuring the GitHub client hits the correct POST/DELETE endpoints, sends the expected JSON body, rejects invalid content locally, and parses returned reaction IDs.

internal/forge/github/github_comment_test.go

statuscomment_test.goAdd Notifier tests for reaction start/completion and cleanup +134/-0

Add Notifier tests for reaction start/completion and cleanup

• Adds unit tests verifying reactions are opt-in, start reaction posting, completion swapping/cleanup behavior, on_failure semantics, handling when no start reaction exists, and ensuring reaction failures are non-fatal.

internal/statuscomment/statuscomment_test.go

Documentation (1) +26 / -0

customizing-agents.mdDocument reaction-based status notifications +26/-0

Document reaction-based status notifications

• Adds a new “Reactions” section describing status_notifications.reaction semantics, defaults (disabled), and outcome mapping (👀/👍/👎). Notes reactions are GitHub-only and clarifies on_failure behavior without notification concerns.

docs/guides/user/customizing-agents.md

Other (1) +36 / -0

fake.goTeach FakeClient to record add/delete issue reactions +36/-0

Teach FakeClient to record add/delete issue reactions

• Adds ReactionRecord plus FakeClient state and counters to track AddIssueReaction calls and DeleteIssueReaction IDs. Implements the new forge.Client methods with error injection support.

internal/forge/fake.go

[github-actions]

Site preview

Preview: https://8a88d494-site.fullsend-ai.workers.dev

Commit: 390f99e6eec34733730000c912d8eb1b438fcbf3

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Reaction swap before comment ✓ Resolved 🐞 Bug ☼ Reliability

Description

Notifier.PostCompletionWithDetail updates/removes reactions before attempting to create/update the
completion comment, so a later comment API failure can leave reactions indicating completion while
the status comment remains in the "Started" state (or removes the start reaction without
successfully posting completion). This introduces inconsistent user-visible status signaling on
transient GitHub API failures.

Code

internal/statuscomment/statuscomment.go[R257-259]

+	n.postCompletionReaction(ctx, status, cleanupReaction, postReaction)
+
+	if !postComment {

Relevance

●●● Strong

Team often hardens statuscomment against unexpected API states; reordering avoids misleading
user-visible completion signaling.

PR-#1871

ⓘ Recommendations generated based on similar findings in past PRs

Evidence

The code invokes reaction cleanup/posting before any completion comment body is built or API calls
are made, but later returns an error if comment update/post fails; reaction operations are
logged-only and not rolled back.

internal/statuscomment/statuscomment.go[239-295]
internal/statuscomment/statuscomment.go[298-316]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`Notifier.PostCompletionWithDetail` performs reaction cleanup/posting (`postCompletionReaction`) before attempting the completion comment update/post. If the comment call fails (and `PostCompletionWithDetail` returns an error), the issue/PR can show a completion (or no) reaction while the status comment still shows "Started", which is inconsistent.

### Issue Context
- `PostCompletionWithDetail` returns errors on `UpdateIssueComment` / `CreateIssueComment` failures.
- Reaction lifecycle operations are intentionally fail-open (logged, not returned), so once the swap happens there is no rollback.

### Fix Focus Areas
- internal/statuscomment/statuscomment.go[239-293]

Suggested approach:
- If `postComment` is true, attempt the comment update/post first; only after it succeeds should you delete the start reaction / add the completion reaction.
- If `postComment` is false (completion comment suppressed), keep current best-effort reaction cleanup/posting behavior.
- Consider clearing `n.startReactionID` after a successful deletion to avoid accidental double-deletes if `PostCompletionWithDetail` is called twice.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context used

✅ Compliance rules (platform): 54 rules

✅ Skills: writing-user-docs, writing-adrs

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}

[ralphbean]

It worked at appdumpster/test-repo#43

[fullsend-ai-review]

Review

Findings

Medium

• [incomplete-documentation] docs/guides/getting-started/operations.md:176 — The "Status notifications" section documents status_notifications.comment but omits the newly added status_notifications.reaction field. Readers consulting this guide will get an incomplete picture of the notification config surface.
  Remediation: Add documentation for the reaction sub-field including schema, default values (disabled), emoji mappings (👀 start, 👍/👎 completion), and the GitHub-only note.

Low

• [scope-documentation] The PR defers ReconcileOrphaned reaction cleanup to future work but no follow-up issue is filed to track the gap. An orphaned 👀 reaction will persist after SIGKILL/OOM.
  Remediation: File a follow-up issue to track orphaned reaction cleanup in ReconcileOrphaned.

• [error-handling] internal/statuscomment/statuscomment.go:253 — In PostCompletionWithDetail, when refreshClient fails and no comment is needed but reaction/comment cleanup is pending, both are silently skipped. The log message ("failed to mint token for completion") doesn't distinguish which operation was intended. This is by design (fail-open for reactions) and low impact.

• [incomplete-documentation] docs/guides/infrastructure/layered-config-reference.md — The per-field merge rules table does not document status_notifications (pre-existing gap, not introduced by this PR, but now more noticeable with the expanded config surface).
  Remediation: Add status_notifications to the merge rules table.

---

Labels: PR modifies agent runner notification lifecycle (status reactions in statuscomment package) and includes user-facing documentation updates.

[fullsend-ai-review]

See the review comment for full details.

[waynesun09]

HIGH: PR head has diverged from its declared base — diff includes stale/unrelated content and reports a real merge conflict

Re-verified live: this PR's mergeable/mergeStateStatus currently report CONFLICTING/DIRTY, and a compare of feat/3697-on-failure-comment-completion...feat/3697-emoji-reaction-notifications reports status: diverged, ahead_by: 16, behind_by: 17. The base branch (#5736) has moved 17 commits ahead of where this branch was cut, while this branch still carries its own older copy of the same on_failure/reconcile-status machinery. The rendered diff for this PR therefore mixes stale duplicate content (internal/cli/reconcilestatus.go, action.yml, docs/guides/dev/cli-internals.md) in with the actual emoji-reaction work, even though the PR summary frames the emoji-reaction work as the only change. This is a real, currently-active state and will guarantee conflicts or silently reintroduce stale content when merged as-is.

Suggestion: Rebase feat/3697-emoji-reaction-notifications onto the current tip of feat/3697-on-failure-comment-completion (or onto main once #5736 merges), then re-diff to confirm only the reaction-related changes remain before requesting re-review.

[waynesun09]

HIGH: Reactions always target the issue/PR, silently dropping the maintainer's explicit slash-command comment-targeting requirement from #3697

Maintainer ralphbean explicitly directed on #3697: "emoji reactions should be on the issue or pull request if it is the issue or pull request event that triggered the agent, but the emoji reactions should be on the comment that triggered the agent if it was invoked by a slash command. This definitely needs behavior tests and its own user-facing docs." The triage summary on #3697 also lists an explicit Gherkin scenario: "Slash command targets the comment, not the issue."

forge.Client.AddIssueReaction/DeleteIssueReaction here (and their only callers in internal/statuscomment/statuscomment.go:175-214, 306-317) only ever take/react to n.number (the issue/PR) — there is no comment-ID parameter or code path to react to a triggering comment, and docs/guides/user/customizing-agents.md:537 states reactions go "on the issue/PR itself" unconditionally. The PR description's "Out of scope" section only mentions the ReconcileOrphaned reaction-reconciliation gap, not this one, so a reviewer relying on the PR body would reasonably believe #3697's reaction requirement is fully covered when an explicit, maintainer-mandated sub-requirement (with its own required test scenario) is silently dropped.

Suggestion: Either implement comment-scoped reactions for slash-command-triggered runs (plumb the triggering comment ID into the Notifier/CLI) with the behavior test the maintainer asked for, or explicitly add this to the PR's "Out of scope" list so it isn't mistaken for complete #3697 coverage.

[ralphbean]

You're right, this was a real gap — thanks for catching it. Implemented comment-scoped reactions in ad6b587: a new AddIssueCommentReaction/DeleteIssueCommentReaction pair on forge.Client, a --status-comment-id flag plumbed through fullsend run and the composite action (efb45c1 wires it through all the reusable workflows), and Notifier now targets the trigger comment when it's set.

The Gherkin scenario itself is still open, though — turns out pkg/behaviourtest only supports per-repo installs and perRepoConfig has no status_notifications field at all, so there's no way to turn reactions on in the live e2e suite today. Filed #5994 for that and marked this PR blocked on it; for now the comment-targeting behavior only has unit coverage (TestPostStart_ReactionTargetsTriggeringComment etc.).

[ascerra]

We've discussed using thumbs up and thumbs down as a way for users to tell us if they liked or disliked the agent response so we can gather metrics on user approval ratings and things like that.

I see the thumbs up is added as the bot so this might not be a problem... our metric collector one day will just need to check for human added emojis

[ralphbean]

Marking this blocked on #5994. Turns out perRepoConfig has no status_notifications field at all, and pkg/behaviourtest only supports per-repo installs — so there's no way to turn reactions on in the live e2e suite today. That leaves the comment-targeting behavior test with unit coverage only for now (TestPostStart_ReactionTargetsTriggeringComment etc.). Once #5994 lands I'll come back and add the live scenario.

[ralphbean]

Converting this to draft until #5994 is resolved.

[ralphbean]

Re: #5957 (comment)

• Docs gap (medium): fixed in 8f1c454.
• No follow-up issue for orphaned-reaction cleanup: went with documenting it inline instead (next to startReactionID and in ReconcileOrphaned's doc comment, ad6b587) rather than a tracking issue — same call as the identical suggestion on the reaction-identity review thread.
• Ambiguous log message on refreshClient failure: leaving as is, agreed this is low impact.
• layered-config-reference.md merge-rules gap: that's pre-existing and unrelated to this PR, not fixing it here.