feat: move agent status comments into fullsend run

[ggallen]

Closes #1859, #1873. Supersedes #1860. Related: #837, #957.

Summary

Adds forge-portable agent status comments to fullsend run. When agents start and complete, the CLI posts status comments on the originating issue/PR — all driven by config.yaml settings and passed via CLI flags from the CI dispatcher.

• Config-driven: A new status_notifications section in config.yaml controls comment start/completion (enabled/disabled)
• Forge-portable: Uses forge.Client interface methods, not gh CLI or GitHub-specific shell commands — works for any forge implementation
• Edit-in-place: When the start comment is still the last entry on the issue, the completion overwrites it to reduce noise; otherwise a new comment is posted to preserve timeline ordering
• Non-blocking: Status notification failures log warnings but never fail the agent run

Comment format

Start:

<!-- fullsend:agent-status:{RUN_ID} -->
🤖 Reviewing This PR · Started 2:34 PM UTC
Commit: `a1b2c3d` · [View workflow run →](https://...)

Completion (edited in place when possible):

<!-- fullsend:agent-status:{RUN_ID} -->
🤖 Finished Reviewing This PR · ✅ Success · Started 2:34 PM UTC · Completed 2:41 PM UTC
Commit: `a1b2c3d` · [View workflow run →](https://...)

Cancellation: Deletes the start comment, leaving no trace on the timeline.

Config example

defaults:
  status_notifications:
    comment:
      start: enabled      # "enabled" (default) | "disabled"
      completion: enabled  # "enabled" (default) | "disabled"

When status_notifications is omitted from config, comments default to enabled.

Changes

Config layer (internal/config/)

• StatusNotificationConfig, CommentNotificationConfig struct types
• StatusNotifications *StatusNotificationConfig field on RepoDefaults (omitempty — existing configs unaffected)
• Validation: comment values must be "", "enabled", or "disabled"

Forge layer (internal/forge/)

• 1 new method on Client interface: DeleteIssueComment
• GitHub implementation: DELETEs by comment ID
• FakeClient stub with recorder slice (DeletedComments)

Status comment package (internal/statuscomment/)

• New Notifier struct managing the full lifecycle
• PostStart: creates comment with HTML marker for identification
• PostCompletion: finds start comment via timeline analysis, edits in place if still last or if agent posted output, otherwise posts new completion comment
• Cancellation path: deletes start comment
• Completion-disabled path: deletes start comment to prevent orphaning

CLI wiring (internal/cli/run.go)

• New flags: --run-url, --status-repo, --status-number, --status-token
• setupStatusNotifier() reads StatusNotifications from config.yaml, creates a forge/github client, and returns a Notifier
• Posts start comment before sandbox creation; defers completion with success/failure derived from runErr
• Falls back to GH_TOKEN env var when --status-token is not provided
• Uses GITHUB_SHA and GITHUB_RUN_ID env vars for commit and run identification

Composite action + workflows

• action.yml: 4 new optional inputs (run-url, status-repo, status-number, status-token); conditional flag construction in the "Run fullsend" step
• All 5 reusable workflows (reusable-{code,fix,retro,review,triage}.yml) pass run-url, status-repo, status-number, status-token to the action

Test plan

• go test ./internal/config/... — notification config parse/validate/marshal
• go test ./internal/forge/github/... — DeleteIssueComment API test
• go test ./internal/statuscomment/... — full lifecycle with FakeClient
• go test ./... — all tests pass
• go vet ./... — clean
• make lint — all checks pass
• Deploy to staging and verify comments appear on a real issue/PR
• Verify edit-in-place works when no intervening comments
• Verify new completion comment when human comments between start and completion
• Verify cancellation deletes the start comment
• Verify no API calls when status_notifications is omitted from config

🤖 Generated with Claude Code

[github-actions]

Site preview

Preview: https://adb98c87-site.fullsend-ai.workers.dev

Commit: 8defe0e86d40ed8167623be420e5e8d89a5d64c9

[fullsend-ai-review]

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified to pass four new status notification inputs (run-url, status-repo, status-number, status-token) to the composite action. The PR links to issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873 which authorize these changes, and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

Low

• [consumer-completeness] .github/workflows/reusable-prioritize.yml — This workflow invokes the composite action with agent: prioritize but does not pass the new run-url, status-repo, status-number, or status-token inputs. All five other reusable workflows (code, fix, retro, review, triage) pass these inputs. The action defaults them to empty strings so nothing breaks, but the prioritize agent will silently skip status comments while every other agent posts them. If this is intentional, a code comment would clarify.

• [edge-case] internal/cli/run.go — In the deferred completion handler, ctx.Err() != nil is checked before runErr != nil. If both context cancellation and an agent error occur, the status is reported as "cancelled" (triggering start comment deletion) rather than "failure". This means the user loses the record that the agent was attempted. Preserved from prior review — unchanged code, deliberate design.

• [scope-completeness] Issue feat: forge-portable agent status notifications #1873 proposes both comment and reaction notifications (the config example includes reaction: { start: eyes, completion: rocket }). This PR implements only comments. While partial implementations are acceptable, the PR description does not explicitly call out that reactions are deferred to follow-up work.

Info

• [scope-alignment] internal/config/config.go — StatusNotifications is placed in RepoDefaults (org-wide) but intentionally excluded from RepoConfig (no per-repo override). The code comment explains this is "an org-wide UX decision," which is a reasonable design choice. Preserved from prior review.

• [pr-metadata-accuracy] The PR body mentions "emoji reactions" in the issue context (feat: forge-portable agent status notifications #1873) but the implementation only covers status comments. No emoji reaction API calls exist in the diff. The statuscomment package uses emoji characters in comment text but does not use GitHub's native reaction API.

Previous run

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified to pass four new status notification inputs (run-url, status-repo, status-number, status-token) to the composite action. The PR links to issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873 which authorize these changes, and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

Low

• [edge-case] internal/cli/run.go — In the deferred completion handler, ctx.Err() != nil is checked before runErr != nil. If both context cancellation and an agent error occur, the status is reported as "cancelled" (triggering start comment deletion) rather than "failure". This means the user loses the record that the agent was attempted. Preserved from prior review — unchanged code, deliberate design.

• [scope-completeness] Issue feat: forge-portable agent status notifications #1873 proposes both comment and reaction notifications (the config example includes reaction: { start: eyes, completion: rocket }). This PR implements only comments. While partial implementations are acceptable, the PR description does not explicitly call out that reactions are deferred to follow-up work.

Info

• [scope-alignment] internal/config/config.go — StatusNotifications is placed in RepoDefaults (org-wide) but intentionally excluded from RepoConfig (no per-repo override). The code comment explains this is "an org-wide UX decision," which is a reasonable design choice. Preserved from prior review.

• [pr-metadata-accuracy] The PR body mentions "emoji reactions" in the issue context (feat: forge-portable agent status notifications #1873) but the implementation only covers status comments. No emoji reaction API calls exist in the diff. The statuscomment package uses emoji characters in comment text but does not use GitHub's native reaction API.

Previous run (2)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified to pass four new status notification inputs (run-url, status-repo, status-number, status-token) to the composite action. The PR links to issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873 which authorize these changes, and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

Low

• [edge-case] internal/cli/run.go — In the deferred completion handler, ctx.Err() != nil is checked before runErr != nil. If both context cancellation and an agent error occur, the status is reported as "cancelled" (triggering start comment deletion) rather than "failure". This means the user loses the record that the agent was attempted. Preserved from prior review — unchanged code, deliberate design.

• [scope-completeness] Issue feat: forge-portable agent status notifications #1873 proposes both comment and reaction notifications (the config example includes reaction: { start: eyes, completion: rocket }). This PR implements only comments. While partial implementations are acceptable, the PR description does not explicitly call out that reactions are deferred to follow-up work.

Info

• [scope-alignment] internal/config/config.go — StatusNotifications is placed in RepoDefaults (org-wide) but intentionally excluded from RepoConfig (no per-repo override). The code comment explains this is "an org-wide UX decision," which is a reasonable design choice. Preserved from prior review.

• [pr-metadata-accuracy] The PR body mentions "emoji reactions" in the issue context (feat: forge-portable agent status notifications #1873) but the implementation only covers status comments. No emoji reaction API calls exist in the diff. The statuscomment package uses emoji characters in comment text but does not use GitHub's native reaction API.

Previous run (3)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified to pass four new status notification inputs (run-url, status-repo, status-number, status-token) to the composite action. The PR links to issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873 which authorize these changes, and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

• [consumer-completeness] internal/forge/fake_test.go — The DeleteIssueComment method is added to the forge.Client interface and implemented on both LiveClient and FakeClient, but it is missing from both the error-injection test table (TestFakeClient_ErrorInjection) and the thread-safety test (TestFakeClient_ThreadSafety). Every other method on the interface is covered by both test tables. This means error injection for DeleteIssueComment is untested on the fake, and race detector coverage is absent for concurrent DeleteIssueComment calls.
  Remediation: Add {"DeleteIssueComment", func(fc *FakeClient) error { return fc.DeleteIssueComment(ctx, "o", "r", 1) }} to the error-injection table and _ = fc.DeleteIssueComment(ctx, "o", "r", 1) to the thread-safety goroutine.

Low

• [edge-case] internal/cli/run.go:285 — In the deferred completion handler, ctx.Err() != nil is checked before runErr != nil. If both context cancellation and agent error occur, the status is reported as "cancelled" (triggering start comment deletion) rather than "failure". This is arguably correct since cancellation is the root cause, but the user loses the record that the agent was attempted. Preserved from prior review — unchanged code, deliberate design.

• [scope-completeness] Issue feat: forge-portable agent status notifications #1873 proposes both comment and reaction notifications (the config example includes reaction: { start: eyes, completion: rocket }). This PR implements only comments. While partial implementations are acceptable, the PR description does not explicitly call out that reactions are deferred to follow-up work.

• [stale-doc-reference] docs/guides/user/running-agents-locally.md — The new text references ADR 0011 as the "config reference" for status_notifications, but ADR 0011 does not document this field (it defers to internal/config/config.go as the canonical schema). The installation guide (docs/guides/getting-started/installation.md) already documents the field directly and would be a better link target.

Info

• [scope-alignment] internal/config/config.go — StatusNotifications is placed in RepoDefaults (org-wide) but intentionally excluded from RepoConfig (no per-repo override). The code comment explains this is "an org-wide UX decision," which is a reasonable design choice. Preserved from prior review.

• [pr-metadata-accuracy] The PR body mentions "emoji reactions" in the issue context (feat: forge-portable agent status notifications #1873) but the implementation only covers status comments. No emoji reaction API calls exist in the diff. The statuscomment package uses emoji characters in comment text but does not use GitHub's native reaction API.

Previous run (4)

Review

Findings

Medium

• [reduced-observability] internal/mintcore/handler.go — The PR removes all server-side logging of granted scope from the mint token handler: the repository_selection=all warning, permission-level mismatch warnings, and the detailed granted scope: log line. These logs were the only server-side mechanism to detect when GitHub over-granted permissions (e.g., repository_selection=all when specific repos were requested, or extra/mismatched permissions). Without these warnings, a misconfigured GitHub App installation that grants broader access than requested will go unnoticed in production logs. The client-side mint-token/action.yml also removes the corresponding granted-scope echo statements. Consider retaining the WARNING log lines for repository_selection=all and permission mismatches as a defense-in-depth measure, even if the GrantedScope type and response fields are removed from the API surface.

• [protected-path] .github/actions/mint-token/action.yml, .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Six files under .github/ are modified. The mint-token action simplifies token extraction (pipes curl through jq directly). The five workflow files pass four new status notification inputs (run-url, status-repo, status-number, status-token) to the composite action. The PR links to issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873 which authorize these changes, and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

Low

• [edge-case] internal/statuscomment/statuscomment.go:107 — When start is "enabled" but completion is "disabled", and the run completes, PostCompletion skips all comment activity. The start comment remains permanently in its "Started" state with no completion update. While this is by design (admin opted out of completion comments), the orphaned start comment may confuse users who see "Started" with no follow-up. Consider: if completion is disabled but a start comment exists, should the start comment be deleted or minimized on completion?

• [edge-case] internal/cli/run.go:285 — In the deferred completion handler, ctx.Err() != nil is checked before runErr != nil. If both context cancellation and agent error occur, the status is reported as "cancelled" (triggering start comment deletion) rather than "failure". This is arguably correct since cancellation is the root cause, but the user loses the record that the agent was attempted. Preserved from prior review — unchanged code, deliberate design.

Info

• [scope-alignment] internal/config/config.go — StatusNotifications is placed in RepoDefaults (org-wide) but intentionally excluded from RepoConfig (no per-repo override). The code comment explains this is "an org-wide UX decision," which is a reasonable design choice. Preserved from prior review.

• [pr-metadata-accuracy] The PR body mentions "emoji reactions" as part of the implementation, but the actual code only implements status comments. No emoji reaction API calls exist in the diff. The statuscomment package uses emoji characters in comment text but does not use GitHub's native reaction API. The prior ADR edits claiming reactions were implemented have been reverted, but the PR body description still contains the inaccurate claim.

Previous run (5)

Review

Findings

High

• [adr-immutability] docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md:129 — The Consequences section of Accepted ADR 0041 is modified in place: the caveat "notification UX may still need follow-up issues" is removed and a new bullet is added claiming lifecycle emoji reactions and status comments are implemented. ADR 0041 has status Accepted and its Consequences section is frozen per CONTRIBUTING.md and AGENTS.md ("Once accepted, their content is frozen — do not edit the Context, Decision, or Consequences sections"). See also: [scope-creep] finding at this location.
  Remediation: Revert the in-place edit to ADR 0041. If documenting the implementation is needed, add it in a new ADR or a non-frozen section (e.g., a "Subsequent work" addendum).

• [adr-immutability] docs/ADRs/0011-admin-install-org-config-yaml-v1.md:38 — A new bullet is appended to the Consequences section of Accepted ADR 0011, documenting the status_notifications config field. This section is frozen per the same governance rules. The config is already documented in code comments (internal/config/config.go) and user-facing docs (docs/guides/getting-started/installation.md).
  Remediation: Remove the appended Consequences bullet. The existing code and user docs already cover this.

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified to pass status notification inputs (run-url, status-repo, status-number, status-token) to the composite action. The PR links to issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873 which authorize these changes, and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

• [scope-creep] docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md:130 — The updated Consequences bullet claims "Lifecycle emoji reactions (Agent lifecycle emoji reactions on trigger comments, issues, and PRs #272)... are now implemented in fullsend run." However, no emoji reaction implementation exists in this PR: StatusNotificationConfig has no Reaction field, the forge.Client interface has no AddIssueReaction or RemoveIssueReaction methods, and internal/statuscomment makes no reaction API calls. The PR body's config example also shows a reaction section that doesn't exist in code. Only status comments (not GitHub API reactions) are implemented. See also: [adr-immutability] finding at this location.

• [error-handling] internal/statuscomment/statuscomment.go:119 — When analyzeTimeline returns an error, the code warns and falls back to creating a new completion comment. However, the original start comment is left on the timeline in its "Started" state — the user sees both a stale "Started" comment and a separate "Finished" comment. While the fail-open behavior is correct, the orphaned start comment is misleading.

Low

• [edge-case] internal/cli/run.go:285 — In the deferred completion handler, ctx.Err() != nil is checked before runErr != nil. If both the context is cancelled and the agent returns an error, the status is reported as "cancelled" (triggering start comment deletion) rather than "failure" (which would preserve a completion record). This is arguably correct since cancellation is the root cause, but the resulting behavior means the user has no record the agent was attempted.

Info

• [scope-alignment] internal/config/config.go — StatusNotifications is placed in RepoDefaults (org-wide) but intentionally excluded from RepoConfig (no per-repo override). The code comment explains this is "an org-wide UX decision," which is a reasonable design choice. Preserved from prior review.

Previous run (6)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified to pass status notification inputs (run-url, status-repo, status-number, status-token) to the composite action. The PR links to issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873 which authorize these changes, and the PR description explains the rationale. Human approval is always required for protected-path changes regardless of context.

Low

• [edge-case] internal/statuscomment/statuscomment.go — The PostCompletion doc comment describes three placement heuristics for success/failure but does not mention that cancellation silently deletes the start comment with no trace left on the timeline. While this is explicitly designed behavior (the PR description states "Cancellation: Deletes the start comment and removes the start reaction"), the function's doc comment should document this fourth path so future maintainers don't need to read the PR description to understand the behavior.

• [markdown-injection] internal/statuscomment/statuscomment.go:222 — isSafeURL rejects non-https schemes, closing parentheses, and newlines, but does not reject ] characters. A crafted --run-url like https://evil.com/x](evil)[click could theoretically break out of the markdown link. Low risk in practice because the run URL is set by reusable workflows from trusted GitHub context (github.server_url/github.repository/github.run_id) and is not user-controllable in normal CI flows.

• [test-adequacy] internal/statuscomment/statuscomment_test.go — No test covers an unknown/unexpected status string (e.g., "timeout"). The code handles it gracefully (warning emoji via the statusEmoji default case, capitalize works on arbitrary strings), but this graceful degradation path is untested.

• [scope-alignment] internal/config/config.go — StatusNotifications is placed in RepoDefaults (org-wide) but intentionally excluded from RepoConfig (no per-repo override), unlike Roles and AutoMerge which exist in both. The comment explains this is "an org-wide UX decision," which is a reasonable design choice but differs from the established config pattern. Consider documenting this rationale in ADR-0011 if per-repo override is intentionally foreclosed.

Previous run (7)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified. The PR links issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873, and the description explains these changes pass four new status notification inputs to the composite action. Human approval is always required for protected-path changes, regardless of context.

Low

• [error-message-consistency] internal/cli/run.go:1825 — The error message format invalid --status-repo %q: expected owner/repo does not match the established codebase pattern must be in owner/repo format, got %q, which is used consistently across 8 call sites in admin.go, mint.go, postreview.go, postcomment.go, and provisioner.go.
  Remediation: Change to --status-repo must be in owner/repo format, got %q to match the established pattern.

• [input-validation] internal/statuscomment/statuscomment.go:211 — The sha value from GITHUB_SHA env var is interpolated into a Markdown code span via backtick wrapping without validating it is hex-only. While GITHUB_SHA is trusted in CI, the --status-token CLI path accepts arbitrary env values. A crafted value containing backticks could break out of the code span. Defense-in-depth concern — realistic attack surface is negligible since the attacker would need the status token.
  Remediation: Validate that sha matches ^[0-9a-fA-F]+$ before interpolating, or strip non-hex characters in shortSHA.

• [test-adequacy] internal/statuscomment/statuscomment_test.go — No test covers the path where GetAuthenticatedUser returns an empty string in analyzeTimeline. When botUser is empty, agentPosted will always be false, which may cause incorrect placement decisions (new comment posted instead of edit-in-place when the bot posted output but could not be identified as the author).
  Remediation: Add a test where AuthenticatedUser is empty and verify the fallback behavior is acceptable.

Previous run (8)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified. The PR links issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873, and the description explains these changes pass four new status notification inputs to the composite action. Human approval is always required for protected-path changes, regardless of context.

Low

• [test-adequacy] internal/statuscomment/statuscomment_test.go — No test covers: (1) cancellation when start comments were disabled (startCommentID == 0), confirming handleCancelled returns nil without attempting deletion; (2) completion when analyzeTimeline returns an error, confirming the fallback to creating a new comment.
  Remediation: Add tests for both paths.

• [design-coherence] internal/statuscomment/statuscomment.go — The new statuscomment package implements marker-based comment lifecycle management (create, update-in-place, delete on cancellation) that overlaps conceptually with the existing internal/sticky package, which also uses HTML markers and update-in-place logic for bot comments. Both packages solve related problems with different feature sets (sticky has history-collapsing; statuscomment has timeline-aware placement heuristics). The separation of concerns between these two abstractions is undocumented.
  Remediation: Add a package comment or code comment in statuscomment.go explaining the relationship to internal/sticky and why a separate package was chosen (e.g., different lifecycle semantics — transient status vs. persistent output).

Previous run (9)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified. The PR links issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873, and the description explains these changes pass four new status notification inputs to the composite action. Human approval is always required for protected-path changes, regardless of context.

Low

• [edge-case] internal/cli/run.go — The defer in runAgent determines status as either "success" or "failure" based on runErr != nil. When the run is cancelled via context (e.g., SIGINT), runErr will be non-nil with a context.Canceled error, so PostCompletion receives status="failure" rather than "cancelled". This means the handleCancelled code path in PostCompletion is unreachable from the current call site — a cancelled run posts a completion comment with a failure emoji instead of cleaning up the start comment. The context.WithoutCancel usage for the deferred context is correct, but the status derivation should detect cancellation.
  Remediation: Detect context cancellation in the defer: if ctx.Err() == context.Canceled { status = "cancelled" }.

• [scope-boundary] internal/cli/run.go — Status notification setup lives at the CLI layer (step 1c of runAgent) rather than in harnesses or post-scripts. The in-code comment (lines 324–327) explains the rationale — wrapping the entire lifecycle including sandbox setup, validation loop, and post-script so it can report cancellation/failure even when the sandbox never starts. This is a pragmatic placement but the architectural trade-off (CLI-layer universality vs. harness-layer consistency per ADR 0024) remains undocumented in ADRs.
  Remediation: Document the CLI-layer placement decision in a follow-up ADR or in ADR 0024's open questions section.

Previous run (10)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified. The PR links issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873, and the description explains these changes pass four new status notification inputs to the composite action. Human approval is always required for protected-path changes, regardless of context.

Low

• [error-handling] internal/cli/run.go — setupStatusNotifier silently ignores os.ReadFile errors other than file-not-found. If config.yaml exists but is unreadable (permission denied), the notifier proceeds with zero-value defaults instead of the intended configuration. The parse-error path now correctly logs a warning, but the ReadFile error path is still silent.
  Remediation: Check for os.IsNotExist(err) explicitly; if the file exists but cannot be read, log a warning via printer.StepWarn.

• [edge-case] internal/statuscomment/statuscomment.go — handleCancelled and the PostCompletion non-cancelled path discard errors from DeleteIssueComment and RemoveIssueReaction via _ = without logging through the warnf function. The fail-open behavior is correct, but silent failures prevent operators from diagnosing API issues (e.g. stale reaction emoji remaining on an issue).
  Remediation: Replace _ = with if err := ...; err != nil { n.warnf(...) } for the cleanup calls.

• [injection] internal/statuscomment/statuscomment.go — runURL is interpolated directly into a Markdown link ([View workflow run →](%s)) without validation. In CI the URL is constructed from trusted github.* context variables, but the --run-url CLI flag accepts arbitrary input. A crafted value containing ) or newlines could break the markdown structure. For forge portability, non-GitHub renderers may not strip javascript: URIs.
  Remediation: Validate that runURL, if non-empty, is an HTTPS URL and does not contain markdown-breaking characters.

• [incomplete-doc] docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md — ADR 0041 identifies the lack of lifecycle emoji reactions (Agent lifecycle emoji reactions on trigger comments, issues, and PRs #272) and notification gaps (Add agent failure notifications to the post-run lifecycle #957, Consolidate and prioritize silent-failure notification issues #988, Failed review agent runs should update the status comment on the PR #837) as pain points. This PR directly addresses several of those issues but the ADR is not updated to reflect that.
  Remediation: Update the Consequences section to note that lifecycle reactions and status notifications have been implemented in fullsend run.

• [scope-boundary] internal/cli/run.go — Status notification setup lives at the CLI layer (step 1c of runAgent) rather than in harnesses or post-scripts. This is pragmatic and enables universal availability, but it deviates from the pattern where agent lifecycle concerns live in harnesses (ADR 0024). The placement decision is not documented.
  Remediation: Document the rationale for CLI-layer placement in the linked issue or a follow-up ADR.

• [configuration-schema-coherence] internal/config/config.go — StatusNotifications is added to RepoDefaults (org-wide) but not to per-repo RepoConfig overrides. Repos cannot independently opt out of or customize status notifications, unlike other defaults (roles, auto_merge).
  Remediation: Clarify design intent — if org-level-only control is deliberate, document the rationale; if repos should be able to override, add StatusNotifications to RepoConfig.

Previous run (11)

Review

Findings

Medium

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified. The PR links issues feat: post agent status comments on workflow start and completion #1859 and feat: forge-portable agent status notifications #1873, and the description explains these changes pass four new status notification inputs to the composite action. Human approval is always required for protected-path changes, regardless of context.

Low

• [error-handling] internal/cli/run.go — setupStatusNotifier silently ignores os.ReadFile errors other than file-not-found. If config.yaml exists but is unreadable (permission denied), the notifier proceeds with zero-value defaults instead of the intended configuration. The parse-error path now correctly logs a warning, but the ReadFile error path is still silent.
  Remediation: Check for os.IsNotExist(err) explicitly; if the file exists but cannot be read, log a warning via printer.StepWarn.

• [edge-case] internal/statuscomment/statuscomment.go — handleCancelled and the PostCompletion non-cancelled path discard errors from DeleteIssueComment and RemoveIssueReaction via _ = without logging through the warnf function. The fail-open behavior is correct, but silent failures prevent operators from diagnosing API issues (e.g. stale reaction emoji remaining on an issue).
  Remediation: Replace _ = with if err := ...; err != nil { n.warnf(...) } for the cleanup calls.

• [injection] internal/statuscomment/statuscomment.go — runURL is interpolated directly into a Markdown link ([View workflow run →](%s)) without validation. In CI the URL is constructed from trusted github.* context variables, but the --run-url CLI flag accepts arbitrary input. A crafted value containing ) or newlines could break the markdown structure. For forge portability, non-GitHub renderers may not strip javascript: URIs.
  Remediation: Validate that runURL, if non-empty, is an HTTPS URL and does not contain markdown-breaking characters.

• [incomplete-doc] docs/ADRs/0041-synchronous-workflow-call-event-dispatch.md — ADR 0041 identifies the lack of lifecycle emoji reactions (Agent lifecycle emoji reactions on trigger comments, issues, and PRs #272) and notification gaps (Add agent failure notifications to the post-run lifecycle #957, Consolidate and prioritize silent-failure notification issues #988, Failed review agent runs should update the status comment on the PR #837) as pain points. This PR directly addresses several of those issues but the ADR is not updated to reflect that.
  Remediation: Update the Consequences section to note that lifecycle reactions and status notifications have been implemented in fullsend run.

• [scope-boundary] internal/cli/run.go — Status notification setup lives at the CLI layer (step 1c of runAgent) rather than in harnesses or post-scripts. This is pragmatic and enables universal availability, but it deviates from the pattern where agent lifecycle concerns live in harnesses (ADR 0024). The placement decision is not documented.
  Remediation: Document the rationale for CLI-layer placement in the linked issue or a follow-up ADR.

• [configuration-schema-coherence] internal/config/config.go — StatusNotifications is added to RepoDefaults (org-wide) but not to per-repo RepoConfig overrides. Repos cannot independently opt out of or customize status notifications, unlike other defaults (roles, auto_merge).
  Remediation: Clarify design intent — if org-level-only control is deliberate, document the rationale; if repos should be able to override, add StatusNotifications to RepoConfig.

Previous run (12)

Review

Findings

High

• [protected-path] .github/workflows/reusable-{code,fix,retro,review,triage}.yml — Five workflow files under .github/ are modified. This is a protected path requiring human approval. The PR has no linked issue explaining why these governance files need to change. While the changes are mechanical (passing 4 new inputs to the composite action), protected-path modifications require justification and human review regardless of scope.
  Remediation: Link this PR to an issue that authorizes the workflow changes, or add justification in the PR description for why the protected files need modification.

Medium

• [context-cancellation] internal/cli/run.go — The deferred notifier.PostCompletion(ctx, ...) reuses the parent context. If the context is cancelled (SIGINT, timeout), the HTTP calls in PostCompletion will fail immediately with a context-cancelled error. The cancellation status comment — arguably the most important one — will never be delivered.
  Remediation: Use a fresh context in the defer: dCtx, dCancel := context.WithTimeout(context.WithoutCancel(ctx), 15*time.Second)

• [pagination] internal/forge/github/github.go — RemoveIssueReaction lists reactions with per_page=100 but has no pagination. If an issue has 100+ reactions, the bot's reaction may not be found, leaving a stale reaction permanently. Other paginated list methods in this file use pagination loops.
  Remediation: Add pagination or iterate until the target reaction is found, consistent with other paginated methods in this file.

• [missing-authorization] internal/statuscomment/statuscomment.go — This PR adds a new feature (status notifications with comments and emoji reactions) without a linked issue establishing authorization. The PR title correctly uses feat: prefix, but per project governance new features need explicit authorization. The feature introduces a new user-facing communication channel that affects notification UX.
  Remediation: Link to an issue that authorizes this feature, or create one documenting the problem being solved and design rationale.

• [design-direction] internal/statuscomment/statuscomment.go — The status notification feature introduces a new dimension of agent visibility (user-facing comments and emoji reactions on issues/PRs) without documented design consideration. docs/problems/operational-observability.md focuses on operator/maintainer visibility (traces, dashboards), not end-user notifications. The UX implications (notification noise, community trust) are undocumented.
  Remediation: Document the design rationale in a problem doc or ADR. Address: Why comments vs. status checks? Notification UX impact? How does this align with the transparency principle?

• [incomplete-doc] docs/guides/user/running-agents-locally.md — Four new fullsend run CLI flags (--run-url, --status-repo, --status-number, --status-token) are not documented in the user guide for running agents.
  Remediation: Add a section documenting the new status notification flags with usage examples.

• [incomplete-doc] docs/guides/getting-started/installation.md — Four new action inputs (run-url, status-repo, status-number, status-token) used by all reusable workflows are not documented in the installation/setup guide.
  Remediation: Add the new inputs to the action inputs reference.

Low

• [error-handling] internal/cli/run.go — setupStatusNotifier silently swallows config parse errors. If config.yaml exists but has a syntax error, the notifier runs with zero-value defaults (all comments enabled, no reactions) instead of the intended configuration.
  Remediation: When os.ReadFile succeeds but ParseOrgConfig fails, return or log the parse error.

• [error-handling] internal/statuscomment/statuscomment.go — In PostCompletion, when isLastComment returns an error, it silently falls through to create a new comment. The fail-open behavior is correct, but the error should be logged so operators can diagnose API issues.

• [input-validation] internal/config/config.go — validateStatusNotifications validates Comment.Start and Comment.Completion against an allowlist but does not validate Reaction.Start or Reaction.Completion. While GitHub's API rejects invalid emoji names, this is inconsistent with the validation applied to comment fields.

• [incomplete-doc] docs/guides/dev/cli-internals.md — CLI command tree does not list the new status notification flags.

• [incomplete-doc] docs/ADRs/0011-admin-install-org-config-yaml-v1.md — The status_notifications config field is not mentioned, though the ADR defers to internal/config/config.go as canonical.

[fullsend-ai-review]

See the review comment for full details.

[ralphbean]

Review

I think this needs one change before we can merge. See inline.

[ralphbean]

LGTM. Token masking and cancellation wiring both look good. Reaction removal is clean.

[ralphbean]

LGTM. One minor note inline.

[ggallen]

Addressing the latest review findings:

[protected-path] — Acknowledged. These are mechanical changes passing status notification inputs to the composite action, authorized by #1859 and #1873.

[error-message-consistency] — Fixed. Changed to --status-repo must be in owner/repo format, got %q matching the pattern used across admin.go, mint.go, postcomment.go, and postreview.go.

[input-validation] SHA hex validation — Fixed. Added isHexOnly() validation in shortSHA() — non-hex input now returns empty string, which causes buildSecondLine to omit the commit line entirely. Backtick injection no longer possible.

[test-adequacy] empty botUser — Fixed. Added three tests:

• TestAnalyzeTimeline_EmptyBotUser_FallsBackToPositionOnly — empty bot user, start is last → edits in place via startIsLast fallback
• TestAnalyzeTimeline_EmptyBotUser_NewCommentWhenNotLast — empty bot user with intervening comments → posts new comment (can't identify agent output)
• TestAnalyzeTimeline_GetAuthenticatedUserError_Warns — API error → warnf fires with error message

Also addressed the inline review comment: GetAuthenticatedUser error now logged via warnf instead of _.

Findings addressed; protected-path requires human approval.

[ralphbean]

LGTM. One minor note inline.

[ralphbean]

LGTM.

[ifireball]

Approving functionality as clarified by the comment discussion