feat(discord): notify captain on task done/blocked/failed status - #82
Merged
Merged
Conversation
Captain asked for a Discord ping on every completed task plus failures and auth/approval blockers, not just the existing merge-ready notification.
…cked/failed notify
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Intent
The developer (captain), operating overnight via a delegated Firstmate supervisor session, worked through a long backlog of fleet/automation maintenance: fixing a Discord duplicate-reply bug in the code-governance-sentinel pipeline, cleaning up 7 duplicate "auto-update-all" Notion issue pages causing repeated false alerts, filtering stale/retired job noise and raw stack traces out of the daily briefing, and reordering truncated Discord quota-alert messages so actionable items aren't cut off. They also asked for an automatic Discord notification whenever a task completes, fails, is blocked, or needs a decision (not just PR-ready/merge-wait), and specified this must go through proper code review/testing (30+ tests passing) and push through the repo's "no-mistakes" pipeline rather than a direct PR, since the firstmate repo requires PRs to go through that gate. The developer corrected a misdiagnosis (rejecting a "leave it as-is / mark 보류" plan) and insisted the duplicate Notion entries be deleted outright rather than reclassified, and told the assistant to act autonomously on review/approval decisions going forward rather than asking for confirmation each time. They also delegated verification of a sentinel false-positive fix (complexity_guard git diff range, model_quota_guard log-scan boundary, stale-lock pid file) to be investigated thoroughly for root-cause rather than surface fixes, in coordination with another agent (codex/opencode2).
What Changed
fm-discord-notify-status.sh: addfm_discord_send_plain_reporthelper and handledone:*,blocked:*,failed:*status lines by sending a plain Discord report (완료/막힘/실패) with the task's note, silently no-op when Discord is unconfigured.fm-watch.sh: updatesignal_discord_decision_notificationscomment to reflect that it now also pushes done/blocked/failed plain reports, not just yolo-off PR-ready.docs/scripts.md: updatefm-discord-notify-status.shdescription to cover the new done/blocked/failed mapping.tests/fm-discord-decision-push.test.sh: add test coverage for the new done/blocked/failed notification paths.🤖 Generated with Claude Code
Risk Assessment
✅ Low: Doc/comment fix is accurate and matches new behavior; added tests exercise real CLI execution and assert actual posted Discord payload content (task id, note text, channel), not source-text matching, closing prior round's coverage gap without introducing new defects.
Testing
Targeted test file for this change was run in full (not the whole repo suite) and all 23 cases passed, including the 3 new ones covering done/blocked/failed. An additional manual adversarial check confirmed the notify-status script silently no-ops when Discord is unconfigured. Both recorded human fix decisions (stale doc/comment update, missing test coverage for new verbs) are present and verified working in this commit.
Pipeline
Updates from git push no-mistakes
✅ **intent** - passed
✅ No issues found.
✅ **Rebase** - passed
✅ No issues found.
🔧 **Review** - 2 issues found → auto-fixed ✅
docs/scripts.md:155- docs/scripts.md:155 still describes fm-discord-notify-status.sh as mapping only "an ask-user gate or yolo-off PR-ready status line," but this change adds done/blocked/failed plain-report notifications too. The sibling comment at bin/fm-watch.sh:1932-1934 ("Push only the two additional captain-facing transitions") is equally stale now that four transitions are pushed. Both are one-line doc/comment fixes with no behavior change.bin/fm-discord-notify-status.sh:80- The done:/blocked:/failed: branches are the headline feature of this change (commit title: "notify on task done/failed/blocked, not just pr-ready"), but no test exercises them — tests/fm-discord-decision-push.test.sh is unchanged and only covers the ask-user-gate and pr-ready paths. A wrong case match, wrong note extraction, or wrong channel selection in these new branches would go undetected.🔧 Fix applied.
✅ Re-checked - no issues remain.
✅ **Test** - passed
✅ No issues found.
bash tests/fm-discord-decision-push.test.sh (full file, 23/23 ok)manual: FM_DISCORD_BOT_TOKEN='' FM_DISCORD_CHANNEL_ID='' bin/fm-discord-notify-status.sh task-z 'done: nothing configured'✅ **Document** - passed
✅ No issues found.
✅ **Lint** - passed
✅ No issues found.
✅ **Push** - passed
✅ No issues found.