fix(notify): stop nudging about plans already approved in the terminal - #177
Conversation
Approving a plan in Claude Code's own UI left stack-nudge reminding about it, locally and in Slack, for about nine more minutes. ExitPlanMode took the blocking permission path: notify.sh created a FIFO and waited on it for a decision. But answering in CC's UI doesn't end that hook, so it ran to its full 550s timeout — and for the whole of that the panel saw a live hook holding a live FIFO, which is exactly its definition of a prompt still waiting. #173 retires a prompt whose hook has died; this one hadn't. ExitPlanMode belongs with AskUserQuestion instead. Both are answered in the agent's own UI, and for both an "allow" written from the panel means "take the default" — here, approving a plan the user hasn't read. With has_action=false there's no FIFO, so panel Enter focuses the editor and no reminder loop starts. Verified against the installed hook: ExitPlanMode now returns in 0s creating no FIFO, AskUserQuestion is unchanged, and a Bash permission still creates its FIFO and blocks. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Review of the ExitPlanMode change turned up two defects underneath it. The EXIT trap's cleanup has never run. Its body referenced `$fifo`, a local of the function that installed it, and a single-quoted trap body expands when the trap fires — by then the function has returned and the local is gone, so it ran as `rm -f ""` and `rmdir .`. Every permission prompt leaked its FIFO directory until the 30-minute sweep collected it. Verified both ways on the normal completion path: before, the directory survives an answered prompt; after, it's removed. That also invalidates a comment in AttentionPolicy: 536 leaked directories were read as proof the trap had been skipped, when they were proof it had run and done nothing. The conclusion there still holds — a SIGKILLed hook skips the trap outright — but the stated evidence for it did not, so it's corrected. And `is_question_event` bailed out when jq was missing, which meant ExitPlanMode went back on the blocking path and the panel offered Allow on a plan nobody had read. Only macOS 15+ preinstalls jq; post_to_panel already parses the same JSON with python3 for exactly this reason. tool_name now falls back the same way, confirmed to still return ExitPlanMode with jq off PATH. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
c4e8060 to
6ef09ee
Compare
The README said the cleanup 'runs on exit' and is only skipped on SIGKILL. It never ran at all: the trap body referenced a local that was out of scope by the time it fired. Says so now, alongside the SIGKILL case which is still true.
A reviewer falsified the story I told about the trap, and reproducing it proves them right. Bash defers a trapped signal until the foreground child exits, so on INT/TERM/HUP the handler runs *inside* wait_for_permission_response, where the local is still in scope: SIGTERM while blocked: TRAP fifo=[/tmp/scopetest.../fifo] -> cleaned up clean exit: TRAP fifo=[] -> leaked So the old trap worked on signals and failed only on the clean-exit path — answered or timed-out prompts. "It cleaned up nothing on any prompt" was wrong, and I'd written it into the code comment, the README and AttentionPolicy. All three now say which path was broken. The 536 leaked directories don't settle it either way: SIGKILL runs no trap at all and produces exactly the same evidence. That was the original mistake — an inference the data couldn't support — and I repeated it in the other direction. Also corrects the INT/TERM/HUP comment, which claimed bash runs the EXIT trap for a plain SIGTERM. It does, but not promptly: a trapped signal doesn't terminate bash, so the hook runs on to its 550s timeout and keeps answering kill(0) regardless. The fix itself is unchanged and still correct — the clean-exit path was genuinely leaking, and PERM_FIFO fixes it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
StuBehan
left a comment
There was a problem hiding this comment.
Fix reads right - I ran hook_tool_name/is_question_event with and without jq on the path and both classify ExitPlanMode as a question, so that fallback is load bearing and not just belt and braces. Trap bug is a good spot too.
One thing not in the diff: the README still promises reminders and a live menu-bar count for prompts waiting on you, and there's now a class of blocking prompt that gets neither. Worth a line there.
Two questions inline.
| case "$tool_name" in | ||
| AskUserQuestion) return 0 ;; | ||
| *) return 1 ;; | ||
| AskUserQuestion|ExitPlanMode) return 0 ;; |
There was a problem hiding this comment.
permission_context has an AskUserQuestion case so the body isn't just the tool name, but ExitPlanMode falls through to the catch-all. That banner is now the only notification a plan gets - could we pull .tool_input.plan and truncate it the same way?
| # it pick a default. By emitting has_action=false, panel Enter falls | ||
| # through to focusing the editor so the user answers in the terminal. | ||
| # | ||
| # ExitPlanMode is the same shape: "allow" approves the plan outright, so the |
There was a problem hiding this comment.
Is there really no liveness signal though? We only register Stop and PermissionRequest - wouldn't a PostToolUse matcher on ExitPlanMode fire when the plan resolves either way? Not for this PR, but "isn't avoidable at this layer" is a strong thing to write down and then never revisit 🤔
Reported in use: approving a plan in Claude Code's own UI leaves stack-nudge reminding about it — locally and in Slack — for roughly nine more minutes.
Cause
ExitPlanModetook the blocking permission path.notify.shcreated a FIFO and waited on it for an allow/deny decision, becauseis_question_eventonly excludedAskUserQuestion.Answering in CC's own UI doesn't end that hook, so it ran to its full 550s timeout. For the whole of that window the panel saw a live hook holding a live FIFO — precisely its definition of "a prompt is still waiting". Reminders are capped at
maxReminders = 3, so the nagging is bounded at three; what persisted the full 550s was the menu-bar pending count and an Allow/Deny affordance that could no longer do anything.The stronger justification is the second one below, and it doesn't depend on the timing story: an
allowon ExitPlanMode emits{"behavior":"allow"}, approving the plan outright — while the banner body is the bare stringExitPlanMode, so no plan text ever reaches the panel or Slack. The button approved something its approver could not see.#173 fixed the neighbouring case, retiring a prompt whose hook has died. This hook hadn't died; it was still blocked on a FIFO nobody was ever going to write to.
Fix
ExitPlanModebelongs withAskUserQuestion. Both are answered in the agent's own UI, and for both an "allow" written from the panel means "take the default" — which here means approving a plan the user hasn't read. Withhas_action=falseno FIFO is created, panel Enter falls through to focusing the editor, and no reminder loop starts.Verification
Against the installed hook, before and after:
ExitPlanMode(before)ExitPlanMode(after)AskUserQuestionBashAlso confirmed directly that a SIGKILLed hook leaves its FIFO directory behind, which is why FIFO existence alone can't stand in for liveness.
Accepted trade-off
ExitPlanMode now gets one banner and one Slack DM, and no reminders or menu-bar count — the same deal AskUserQuestion has had. A plan approval is the canonical "agent is blocked until you come back" event, so that is a real cost.
It isn't avoidable at this layer: without a blocking hook there is no liveness signal, so the panel cannot tell a plan still waiting from one already approved. Reminding anyway would reinstate exactly the bug this fixes. Decoupling "answerable from the panel" from "worth reminding about" would need a signal we don't currently have.
Follow-up worth filing
notify.shstill has no test harness —bash -nand shellcheck only — sois_question_eventand the trap fix are covered by nothing but the manual checks above. That's the gap flagged when #169 went in, and all three bugs in this PR lived exactly there. A ~30-line harness (fixture on stdin, a socket stub, asserthas_actionandhook_pid) would pin the lot.Separately:
make reloaddoesn't restart the app on my machine.launchctl kickstartfails with "Could not find service com.stackonehq.stack-nudge" and|| trueswallows it, so it prints "reloaded" while the old binary keeps running.