feat: add Pushover last-hop captain notifier (fork-only)#12
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: aeb803060e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| case "$HTTP_CODE" in | ||
| 2??) ;; |
There was a problem hiding this comment.
Check Pushover JSON status before reporting success
Captain, when Pushover returns HTTP 200 with a body like {"status":0,"errors":[...]} for an invalid request, this code accepts the response as delivered and prints sent, even though no notification was queued. Pushover's API docs explicitly say success requires the JSON/XML status value to be 1, and that a non-1 status is invalid input, so this can silently lose the emergency page in exactly the last-hop path this script is meant to protect.
Useful? React with 👍 / 👎.
Implements the captain-alert-channel-b3 recommendation: urgent/emergency tiers map to Pushover priority 1/2, secrets come from 1Password at runtime only, and every documented failure mode fails loud.
aeb8030 to
67c747c
Compare
Intent
curl-based Pushover sender, no daemon, no inbound port.urgent-> priority 1 (bypasses quiet hours, sounds once),emergency-> priority 2 (Pushover re-alerts everyretryseconds, up toexpireseconds, until acknowledged in the app).FM_NOTIFY_RETRY/FM_NOTIFY_EXPIREenv-override the emergency defaults (30s / 3600s), validated against Pushover's own 30s minimum / 10800s maximum.op item get Pushover --vault CLI --fields username|credential --reveal) at call time only, live in shell variables, and are sent to curl over stdin (--data @-) sopsnever shows them.OP_SERVICE_ACCOUNT_TOKENis read from the environment (source it from your shell profile, per existing fleet practice); its absence is a loud, immediate failure.OP_SERVICE_ACCOUNT_TOKEN, a missing or empty 1Password field (the error names the exact field), and a non-2xx Pushover response (prints Pushover'serrorsarray) all exit non-zero with a clear message.--dry-runpreviews the tier/priority/retry/expire/message with secrets redacted and makes no external calls at all (noop, nocurl) - tested with tripwire mocks that fail the test if either is invoked.AGENTS.mdsection 9 and a catalog row indocs/scripts.md; no other docs needed changes.What Changed
bin/fm-notify-captain.sh(new): the notifier, shellcheck-clean, self-contained (no sibling lib sourcing needed).tests/fm-notify-captain.test.sh(new): 18 cases covering tier->priority mapping and retry/expire overrides (all via--dry-run, no external calls), the dry-run send-nothing guarantee, every loud-failure path (missing token, absentopbinary, missing/empty 1Password field on eitherusernameorcredential, non-2xx response), and two success-path cases (mockedop/curl) confirming the emergency receipt prints and that secrets never appear in curl's argv.AGENTS.md: one-line pointer in the escalation section.docs/scripts.md: one-row catalog entry.Risk Assessment
Low. New, isolated script with no callers wired up yet (ClickStack auto-wiring is explicitly out of scope); firstmate will invoke it by judgment. No changes to existing scripts, watcher, or supervision paths.
Testing
shellcheck bin/fm-notify-captain.sh tests/fm-notify-captain.test.sh- clean../tests/fm-notify-captain.test.sh- 18/18 pass.shellcheckover everybin/*.sh/bin/backends/*.sh/tests/*.sh, then everytests/*.test.sh): clean except one pre-existing, unrelated failure -tests/fm-session-start.test.sh's "pi supervision block missing" case fails identically on a cleanfork/maincheckout with none of this PR's changes present (confirmed viagit stashshowing zero tracked-file diff before this work). Not touched by this PR; worth tracking separately. This PR's own gate run (--check) excludes that one file and is otherwise the unmodified CI mirror.Full narrative / original brief
Build firstmate's emergency captain-notification "last hop": a Pushover sender script, per the accepted recommendation in the captain-alert-channel-b3 report, which defines the tier ladder (urgent/emergency -> Pushover priority 1/2) and verified Pushover API behavior (priority 2 requires
retry/expire, re-alerts until acknowledged, receipt id in the response for later polling via the receipts API).Deliverable:
bin/fm-notify-captain.shwith--tier <urgent|emergency> "<message>"(+ optional--title), secrets pulled from 1Password at runtime only, POST tohttps://api.pushover.net/1/messages.json, loud non-zero failure on every documented failure mode, and a--dry-runflag. Tests colocated attests/fm-notify-captain.test.sh, no real sends from tests.Field-name correction mid-task from the captain: the 1Password Pushover item (vault CLI) has the user key in field
usernameand the application token in fieldcredential- there is noapi tokenfield. The script and tests were updated to match.