From 833b7b7f3a74e2d43a2067d3ac6ce3b069f076ff Mon Sep 17 00:00:00 2001 From: Hisku Date: Thu, 10 Sep 2026 14:46:38 +0100 Subject: [PATCH 1/4] fix(notify): rate-limit finished-turn DMs so an absence isn't fifty messages MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The idle threshold is a floor, not a rate limit. Once you cross it the condition stays true for the whole time you are away, so every event passed it independently — the setting answers "are you away?" and nothing answered "how often should I interrupt you while you are?" Diagnosed against a real event log: 1284 records, 96% of them `stop`, with busy hours running 50 to 56 events. With "also notify on finished turns" on, an hour away was around fifty DMs rather than the six a ten-minute idle setting suggests. Finished turns now collapse into one message per 15 minutes, and that message says what it swallowed ("finished a turn · 7 more while you were away") so a quiet hour reads as one message about eight turns rather than looking like eight went missing. The cooldown resets whenever the user is back at the machine, so the first stop of each absence still arrives promptly instead of being eaten by a window left running from the previous one. With the idle gate set to Always there is no "present" to detect and the cooldown simply runs continuously, which is what that setting asks for. Permission prompts are deliberately exempt. Each blocks an agent until it is answered, they are rare — 45 of those 1284 — and repeats of any single one are already bounded by AttentionPolicy.maxReminders. Throttling them would withhold precisely the notifications that are actionable. The burst test asserts conservation rather than a message count: every finished turn is either reported in a sent message or still pending in the suppressed count. A count alone would have passed while silently dropping events. Co-Authored-By: Claude Opus 5 --- README.md | 1 + .../SlackDeliveryTests.swift | 92 +++++++++++++++++++ panel/Panel.swift | 42 ++++++++- panel/SlackNotifier.swift | 44 ++++++++- 4 files changed, 176 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 8ebc849..4736107 100644 --- a/README.md +++ b/README.md @@ -185,6 +185,7 @@ The token lives in the **Keychain**, never `~/.stack-nudge/config`. For scripted Three things worth knowing about how it behaves: - **It's idle-gated, not a mirror.** By default nothing reaches Slack until the Mac has been untouched for 5 minutes — there's no point pinging your phone about a prompt you're already looking at. Set it to `Always` if you'd rather have everything. Reminders for prompts you never answered skip the gate: you may be back at the desk and still not have seen the banner. +- **Finished turns are rate-limited; permission prompts aren't.** The idle setting decides *whether* you're away, not how often you're interrupted while you are — so on its own it would send a DM per event, and an unattended hour can produce fifty. Finished turns are therefore folded into one message per 15 minutes, which says what it swallowed (*"finished a turn · 7 more while you were away"*). Permission prompts are never batched: each one is blocking something until you answer it, and repeats of a single prompt are already capped at three. - **A global mute does *not* silence Slack.** Mute means "stop interrupting me *here*", and Slack exists precisely because you're elsewhere. Use the Slack switch to stop it. A *per-session* mute does apply, same as it does to banners. - **Titles only, by default.** A DM reads *"Claude Code in attack-lib needs permission"*. The tool call itself (`Bash(rm -rf build/)`) stays on your machine unless you turn on **Include message text**, because command lines carry paths, hostnames, and sometimes secrets, and this is the one path that leaves the Mac. diff --git a/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift b/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift index e848f02..a1f7d02 100644 --- a/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift +++ b/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift @@ -133,4 +133,96 @@ final class SlackDeliveryTests: XCTestCase { XCTAssertEqual(SlackDelivery.idleLabel(5), "5m") XCTAssertEqual(SlackDelivery.idleLabel(60), "60m") } + + // MARK: - Finished-turn rate limiting + + private let t0 = Date(timeIntervalSince1970: 1_800_000_000) + + // The bug: the idle threshold is a floor, not a rate limit. Once you cross + // it the condition stays true for the whole absence, so every event passes + // independently. Diagnosed from a real log — 1284 events, 96% of them stop, + // busy hours running 50+ — which meant an hour away was ~50 DMs. + func test_burstOfFinishedTurnsCollapsesToOneMessage() { + let turns = 50 + let spacing = 72.0 // ~every 72s, i.e. just under an hour total + var lastSent: Date? + var suppressed = 0 + var sent = 0 + var reported = 0 // events accounted for in a sent message + + for i in 0.. 0, + IdleTime.seconds() < TimeInterval(nav.slackIdleMinutes * 60) + else { return } + lastStopSlackAt = nil + suppressedStopCount = 0 + } + // A permission prompt we can *prove* is still blocking: notify.sh creates // the FIFO before emitting the event and removes it on exit (the EXIT trap // in wait_for_permission_response), so the file existing means nobody has diff --git a/panel/SlackNotifier.swift b/panel/SlackNotifier.swift index 5fe3d31..c111739 100644 --- a/panel/SlackNotifier.swift +++ b/panel/SlackNotifier.swift @@ -58,6 +58,41 @@ enum SlackDelivery { return idleSeconds >= TimeInterval(idleThresholdMinutes * 60) } + // MARK: - Rate limiting + + // The idle threshold is a floor, not a rate limit. Once you cross it the + // condition stays true for the whole time you are away, so every event + // passes it independently — on the log this was diagnosed from, 1284 events + // of which 96% were `stop`, with busy hours running 50+. An hour away with + // "notify on finished turns" on meant fifty DMs, not one. + // + // A stop DM says "your agent finished, come back". Once you know that, the + // next seven say nothing new, so they are folded into a count on the next + // one that gets through rather than dropped silently. + // + // Permission prompts are deliberately exempt. Each blocks an agent until it + // is answered, they are rare (45 of those 1284), and repeats of any single + // one are already bounded by AttentionPolicy.maxReminders. Throttling them + // would withhold the notifications that are actually actionable — the more + // so now that a prompt can be answered from Slack. + static let stopCooldown: TimeInterval = 15 * 60 + + enum StopThrottle: Equatable { + case send(coalesced: Int) // how many were swallowed since the last send + case suppress + } + + static func throttleStop(now: Date, + lastSentAt: Date?, + suppressed: Int, + cooldown: TimeInterval = stopCooldown) -> StopThrottle { + // No previous send means this is the first stop of an absence, which is + // the one worth having promptly — the cooldown starts from it. + guard let lastSentAt else { return .send(coalesced: suppressed) } + guard now.timeIntervalSince(lastSentAt) >= cooldown else { return .suppress } + return .send(coalesced: suppressed) + } + // `label` is the resolved session name; falls back to the repo the event came // from. Detail is opt-in because a permission message is raw tool text — // "Bash(rm -rf build/)" — which can carry paths, hostnames, and secrets in @@ -65,7 +100,8 @@ enum SlackDelivery { static func text(for event: NudgeEvent, label: String?, includeDetail: Bool, - isReminder: Bool) -> String { + isReminder: Bool, + coalesced: Int = 0) -> String { let who = agentName(event.agent) let subject = (label ?? projectName(event.projectPath)) .map { "\(who) in \($0)" } ?? who @@ -77,7 +113,11 @@ enum SlackDelivery { ? "\(subject) is still waiting for permission" : "\(subject) needs permission" case .stop: - headline = "\(subject) finished a turn" + // Say what was folded in, so a quiet hour reads as one message about + // eight turns rather than looking like eight turns went missing. + headline = coalesced > 0 + ? "\(subject) finished a turn · \(coalesced) more while you were away" + : "\(subject) finished a turn" case .other: headline = "\(subject) sent a nudge" } From 174acb03f0d1442f8e38891c88bb56186e6aee52 Mon Sep 17 00:00:00 2001 From: Hisku Date: Fri, 11 Sep 2026 10:05:31 +0100 Subject: [PATCH 2/4] fix(notify): retire a prompt whose hook is gone, instead of nudging about it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Approving a plan in the terminal left the notification standing, locally and in Slack, and kept re-nudging about a prompt that had already been answered. The panel treated the existence of notify.sh's FIFO as proof a prompt was still blocking. That holds only while the hook that created it is alive to read a decision, and the cleanup is a trap on EXIT — which bash honours for SIGTERM (verified) but nothing honours for SIGKILL, which is what the agent does to the hook when the user answers in its own UI. The file then outlives the process. The evidence was sitting on disk: 536 leaked FIFO directories going back three months, every single one still holding a live FIFO, so the trap had not run once, and the newest was from yesterday. So liveness is now checked alongside the file. The FIFO answers "was a prompt raised?"; the hook's pid, which now travels with the event, answers "is anyone still listening?". A prompt whose hook has gone cannot be answered from the panel — there is nothing left to read the decision — so it retires within a tick and stops being counted and reminded about. An event from an older notify.sh carries no pid, and falls back to the previous behaviour rather than having its Allow/Deny silently disabled for the first event after an upgrade. Also traps INT/TERM/HUP explicitly and sweeps leaked directories older than 30 minutes, comfortably past the 550s a prompt can live for so one in use is never in range. Neither is load-bearing now that the panel no longer trusts the trap, but 536 leaked FIFOs is its own small problem. Considered and rejected: probing the FIFO with open(O_WRONLY|O_NONBLOCK) and reading ENXIO as "no reader". It needs no new field, but the hook selects on the read end, so a probe opening and closing the write end delivers EOF, the hook exits with an empty decision, and the panel's Allow/Deny stops working — a worse bug than the one being fixed. Co-Authored-By: Claude Opus 5 --- README.md | 2 + .../AttentionPolicyTests.swift | 63 +++++++++++++++++++ notify.sh | 24 ++++++- panel/AttentionPolicy.swift | 34 ++++++++++ panel/EventListener.swift | 2 + panel/EventStore.swift | 5 ++ panel/Panel.swift | 12 +++- 7 files changed, 139 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 4736107..0198bfe 100644 --- a/README.md +++ b/README.md @@ -162,6 +162,8 @@ History is deliberately a separate pane rather than more rows in the queue. The Records live in `~/.stack-nudge/events.jsonl` (mode 0600), one JSON object per line, holding timestamp, agent, kind, title, message, repo path, and session id — the descriptive fields only, never the FIFO paths or PIDs. They're kept for 30 days or 10,000 records, whichever binds first, trimmed once per launch. **The log contains prompt and tool text**, so if that isn't something you want on disk, turn it off in Settings → **Event history** (`STACKNUDGE_EVENT_HISTORY=false`); Settings → **Clear event history** deletes the file outright. Nothing is ever sent anywhere — this is a local file, same as the rest of `~/.stack-nudge`. +**Answering elsewhere clears the prompt.** A permission prompt is tracked by two things: the FIFO notify.sh creates for your decision, and the hook process waiting to read it. Both are required. The FIFO alone isn't proof — the cleanup runs on exit, and nothing runs on `SIGKILL`, which is what the agent does to the hook when you answer in its own UI. So approving a plan in the terminal used to leave the panel convinced the prompt was still blocking, keeping it in the menu-bar count and re-nudging you (and Slack) for the full 550 seconds. Now a prompt whose hook has gone is retired within a tick, because there's nobody left to read a decision. + **Reminders for prompts you didn't answer.** macOS slides a banner into Notification Center after a few seconds, so a permission prompt you missed used to leave the agent blocked with nothing on screen to say so. stack-nudge now re-nudges: while a prompt is still waiting, it fires again on an interval (`STACKNUDGE_REMIND_MIN`, one of 1 / 2 / 5 minutes, default 2, `Off` to disable), up to three times — or fewer at longer intervals, since reminders stop once the hook hits its own 550-second timeout. The reminder carries the same **Allow** / **Deny** buttons and reads *"Still waiting 4m · Bash(rm -rf build/)"*, and the menu-bar icon shows a live count of prompts waiting on you — the one signal that survives an expired banner. Reminders only fire for prompts stack-nudge can *prove* are unanswered. Claude Code and Codex permission hooks block on a FIFO that's removed the moment the hook exits, so its presence means nobody has answered — in the panel, on the banner, or in the terminal. Gemini and Antigravity route permissions through fire-and-forget notification hooks with no such signal, so they get the single banner they always did rather than reminders that might be about something you already handled. Reminders stop at three, and stop early once the hook hits its own 550-second timeout and the agent falls back to prompting in the terminal. A per-session mute silences them like any other nudge. A *global* mute silences the banner and sound but not the Slack DM — see the Slack section for why. diff --git a/Tests/StackNudgePanelCoreTests/AttentionPolicyTests.swift b/Tests/StackNudgePanelCoreTests/AttentionPolicyTests.swift index b98ed1c..576d117 100644 --- a/Tests/StackNudgePanelCoreTests/AttentionPolicyTests.swift +++ b/Tests/StackNudgePanelCoreTests/AttentionPolicyTests.swift @@ -194,4 +194,67 @@ final class AttentionPolicyTests: XCTestCase { XCTAssertEqual(list.first, 0, "each list needs an Off entry at index 0") } } + + // MARK: - isAnswerable + + private func answerable(kind: NudgeKind = .permission, + fifo: String? = "/tmp/x/fifo", + hookPID: Int? = 4242, + fifoExists: Bool = true, + alive: Bool = true) -> Bool { + AttentionPolicy.isAnswerable(kind: kind, fifoPath: fifo, hookPID: hookPID, + fifoExists: { _ in fifoExists }, + processAlive: { _ in alive }) + } + + func test_answerable_whenTheHookIsStillWaiting() { + XCTAssertTrue(answerable()) + } + + // The bug this exists for. The FIFO's trap is on EXIT, which nothing honours + // for SIGKILL, and the agent kills the hook when the user answers in its own + // UI — so the file outlives the process. Evidence from one machine: 536 + // leaked FIFO dirs over three months, every one still holding a live FIFO. + // Treating the file alone as proof kept the prompt in the menu-bar count and + // fired reminders, at the user and at Slack, for the full 550 seconds after + // they had already answered. + func test_notAnswerable_whenTheFifoOutlivedItsHook() { + XCTAssertFalse(answerable(fifoExists: true, alive: false), + "a FIFO nobody is reading cannot carry a decision") + } + + func test_notAnswerable_onceTheFifoIsGone() { + XCTAssertFalse(answerable(fifoExists: false)) + } + + // Observability-only agents get no FIFO, and a finished turn has nothing to + // answer — neither should ever be counted or reminded about. + func test_notAnswerable_withoutAFifo() { + XCTAssertFalse(answerable(fifo: nil)) + } + + func test_notAnswerable_forNonPermissionEvents() { + XCTAssertFalse(answerable(kind: .stop)) + XCTAssertFalse(answerable(kind: .other)) + } + + // A hook from before this field existed reports no pid. Treating that as + // dead would silently disable the panel's Allow/Deny for the first event + // after an upgrade, which is worse than the leak it protects against. + func test_missingHookPIDFallsBackToTheOldBehaviour() { + XCTAssertTrue(answerable(hookPID: nil, alive: false), + "no pid reported means we cannot judge liveness, so trust the FIFO") + XCTAssertFalse(answerable(hookPID: nil, fifoExists: false)) + } + + // Liveness is only consulted when there is a FIFO to answer — otherwise a + // dead-process check would be doing work for events that can never qualify. + func test_livenessIsNotConsultedWithoutAFifo() { + var asked = false + _ = AttentionPolicy.isAnswerable(kind: .permission, fifoPath: nil, hookPID: 1, + fifoExists: { _ in true }, + processAlive: { _ in asked = true; return true }) + XCTAssertFalse(asked) + } + } diff --git a/notify.sh b/notify.sh index cb8ffbe..249d6f0 100755 --- a/notify.sh +++ b/notify.sh @@ -582,6 +582,7 @@ optional = { "window_title": env.get("NUDGE_WINDOW"), "ipc_hook": env.get("NUDGE_IPC_HOOK"), "fifo_path": env.get("NUDGE_FIFO"), + "hook_pid": env.get("NUDGE_HOOK_PID"), "agent_pid": env.get("NUDGE_AGENT_PID"), "shell_pid": env.get("NUDGE_SHELL_PID"), "terminal_pid": env.get("NUDGE_TERMINAL_PID"), @@ -718,6 +719,11 @@ notify_macos() { # in-app mute-when-focused check has the right value to compare # against the frontmost window. project_name is still derived from $PWD # via NUDGE_PROJECT inside post_to_panel. + # The hook's own pid travels with the event so the panel can tell "a prompt is + # still waiting" from "a FIFO outlived the process that was reading it". The + # EXIT trap below does not run when the agent SIGKILLs us, which it does when + # the user answers in its own UI — see AttentionPolicy.isAnswerable. + export NUDGE_HOOK_PID="$$" post_to_panel "${title}" "${message}" "${bundle_id}" "${win_title}" \ "${has_action}" "${fifo_path}" "${voice_message}" "${sound}" "${bypass_mute}" & @@ -732,7 +738,19 @@ notify_macos() { # Create a unique FIFO at /tmp for the user's response. Echoes the path. # Returns empty if mkfifo fails. +# Remove permission FIFO dirs left by hooks that were SIGKILLed before their +# trap could run. Bounded and best-effort: only our own mktemp pattern, only +# entries older than 30 minutes — comfortably past the 550s (9m10s) a prompt can +# live for, so a dir in use is never in range — and errors ignored, because a +# sweep that fails must never delay the notification it runs alongside. +sweep_stale_perm_fifos() { + local root="${TMPDIR:-/tmp}" + find "$root" -maxdepth 1 -name 'stack-nudge-perm.*' -type d -mmin +30 \ + -exec rm -rf {} + 2>/dev/null || true +} + create_perm_fifo() { + sweep_stale_perm_fifos # Place the FIFO inside a private mktemp dir (mode 0700, CSPRNG-named) rather # than a $RANDOM-suffixed /tmp path — $RANDOM is only 16-bit, so the old name # was guessable, letting a local attacker pre-create the FIFO or inject a @@ -755,7 +773,11 @@ wait_for_permission_response() { local fifo="$1" local timeout=550 # Claude Code's hook timeout defaults to 600s — leave buffer - trap 'rm -f "$fifo"; rmdir "$(dirname "$fifo")" 2>/dev/null' EXIT + # INT/TERM/HUP as well as EXIT: bash runs the EXIT trap for a plain SIGTERM, + # but naming the signals makes the intent explicit and covers the shells that + # don't. Nothing catches SIGKILL, which is why the panel no longer relies on + # this trap alone to know a prompt is over. + trap 'rm -f "$fifo"; rmdir "$(dirname "$fifo")" 2>/dev/null' EXIT INT TERM HUP local decision decision=$(NUDGE_FIFO="$fifo" NUDGE_TIMEOUT="$timeout" python3 - <<'PY' 2>/dev/null diff --git a/panel/AttentionPolicy.swift b/panel/AttentionPolicy.swift index 18206ef..eaa8f39 100644 --- a/panel/AttentionPolicy.swift +++ b/panel/AttentionPolicy.swift @@ -69,6 +69,40 @@ enum AttentionPolicy { return "\(seconds / 60)m" } + // MARK: - Is a prompt still answerable? + + // The FIFO's existence was treated as proof a prompt is still blocking, on + // the grounds that notify.sh removes it on exit. It doesn't always: the trap + // is on EXIT, which bash honours for SIGTERM but nothing honours for + // SIGKILL, and the agent kills the hook outright when the user answers in + // its own UI. Evidence from one machine: 536 leaked FIFO directories going + // back three months, every single one still holding a live FIFO, so the + // trap had not run once. + // + // The consequence is the bug this fixes. Approve a plan in the terminal and + // the hook is killed, the FIFO survives, and the panel goes on believing the + // prompt is blocking — keeping it in the menu-bar count and firing reminders + // at you, and at Slack, for the full 550 seconds. + // + // So the FIFO answers "was a prompt raised?" and the hook's liveness answers + // "is anyone still listening?". Both are required: a prompt whose hook is + // gone cannot be answered from the panel, because there is nothing left to + // read the decision. + // + // `hookPID` nil means the hook predates this field, so fall back to the old + // behaviour rather than treating every prompt from an older notify.sh as + // dead — the script self-updates, but not before the first event after an + // upgrade. + static func isAnswerable(kind: NudgeKind, + fifoPath: String?, + hookPID: Int?, + fifoExists: (String) -> Bool, + processAlive: (Int) -> Bool) -> Bool { + guard kind == .permission, let fifoPath, fifoExists(fifoPath) else { return false } + guard let hookPID else { return true } + return processAlive(hookPID) + } + // MARK: - Stalled sessions // Thresholds offered in Settings, in minutes. 0 = off. diff --git a/panel/EventListener.swift b/panel/EventListener.swift index 28d7ce0..3ee435f 100644 --- a/panel/EventListener.swift +++ b/panel/EventListener.swift @@ -167,6 +167,7 @@ private struct NudgeEventDTO: Decodable { let session_id: String? let iterm_tab_name: String? let fifo_path: String? + let hook_pid: Int? let voice_message: String? let voice_template: String? let sound_name: String? @@ -213,6 +214,7 @@ private struct NudgeEventDTO: Decodable { sessionID: session_id, itermTabName: iterm_tab_name, fifoPath: Self.validatedFifoPath(fifo_path), + hookPID: hook_pid, voiceMessage: voice_message, voiceTemplate: voice_template, soundName: sound_name, diff --git a/panel/EventStore.swift b/panel/EventStore.swift index 03b7e66..d3689db 100644 --- a/panel/EventStore.swift +++ b/panel/EventStore.swift @@ -46,6 +46,8 @@ struct NudgeEvent: Identifiable, Equatable { // "allow" or "deny" to it lets stack-nudge return a PermissionRequest // decision to Claude Code without touching the terminal UI. let fifoPath: String? + // pid of the notify.sh hook waiting on `fifoPath`; nil from older hooks. + let hookPID: Int? // Curated phrase for the voice engine (different from the visible // `message` — the banner shows the tool / file context, the voice // speaks a conversational sentence). @@ -79,6 +81,7 @@ struct NudgeEvent: Identifiable, Equatable { termProgram: String? = nil, sessionID: String? = nil, itermTabName: String? = nil, fifoPath: String? = nil, + hookPID: Int? = nil, voiceMessage: String? = nil, voiceTemplate: String? = nil, soundName: String? = nil, @@ -106,6 +109,7 @@ struct NudgeEvent: Identifiable, Equatable { self.sessionID = sessionID self.itermTabName = itermTabName self.fifoPath = fifoPath + self.hookPID = hookPID self.voiceMessage = voiceMessage self.voiceTemplate = voiceTemplate self.soundName = soundName @@ -128,6 +132,7 @@ struct NudgeEvent: Identifiable, Equatable { termProgram: termProgram, sessionID: sessionID, itermTabName: itermTabName, fifoPath: fifoPath, + hookPID: hookPID, voiceMessage: voiceMessage, voiceTemplate: voiceTemplate, soundName: soundName, diff --git a/panel/Panel.swift b/panel/Panel.swift index 5e11ab3..b280f58 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -2968,8 +2968,16 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, // reminder we can't verify would eventually fire for prompts already // handled in the terminal, and a nudge that cries wolf is worse than none. private func isBlockingPrompt(_ event: NudgeEvent) -> Bool { - guard event.kind == .permission, let fifo = event.fifoPath else { return false } - return FileManager.default.fileExists(atPath: fifo) + AttentionPolicy.isAnswerable( + kind: event.kind, + fifoPath: event.fifoPath, + hookPID: event.hookPID, + fifoExists: { FileManager.default.fileExists(atPath: $0) }, + // kill(pid, 0) asks "does this process exist and may I signal it" + // without sending anything. EPERM would mean it exists but isn't + // ours, which can't happen for a hook we spawned, so treating only + // success as alive is right. + processAlive: { kill(pid_t($0), 0) == 0 }) } // Register newly-arrived prompts and drop answered ones. Deliberately From 0adcd129bb36bffca4aeb3855e47411ad5fc7d3c Mon Sep 17 00:00:00 2001 From: Hisku Date: Fri, 11 Sep 2026 13:57:34 +0100 Subject: [PATCH 3/4] fix(notify): keep unreported turns when the user touches the machine MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adversarial review found the throttle's headline property was false end to end. resetStopThrottleIfPresent zeroed suppressedStopCount along with the cooldown, so turns that finished while the user was away and had not been reported yet were discarded rather than folded into the next message. Idle is measured from the last HID event, which makes this easy to hit without ever returning: a stray trackpad bump or a notification click reads as "present", the pending count is wiped, and the next DM claims a bare "finished a turn" as though nothing preceded it. The burst test asserted conservation over throttleStop alone and so never saw it. Only the cooldown is cleared now. Keeping the count is accurate rather than merely safe: it is incremented only past the idle gate in notifySlack, so it can only ever hold turns that genuinely finished while the user was away, however many brief presences the absence is split across. Also records why pid reuse is bounded, since it is the obvious objection to the liveness check: a watch is created with firstSeenAt taken from the event and retired in the same pass once that exceeds promptLifetime, so a reused pid cannot resurrect a stale prompt — the window is 550s from the prompt, not the 30 minute sweep interval, and inside it the worst case is the behaviour this replaced. Co-Authored-By: Claude Opus 5 --- .../SlackDeliveryTests.swift | 24 +++++++++++++++++++ panel/AttentionPolicy.swift | 9 +++++++ panel/Panel.swift | 12 +++++++++- 3 files changed, 44 insertions(+), 1 deletion(-) diff --git a/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift b/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift index a1f7d02..87112a5 100644 --- a/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift +++ b/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift @@ -176,6 +176,30 @@ final class SlackDeliveryTests: XCTestCase { "every finished turn must be accounted for somewhere") } + // Returning to the machine clears the cooldown but must NOT discard turns + // that are still waiting to be reported. Idle is measured from the last HID + // event, so a stray trackpad bump reads as "present" — and an earlier cut + // zeroed the pending count there, so everything missed vanished and the next + // DM read as a bare "finished a turn" with nothing preceding it. + func test_pendingCountSurvivesAReturnToTheMachine() { + // Away: one sent, then three swallowed inside the cooldown. + var lastSent: Date? = t0 + var suppressed = 3 + + // The user brushes the trackpad; the controller clears only the cooldown. + lastSent = nil + + // They leave again and another turn finishes: it sends at once, and + // reports the three that were waiting. + XCTAssertEqual(SlackDelivery.throttleStop(now: t0.addingTimeInterval(60), + lastSentAt: lastSent, + suppressed: suppressed), + .send(coalesced: 3), + "turns missed while away must not be dropped on a stray keystroke") + suppressed = 0 + XCTAssertEqual(suppressed, 0) + } + // The first stop of an absence is the one worth having promptly. func test_firstStopSendsImmediately() { XCTAssertEqual(SlackDelivery.throttleStop(now: t0, lastSentAt: nil, suppressed: 0), diff --git a/panel/AttentionPolicy.swift b/panel/AttentionPolicy.swift index eaa8f39..3bffcbd 100644 --- a/panel/AttentionPolicy.swift +++ b/panel/AttentionPolicy.swift @@ -89,6 +89,15 @@ enum AttentionPolicy { // gone cannot be answered from the panel, because there is nothing left to // read the decision. // + // Pid reuse is the obvious objection: a SIGKILLed hook's pid can be recycled, + // and a recycled pid answers kill(0). It is bounded and benign. A watch is + // built with `firstSeenAt: event.timestamp` and retired in the same pass once + // that exceeds promptLifetime, so a stale event cannot be resurrected by a + // reused pid even if one appears — the window is 550s from the prompt, not + // the sweep interval. And inside that window the worst case is simply the + // behaviour this fix replaced: a prompt counted slightly too long. Never + // worse than the bug, and usually much better. + // // `hookPID` nil means the hook predates this field, so fall back to the old // behaviour rather than treating every prompt from an older notify.sh as // dead — the script self-updates, but not before the first event after an diff --git a/panel/Panel.swift b/panel/Panel.swift index b280f58..041c560 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -2954,8 +2954,18 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, guard nav.slackIdleMinutes > 0, IdleTime.seconds() < TimeInterval(nav.slackIdleMinutes * 60) else { return } + // Only the cooldown is cleared. `suppressedStopCount` is deliberately + // kept: it holds turns that finished while the user was away and have + // not been reported yet, and zeroing it here threw them away. Idle is + // measured from the last HID event, so a stray trackpad bump — or a + // notification click — counts as "present" and would silently discard + // the record of everything missed, leaving the next DM claiming a bare + // "finished a turn" as though nothing had preceded it. + // + // Carrying it is accurate rather than merely safe: the counter is only + // ever incremented past the idle gate in notifySlack, so it can only + // contain turns that genuinely finished while the user was away. lastStopSlackAt = nil - suppressedStopCount = 0 } // A permission prompt we can *prove* is still blocking: notify.sh creates From 1bc3f475ace41389a8fa3931aefa16244f84be89 Mon Sep 17 00:00:00 2001 From: Hisku Date: Fri, 11 Sep 2026 18:47:43 +0100 Subject: [PATCH 4/4] =?UTF-8?q?fix(notify):=20address=20review=20=E2=80=94?= =?UTF-8?q?=20sweep=20off=20the=20hot=20path,=20and=20a=20presence=20that?= =?UTF-8?q?=20means=20it?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Nine review threads. The two blockers: The stale-FIFO sweep ran before post_to_panel, so it sat on the path to the banner. Measured on a large $TMPDIR: 2.2s cold, i.e. two seconds before the user learns an agent is blocked, on the one path where latency is the product. It is backgrounded now — the dir just created is seconds old so can never be in range of -mmin +30, and the hook lives 550s, so the sweep has all the time it needs. Keeping the suppressed count on any presence traded one bug for another: a count from yesterday's absence could be reported on tomorrow's first DM. The discriminator is how long the presence lasted. A single stray HID event keeps idle under the threshold for exactly the threshold and not a second more, so a presence that outlasts it cannot be one event — it takes two, far enough apart, which is a person. A brief presence now resets only the cooldown; a sustained one clears what was missed, because the panel has had it on screen throughout. Also from review: The hook pid was unvalidated while fifo_path on the same socket was not. Verified against the real syscall: kill(0, 0) signals the caller's process group and kill(-1, 0) every process it may signal, both returning 0, so either value made every prompt read as alive and undid the fix. Worse and unflagged — pid_t is Int32 and pid_t(4_000_000_000) traps, so an oversized number in the payload crashed the panel. Both rejected now, with the conversion made non-trapping as well rather than left one guard away. A dead prompt kept offering Allow/Deny, where writeFIFO gets ENXIO and returns silently — a button that does nothing. Now the two are distinguishable, the affordance goes with the liveness. The check moved to NudgeEvent.isStillBlocking so the view layer and the controller ask it the same way. Two comments asserted the belief this PR disproves ("the file existing means nobody has answered yet", "retire on the FIFO alone"). Corrected. The zombie window now says why reaping is load-bearing rather than incidental. Dropped a test assertion that checked a local assigned on the line above, which could not fail. Fair hit, on a PR arguing for conservation over counting. README: Always removes the idle gate, not the rate limit; and the limit is global, so the DM means "go and look" rather than "look here specifically". Co-Authored-By: Claude Opus 5 --- README.md | 4 +- .../EventListenerTests.swift | 29 +++++++ .../SlackDeliveryTests.swift | 45 ++++++++++- notify.sh | 7 +- panel/AttentionPolicy.swift | 44 +++++++++++ panel/EventListener.swift | 2 +- panel/Panel.swift | 77 ++++++++++--------- panel/SlackNotifier.swift | 31 ++++++++ 8 files changed, 195 insertions(+), 44 deletions(-) diff --git a/README.md b/README.md index 0198bfe..bd1e884 100644 --- a/README.md +++ b/README.md @@ -186,8 +186,8 @@ The token lives in the **Keychain**, never `~/.stack-nudge/config`. For scripted Three things worth knowing about how it behaves: -- **It's idle-gated, not a mirror.** By default nothing reaches Slack until the Mac has been untouched for 5 minutes — there's no point pinging your phone about a prompt you're already looking at. Set it to `Always` if you'd rather have everything. Reminders for prompts you never answered skip the gate: you may be back at the desk and still not have seen the banner. -- **Finished turns are rate-limited; permission prompts aren't.** The idle setting decides *whether* you're away, not how often you're interrupted while you are — so on its own it would send a DM per event, and an unattended hour can produce fifty. Finished turns are therefore folded into one message per 15 minutes, which says what it swallowed (*"finished a turn · 7 more while you were away"*). Permission prompts are never batched: each one is blocking something until you answer it, and repeats of a single prompt are already capped at three. +- **It's idle-gated, not a mirror.** By default nothing reaches Slack until the Mac has been untouched for 5 minutes — there's no point pinging your phone about a prompt you're already looking at. Set it to `Always` if you'd rather have everything — that removes the *idle* gate, not the rate limit below. Reminders for prompts you never answered skip the gate: you may be back at the desk and still not have seen the banner. +- **Finished turns are rate-limited; permission prompts aren't.** The limit is global rather than per session, so with several agents running the headline names one of them and the rest are folded into the count — the DM means "go and look", not "look here specifically". The idle setting decides *whether* you're away, not how often you're interrupted while you are — so on its own it would send a DM per event, and an unattended hour can produce fifty. Finished turns are therefore folded into one message per 15 minutes, which says what it swallowed (*"finished a turn · 7 more while you were away"*). Permission prompts are never batched: each one is blocking something until you answer it, and repeats of a single prompt are already capped at three. Coming back to the machine clears the wait — but only a stay longer than the idle threshold does, since a single stray keystroke is indistinguishable from a trackpad bump. - **A global mute does *not* silence Slack.** Mute means "stop interrupting me *here*", and Slack exists precisely because you're elsewhere. Use the Slack switch to stop it. A *per-session* mute does apply, same as it does to banners. - **Titles only, by default.** A DM reads *"Claude Code in attack-lib needs permission"*. The tool call itself (`Bash(rm -rf build/)`) stays on your machine unless you turn on **Include message text**, because command lines carry paths, hostnames, and sometimes secrets, and this is the one path that leaves the Mac. diff --git a/Tests/StackNudgePanelCoreTests/EventListenerTests.swift b/Tests/StackNudgePanelCoreTests/EventListenerTests.swift index a88d49b..9dd07bb 100644 --- a/Tests/StackNudgePanelCoreTests/EventListenerTests.swift +++ b/Tests/StackNudgePanelCoreTests/EventListenerTests.swift @@ -115,4 +115,33 @@ final class EventListenerTests: XCTestCase { XCTAssertEqual(EventListener.parseEvents(Data()).count, 0) XCTAssertEqual(EventListener.parseEvents(payload("\n\n\n")).count, 0) } + + // MARK: - hookPID validation + + // The pid decides whether a prompt still counts as blocking, and it arrives + // on the same local socket as fifo_path, which is already validated. + // + // Both rejected values were verified against the real syscall: kill(0, 0) + // signals the caller's whole process group and kill(-1, 0) every process it + // may signal, and both return 0 — so either would make every prompt read as + // alive and quietly undo the fix. + func test_hookPID_rejectsGroupAndBroadcastTargets() { + XCTAssertNil(AttentionPolicy.validHookPID(0)) + XCTAssertNil(AttentionPolicy.validHookPID(-1)) + XCTAssertNil(AttentionPolicy.validHookPID(-4242)) + } + + // pid_t is Int32, and pid_t(4_000_000_000) traps rather than wrapping, so an + // oversized number in the payload would crash the panel outright. + func test_hookPID_rejectsAnythingPidTCannotHold() { + XCTAssertNil(AttentionPolicy.validHookPID(Int(pid_t.max) + 1)) + XCTAssertNil(AttentionPolicy.validHookPID(4_000_000_000)) + XCTAssertEqual(AttentionPolicy.validHookPID(Int(pid_t.max)), Int(pid_t.max)) + } + + func test_hookPID_acceptsARealPidAndPassesNilThrough() { + XCTAssertEqual(AttentionPolicy.validHookPID(4242), 4242) + XCTAssertNil(AttentionPolicy.validHookPID(nil)) + } + } diff --git a/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift b/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift index 87112a5..673faa9 100644 --- a/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift +++ b/Tests/StackNudgePanelCoreTests/SlackDeliveryTests.swift @@ -183,8 +183,8 @@ final class SlackDeliveryTests: XCTestCase { // DM read as a bare "finished a turn" with nothing preceding it. func test_pendingCountSurvivesAReturnToTheMachine() { // Away: one sent, then three swallowed inside the cooldown. + let suppressed = 3 var lastSent: Date? = t0 - var suppressed = 3 // The user brushes the trackpad; the controller clears only the cooldown. lastSent = nil @@ -196,8 +196,6 @@ final class SlackDeliveryTests: XCTestCase { suppressed: suppressed), .send(coalesced: 3), "turns missed while away must not be dropped on a stray keystroke") - suppressed = 0 - XCTAssertEqual(suppressed, 0) } // The first stop of an absence is the one worth having promptly. @@ -249,4 +247,45 @@ final class SlackDeliveryTests: XCTestCase { } } + + // MARK: - Presence + + private func effect(idle: TimeInterval, threshold: Int = 10, + presentFor: TimeInterval = 0) -> SlackDelivery.PresenceEffect { + SlackDelivery.presenceEffect(idleSeconds: idle, idleThresholdMinutes: threshold, + presentFor: presentFor) + } + + func test_presence_awayWhileIdlePastTheThreshold() { + XCTAssertEqual(effect(idle: 11 * 60), .away) + } + + // The discriminator. A single stray HID event keeps idle under the threshold + // for exactly the threshold and not a second longer, so it can never reach + // `sustainedPresent` — which is what stops a trackpad bump discarding the + // record of everything missed. + func test_presence_oneStrayEventCanNeverLookSustained() { + let threshold = 10.0 * 60 + // Walk the whole life of a single bump: idle climbs from 0, and the + // time "present" climbs with it, so the two are always equal. + for elapsed in stride(from: 0.0, to: threshold, by: 30) { + XCTAssertEqual(effect(idle: elapsed, presentFor: elapsed), .brieflyPresent, + "at \(elapsed)s a lone event must stay 'brief'") + } + // Past the threshold the bump stops counting as present at all. + XCTAssertEqual(effect(idle: threshold, presentFor: threshold), .away) + } + + // Two events far enough apart is a person: idle resets while the presence + // keeps running, so presentFor outgrows idle. + func test_presence_sustainedNeedsMoreThanOneEvent() { + XCTAssertEqual(effect(idle: 60, presentFor: 11 * 60), .sustainedPresent) + } + + // "Always" removes the idle gate, so there is no presence to detect and the + // cooldown runs continuously. + func test_presence_alwaysHasNoPresenceToDetect() { + XCTAssertEqual(effect(idle: 0, threshold: 0, presentFor: 9999), .away) + } + } diff --git a/notify.sh b/notify.sh index 249d6f0..89fbd32 100755 --- a/notify.sh +++ b/notify.sh @@ -750,7 +750,12 @@ sweep_stale_perm_fifos() { } create_perm_fifo() { - sweep_stale_perm_fifos + # Backgrounded: this sits on the path to the banner, and a cold stat of a + # large $TMPDIR was measured at 2.2s — two seconds before the user learns an + # agent is blocked, on the one path where latency is the product. The dir + # created just below is seconds old so it can never fall in range of -mmin + # +30, and the hook lives for 550s, so the sweep has all the time it needs. + sweep_stale_perm_fifos & # Place the FIFO inside a private mktemp dir (mode 0700, CSPRNG-named) rather # than a $RANDOM-suffixed /tmp path — $RANDOM is only 16-bit, so the old name # was guessable, letting a local attacker pre-create the FIFO or inject a diff --git a/panel/AttentionPolicy.swift b/panel/AttentionPolicy.swift index 3bffcbd..4e86caf 100644 --- a/panel/AttentionPolicy.swift +++ b/panel/AttentionPolicy.swift @@ -89,6 +89,14 @@ enum AttentionPolicy { // gone cannot be answered from the panel, because there is nothing left to // read the decision. // + // A zombie — exited but not yet reaped — still answers kill(0), so there is a + // window where a dead hook reads as alive. It closes when the agent reaps + // its child, which is prompt in practice, and the next 5s tick corrects it. + // That reaping is load-bearing rather than incidental: a SIGKILLed child + // still answering kill(0) is precisely the case this exists to catch, so if + // an agent ever left hooks unreaped the check would degrade to the old + // FIFO-only behaviour — never worse, but no longer a fix. + // // Pid reuse is the obvious objection: a SIGKILLed hook's pid can be recycled, // and a recycled pid answers kill(0). It is bounded and benign. A watch is // built with `firstSeenAt: event.timestamp` and retired in the same pass once @@ -102,6 +110,20 @@ enum AttentionPolicy { // behaviour rather than treating every prompt from an older notify.sh as // dead — the script self-updates, but not before the first event after an // upgrade. + // A pid arrives on the same local socket as fifo_path, which is validated, so + // this is validated too rather than hardening one half of a pair. + // + // Both rejections were checked against the real syscall. kill(0, 0) signals + // the caller's whole process group and kill(-1, 0) every process it may + // signal, and both return 0 — so a pid of 0 or a negative would make every + // prompt read as alive and quietly undo the liveness check. And pid_t is + // Int32, so pid_t(4_000_000_000) traps rather than wrapping: an oversized + // number in the payload would crash the panel outright. + static func validHookPID(_ raw: Int?) -> Int? { + guard let raw, raw > 0, raw <= Int(pid_t.max) else { return nil } + return raw + } + static func isAnswerable(kind: NudgeKind, fifoPath: String?, hookPID: Int?, @@ -144,3 +166,25 @@ enum AttentionPolicy { return minutes % 60 == 0 ? "\(minutes / 60)h" : "\(minutes)m" } } + +extension NudgeEvent { + // "Is this prompt still waiting on me?", asked against the live system. + // Lives here rather than on PanelController because the view layer asks it + // too — a dead prompt must not keep offering Allow/Deny, since writeFIFO + // would get ENXIO and the button would silently do nothing. + var isStillBlocking: Bool { + AttentionPolicy.isAnswerable( + kind: kind, + fifoPath: fifoPath, + hookPID: hookPID, + fifoExists: { FileManager.default.fileExists(atPath: $0) }, + // kill(pid, 0) asks "does this exist and may I signal it" without + // sending anything. `exactly:` rather than pid_t(_:) because that + // traps above Int32.max — the listener rejects such a pid, but a + // crash is not something to leave one guard away. + processAlive: { pid in + guard let pid = pid_t(exactly: pid) else { return false } + return kill(pid, 0) == 0 + }) + } +} diff --git a/panel/EventListener.swift b/panel/EventListener.swift index 3ee435f..2606c27 100644 --- a/panel/EventListener.swift +++ b/panel/EventListener.swift @@ -214,7 +214,7 @@ private struct NudgeEventDTO: Decodable { sessionID: session_id, itermTabName: iterm_tab_name, fifoPath: Self.validatedFifoPath(fifo_path), - hookPID: hook_pid, + hookPID: AttentionPolicy.validHookPID(hook_pid), voiceMessage: voice_message, voiceTemplate: voice_template, soundName: sound_name, diff --git a/panel/Panel.swift b/panel/Panel.swift index 041c560..38f719e 100644 --- a/panel/Panel.swift +++ b/panel/Panel.swift @@ -577,6 +577,7 @@ struct PanelContentView: View { private var snoozeEnabled: Bool { guard let selected = store.selectedEvent else { return false } return selected.kind == .permission && selected.hasActionButton + && selected.isStillBlocking } private var primaryActionLabel: String? { @@ -588,7 +589,11 @@ struct PanelContentView: View { // event it's a plain dismiss. Label it to match so the gesture isn't a surprise. private var dismissLabel: String { guard let event = store.selectedEvent, - event.kind == .permission, event.hasActionButton else { return "Dismiss" } + event.kind == .permission, event.hasActionButton, + // A dead hook can't read a decision, so offering one would be a + // button that silently does nothing: writeFIFO gets ENXIO and + // returns. Now that the two are distinguishable, say so. + event.isStillBlocking else { return "Dismiss" } return "Deny" } } @@ -2920,6 +2925,9 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, // running from the previous one. private var lastStopSlackAt: Date? private var suppressedStopCount = 0 + // When the current run of "user is at the machine" began — see + // SlackDelivery.presenceEffect for why the duration is what matters. + private var presentSince: Date? // 5s so the menu-bar count clears promptly after an approval. Nothing // explicitly deregisters a watch — see reconcilePromptWatches — so the tick @@ -2941,7 +2949,7 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, } remindOnOldestDuePrompt(now: now) refreshStalledSessions(now: now) - resetStopThrottleIfPresent() + applyPresenceToStopThrottle(now: now) if nav.slackTokenPresent { refreshSlackStatus() } } @@ -2950,45 +2958,39 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, // being swallowed by a window left over from the previous absence. With the // idle gate set to Always there is no "present" to detect, so the cooldown // runs continuously — which is what that setting asks for. - private func resetStopThrottleIfPresent() { - guard nav.slackIdleMinutes > 0, - IdleTime.seconds() < TimeInterval(nav.slackIdleMinutes * 60) - else { return } - // Only the cooldown is cleared. `suppressedStopCount` is deliberately - // kept: it holds turns that finished while the user was away and have - // not been reported yet, and zeroing it here threw them away. Idle is - // measured from the last HID event, so a stray trackpad bump — or a - // notification click — counts as "present" and would silently discard - // the record of everything missed, leaving the next DM claiming a bare - // "finished a turn" as though nothing had preceded it. - // - // Carrying it is accurate rather than merely safe: the counter is only - // ever incremented past the idle gate in notifySlack, so it can only - // contain turns that genuinely finished while the user was away. - lastStopSlackAt = nil - } - - // A permission prompt we can *prove* is still blocking: notify.sh creates - // the FIFO before emitting the event and removes it on exit (the EXIT trap - // in wait_for_permission_response), so the file existing means nobody has - // answered yet — in the panel, on the banner, or in the terminal. + private func applyPresenceToStopThrottle(now: Date) { + let presentFor = presentSince.map { now.timeIntervalSince($0) } ?? 0 + switch SlackDelivery.presenceEffect(idleSeconds: IdleTime.seconds(), + idleThresholdMinutes: nav.slackIdleMinutes, + presentFor: presentFor) { + case .away: + presentSince = nil + case .brieflyPresent: + // Could be one stray HID event, so the cooldown restarts but + // anything not yet reported is kept for the next message. + if presentSince == nil { presentSince = now } + lastStopSlackAt = nil + case .sustainedPresent: + // Long enough to have been a person, so what they missed is no + // longer news — the panel has had it in front of them all along. + lastStopSlackAt = nil + suppressedStopCount = 0 + } + } + + // A permission prompt we can *prove* is still blocking. + // + // The file existing is necessary but not sufficient, which is what this used + // to claim. notify.sh removes the FIFO from a trap on EXIT, and nothing runs + // that for SIGKILL — which is what the agent does to the hook when the user + // answers in its own UI — so the file routinely outlives the process. The + // hook's liveness is the other half: see AttentionPolicy.isAnswerable. // // Observability-only agents (Gemini, Antigravity) get no FIFO because their // hooks can't consume a decision, so they're never reminded about. A // reminder we can't verify would eventually fire for prompts already // handled in the terminal, and a nudge that cries wolf is worse than none. - private func isBlockingPrompt(_ event: NudgeEvent) -> Bool { - AttentionPolicy.isAnswerable( - kind: event.kind, - fifoPath: event.fifoPath, - hookPID: event.hookPID, - fifoExists: { FileManager.default.fileExists(atPath: $0) }, - // kill(pid, 0) asks "does this process exist and may I signal it" - // without sending anything. EPERM would mean it exists but isn't - // ours, which can't happen for a hook we spawned, so treating only - // success as alive is right. - processAlive: { kill(pid_t($0), 0) == 0 }) - } + private func isBlockingPrompt(_ event: NudgeEvent) -> Bool { event.isStillBlocking } // Register newly-arrived prompts and drop answered ones. Deliberately // reconciled from observable state rather than hooked into each resolution @@ -3003,7 +3005,8 @@ final class PanelController: NSObject, NSApplicationDelegate, PanelKeyDelegate, remindersSent: 0) } } - // Retire on the FIFO alone, never on store membership — see PromptWatch. + // Retire on the FIFO *and* its hook's liveness, never on store + // membership — see PromptWatch. // The age cap is the backstop for a hook killed hard enough to skip its // cleanup trap: past its own timeout the prompt can't be answered from // the panel anyway, so a leaked FIFO must not pin the count on forever. diff --git a/panel/SlackNotifier.swift b/panel/SlackNotifier.swift index c111739..7eb7e5b 100644 --- a/panel/SlackNotifier.swift +++ b/panel/SlackNotifier.swift @@ -93,6 +93,37 @@ enum SlackDelivery { return .send(coalesced: suppressed) } + // What being at the machine should do to the finished-turn throttle. + // + // Clearing the backlog on any presence was wrong in both directions. Idle is + // measured from the last HID event, so a single trackpad bump reads as + // "present" and threw away the record of everything missed; but keeping the + // backlog forever meant a count from yesterday's absence could be reported + // on tomorrow's first DM. + // + // The discriminator is how *long* the presence has lasted. One stray event + // keeps idle under the threshold for exactly the threshold and not a second + // more, so a presence that has outlasted it cannot be a single event — it + // takes two, far enough apart, which is a person. Only that clears what was + // missed; a brief one just restarts the cooldown so the next absence opens + // with a prompt message. + enum PresenceEffect: Equatable { + case away // leave the throttle alone + case brieflyPresent // reset the cooldown, keep what wasn't reported + case sustainedPresent // they have actually been here: clear both + } + + static func presenceEffect(idleSeconds: TimeInterval, + idleThresholdMinutes: Int, + presentFor: TimeInterval) -> PresenceEffect { + // "Always" removes the idle gate, so there is no "present" to detect and + // the cooldown simply runs continuously. + guard idleThresholdMinutes > 0 else { return .away } + let threshold = TimeInterval(idleThresholdMinutes * 60) + guard idleSeconds < threshold else { return .away } + return presentFor >= threshold ? .sustainedPresent : .brieflyPresent + } + // `label` is the resolved session name; falls back to the repo the event came // from. Detail is opt-in because a permission message is raw tool text — // "Bash(rm -rf build/)" — which can carry paths, hostnames, and secrets in