From 94b763ece14b49a99fa73a521d0bb78fa5fad5a3 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 3 Nov 2026 00:31:27 +0000 Subject: [PATCH] Re-sync KITT db6e309: one ask rings once, not on every later hand-back Brings KITT #279: the needs-you alert remembers the last ask it rang per session, so a session working past an unanswered ask no longer re-rings it on every hand-back; the open-ask line rings nothing. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01UMMrrs6msaFav9KMGMkyyd --- .agents/core/claude-core.md | 27 +++++--- .claude/skills/pr-watch/SKILL.md | 2 +- scripts/__tests__/needs-you-alert.test.mjs | 68 ++++++++++++++++--- scripts/needs-you-alert.mjs | 79 ++++++++++++++++++++-- 4 files changed, 148 insertions(+), 28 deletions(-) diff --git a/.agents/core/claude-core.md b/.agents/core/claude-core.md index 91059a3..5673241 100644 --- a/.agents/core/claude-core.md +++ b/.agents/core/claude-core.md @@ -139,15 +139,17 @@ where I put it and treat it as binding. fires `PushNotification` first, then writes the message once, as the turn's last text** โ€” no threshold, no "he probably saw it", and "he's clearly active" is not a reason to skip: the tool dedupes, my judgement - doesn't. **The banner and the open-ask line are also a trigger for the - alert that does not depend on me** (David, 2026-10-30): a `Stop` hook reads - the turn's last message and sends David a Pushover alert, once + doesn't. **The banner is also a trigger for the alert that does not + depend on me** (David, 2026-10-30): a `Stop` hook reads the turn's last + message and sends David a Pushover alert, once (`scripts/needs-you-alert.mjs`), because 4 of 12 sampled asks had no `PushNotification` call and the tool reports only - *"Mobile push requested."*, never delivery. So both shapes are written - exactly as above, `๐Ÿ›‘ **NEED YOU** โ€”` and `Still waiting on you:`, each - at the start of its own line (a heading mark or bullet before it is fine); a paraphrase of either alerts nobody, and a - mid-line quotation of either is not an ask. Text between + *"Mobile push requested."*, never delivery; the open-ask line is read by + the same hook as the mark of an ask already rung, and so rings nothing + (below). So both shapes are written exactly as above, `๐Ÿ›‘ **NEED YOU** โ€”` + and `Still waiting on you:`, each at the start of its own line (a + heading mark or bullet before it is fine); a paraphrased banner alerts + nobody, and a mid-line quotation of either is not an ask. Text between tool calls reaches him paraphrased ([`chat-text-between-tool-calls-is-summarised.md`](../../.agents/memory/chat-text-between-tool-calls-is-summarised.md)). **His phone rings only when I am waiting on him, and each alert rings @@ -168,7 +170,13 @@ where I put it and treat it as binding. a `running โ€”` or `after โ€”` hand-back, not a build report, not a ๐Ÿ‘€ FYI, not a footer in none of the four forms, not text with no footer. Every alert goes at Pushover's high priority, through his quiet hours, - and never repeats; Pushover's history is where he catches up. So the + and never repeats; Pushover's history is where he catches up. **One ask + rings once** (David, 2026-11-03, after a session working past an + unanswered ask re-rang it on every hand-back): the `Still waiting on + you:` line rings nothing, and a footer rings only when what it waits on + differs from the last thing that rang in that session โ€” the hook keeps + that one key per session and forgets it at a hand-back that waits on + nothing. So the footer's fields and the `Next:` forms are written exactly as *Waiting, and scheduled check-ins* item 0 fixes them โ€” the hook, not only he, reads them โ€” and a hand-back that waits on him dressed as `running` or @@ -181,7 +189,8 @@ where I put it and treat it as binding. too, and repeated an idle one for up to three hours until he acknowledged it). **An ask is shown once:** the banner alone carries the options and recommendation; a later turn leaving it unchanged - re-notifies and, if it has other text, ends with `Still waiting on you: + fires `PushNotification` again (the tool dedupes; the hook rings nothing + for it) and, if it has other text, ends with `Still waiting on you: , above.`, else writes nothing; a changed ask replaces the banner. **`/status` is the one exception to "shown once"**: it reproduces every open ask in full โ€” banner, context, options and diff --git a/.claude/skills/pr-watch/SKILL.md b/.claude/skills/pr-watch/SKILL.md index f527818..ed84f3c 100644 --- a/.claude/skills/pr-watch/SKILL.md +++ b/.claude/skills/pr-watch/SKILL.md @@ -322,7 +322,7 @@ requires it. decision (`agent-working-rules.md`, *A reversible choice proceeds on the recommendation*) โ€” and stay open until he answers โ€” a later clean round never clears them, and **neither does a default of mine.** A question of his that goes unanswered - stays **open** โ€” re-notified and pointed back to, reprinted only when + stays **open** โ€” `PushNotification` re-fired (the alert hook rings it only once) and pointed back to, reprinted only when the ask itself changes (`claude-core.md` interaction preference 6) โ€” not resolved by whatever I pre-registered as the fallback: on #120 round 5 a question about his own ruling was closed by my default diff --git a/scripts/__tests__/needs-you-alert.test.mjs b/scripts/__tests__/needs-you-alert.test.mjs index e7ed06c..5b61241 100644 --- a/scripts/__tests__/needs-you-alert.test.mjs +++ b/scripts/__tests__/needs-you-alert.test.mjs @@ -2,14 +2,18 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; +import { mkdtempSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; -import { decide, send, pushoverBody, sessionUrl, plain, footer, nextForm, needsSomething, WAITING, withoutFences } from "../needs-you-alert.mjs"; +import { decide, alertFor, send, pushoverBody, sessionUrl, plain, footer, nextForm, needsSomething, WAITING, withoutFences } from "../needs-you-alert.mjs"; const SCRIPT = join(dirname(fileURLToPath(import.meta.url)), "..", "needs-you-alert.mjs"); const env = { CLAUDE_PROJECT_DIR: "/home/user/Overhype.me" }; -const stop = (last_assistant_message) => decide({ hook_event_name: "Stop", last_assistant_message }, env); +// What a single Stop owes on a fresh session: the deduplicated alert, never the internal `clear` marker. +const stop = (last_assistant_message) => + alertFor({ hook_event_name: "Stop", session_id: "t", last_assistant_message }, { ...env, NEEDS_YOU_ALERT_STATE_DIR: mkdtempSync(join(tmpdir(), "nya-")) }); const FOOTER_RUNNING = "**Done:** wrote the hook. ยท **Next:** running โ€” the PR's CI, woken by its webhook. ยท **Needs you:** nothing. ยท **Spend:** 2 min, 10,000 tokens, $0.15."; const FOOTER_STOPPED = "**Done:** the plan. ยท **Next:** stopped โ€” the UAT is yours; a message from David brings me back. ยท **Needs you:** nothing. ยท **Spend:** $1."; @@ -40,12 +44,56 @@ test("a turn ending on the blocking-ask banner alerts, carrying the ask", () => assert.equal(alert.message, "pick the alert channel"); }); -test("an unchanged ask's open-ask line alerts", () => { - assert.equal(stop("Still waiting on you: the Astra code, above.").message, "the Astra code, above."); +test("an open-ask line rings nothing: it marks an ask already rung and unchanged, whatever the footer says", () => { + assert.equal(stop("Still waiting on you: the Astra code, above."), null); + assert.equal(stop(`${FOOTER_RUNNING}\n\nStill waiting on you: the code, above.`), null); + assert.equal(stop("**Done:** x ยท **Next:** after Fable โ€” fix ยท **Needs you:** pick the fonts ยท **Spend:** $1\n\nStill waiting on you: approve the fonts, above."), null); }); -test("the ask wins over the footer: a turn ending on an ask rings as an ask", () => { - assert.equal(stop(`${FOOTER_RUNNING}\n\nStill waiting on you: the code, above.`).title, "๐Ÿ›‘ Overhype.me: Claude needs you"); +test("the banner wins over the footer: a turn ending on a new ask rings as an ask", () => { + assert.equal(stop(`${FOOTER_RUNNING}\n\n๐Ÿ›‘ **NEED YOU** โ€” the code`).title, "๐Ÿ›‘ Overhype.me: Claude needs you"); +}); + +test("one ask, one ring: an unchanged Needs-you on later hand-backs rings nothing, a changed one rings, and a hand-back waiting on nothing resets it (David, 2026-11-03)", () => { + const e = { ...env, NEEDS_YOU_ALERT_STATE_DIR: mkdtempSync(join(tmpdir(), "nya-")) }; + const at = (text, session_id = "s1") => alertFor({ hook_event_name: "Stop", session_id, last_assistant_message: text }, e); + const fonts = (done) => `**Done:** ${done} ยท **Next:** after Fable's assessment โ€” the fix ยท **Needs you:** pick the caption fonts ยท **Spend:** $1`; + assert.ok(at(fonts("round 1")), "the first hand-back naming the ask rings"); + assert.equal(at(fonts("round 2")), null, "the same ask on the next hand-back does not"); + assert.equal(at(fonts("round 3")), null, "nor on the one after"); + assert.ok(at(fonts("round 1"), "s2"), "another session keeps its own memory"); + assert.ok(at("**Done:** x ยท **Next:** running โ€” CI ยท **Needs you:** the sign-in code ยท **Spend:** $1"), "a different ask rings"); + assert.equal(at(FOOTER_RUNNING), null, "a hand-back waiting on nothing rings nothing"); + assert.ok(at(fonts("round 4")), "and after it, the old ask rings again as a new one"); + assert.ok(at(FOOTER_STOPPED)); + assert.equal(at(FOOTER_STOPPED), null, "an identical stopped footer does not ring twice"); + assert.ok(at("๐Ÿ›‘ **NEED YOU** โ€” sign in")); + assert.equal(at("๐Ÿ›‘ **NEED YOU** โ€” sign in"), null, "nor an identical banner"); + assert.ok(alertFor({ hook_event_name: "Notification", session_id: "s1", notification_type: "permission_prompt" }, e)); + assert.ok(alertFor({ hook_event_name: "Notification", session_id: "s1", notification_type: "permission_prompt" }, e), "prompts are not deduplicated"); +}); + +test("two finished tasks in a row each ring, though both end on the same Next; long asks differing only at the end stay distinct (Codex, PR #279 round 1)", () => { + const e = { ...env, NEEDS_YOU_ALERT_STATE_DIR: mkdtempSync(join(tmpdir(), "nya-")) }; + const at = (text) => alertFor({ hook_event_name: "Stop", session_id: "s", last_assistant_message: text }, e); + const done = (what) => `**Done:** ${what} ยท **Next:** nothing โ€” the plan is done ยท **Needs you:** nothing ยท **Spend:** $1`; + assert.ok(at(done("task one shipped"))); + assert.ok(at(done("task two shipped")), "a second completion with its own Done rings"); + const long = "review the release notes carefully, ".repeat(10); + assert.ok(at("๐Ÿ›‘ **NEED YOU** โ€” approve the production deployment\n\nRelease 1.4.0.\n\n1. Ship\n2. Hold")); + assert.ok(at("๐Ÿ›‘ **NEED YOU** โ€” approve the production deployment\n\nRelease 1.5.0.\n\n1. Ship\n2. Hold"), "a changed decision under the same banner heading rings"); + assert.ok(at(`**Done:** x ยท **Next:** running โ€” y ยท **Needs you:** ${long}for release A ยท **Spend:** $1`)); + assert.ok(at(`**Done:** x ยท **Next:** running โ€” y ยท **Needs you:** ${long}for release B ยท **Spend:** $1`), "a change past 240 characters rings"); +}); + +test("a state file that cannot be written still rings: a duplicate is cheaper than a missed ask", () => { + // A directory path running through a regular file: every read and write fails at once (ENOTDIR). + const file = join(mkdtempSync(join(tmpdir(), "nya-")), "not-a-dir"); + writeFileSync(file, ""); + const e = { ...env, NEEDS_YOU_ALERT_STATE_DIR: join(file, "state") }; + const at = () => alertFor({ hook_event_name: "Stop", session_id: "x", last_assistant_message: FOOTER_STOPPED }, e); + assert.ok(at()); + assert.ok(at()); }); test("a message that only quotes the ask shapes is not an ask", () => { @@ -74,8 +122,6 @@ test("an ask dressed in a heading, bullet or emphasis still rings", () => { for (const text of [ "## ๐Ÿ›‘ **NEED YOU** โ€” pick a channel", "- ๐Ÿ›‘ **NEED YOU** โ€” pick a channel", - "1. Still waiting on you: pick a channel", - "**Still waiting on you:** pick a channel", ]) { assert.match(stop(text)?.message ?? "", /pick a channel/, text); } @@ -158,7 +204,7 @@ test("a turn that died on an error alerts, since nothing resumes it until David }); test("an event inside a subagent owes nothing", () => { - assert.equal(decide({ hook_event_name: "Stop", agent_id: "sub-1", last_assistant_message: "Still waiting on you: b" }, env), null); + assert.equal(decide({ hook_event_name: "Stop", agent_id: "sub-1", last_assistant_message: "๐Ÿ›‘ **NEED YOU** โ€” b" }, env), null); }); test("the alert links to the session's own page", () => { @@ -188,7 +234,7 @@ test("a refusal from Pushover is reported with its reason", async () => { test("as a hook: owed but unsendable prints one systemMessage, and still exits 0", () => { const r = spawnSync("node", [SCRIPT], { input: JSON.stringify({ hook_event_name: "Stop", last_assistant_message: "๐Ÿ‘€ **FYI** โ€” a\n\n๐Ÿ›‘ **NEED YOU** โ€” x" }), - env: { PATH: process.env.PATH }, + env: { PATH: process.env.PATH, NEEDS_YOU_ALERT_STATE_DIR: mkdtempSync(join(tmpdir(), "nya-")) }, encoding: "utf8", }); assert.equal(r.status, 0); @@ -205,7 +251,7 @@ test("as a hook: nothing owed prints nothing, and bad input never blocks a stop JSON.stringify({ hook_event_name: "Notification", notification_type: "idle_prompt" }), "not json", ]) { - const r = spawnSync("node", [SCRIPT], { input, env: { PATH: process.env.PATH }, encoding: "utf8" }); + const r = spawnSync("node", [SCRIPT], { input, env: { PATH: process.env.PATH, NEEDS_YOU_ALERT_STATE_DIR: mkdtempSync(join(tmpdir(), "nya-")) }, encoding: "utf8" }); assert.equal(r.status, 0, input); assert.equal(r.stdout, "", input); } diff --git a/scripts/needs-you-alert.mjs b/scripts/needs-you-alert.mjs index b3b1276..4231fb5 100644 --- a/scripts/needs-you-alert.mjs +++ b/scripts/needs-you-alert.mjs @@ -41,6 +41,20 @@ * - StopFailure: the turn ended on an error, so nothing resumes the session * until he does. * + * ONE ASK, ONE RING (David, 2026-11-03: a session that kept working past an + * unanswered ask re-rang it on every hand-back, because each footer still + * named it). So on `Stop`: + * - a `Still waiting on you:` line rings nothing โ€” by definition it marks + * an ask already rung and unchanged; + * - a footer that waits on him rings only when what it waits on differs + * from the last thing that rang in this session. The hook keeps that one + * key in a small file under the OS temp directory, keyed by session id, + * and clears it on a hand-back that waits on nothing, so the next ask โ€” + * even a word-for-word repeat of an answered one โ€” rings. + * A file that cannot be read or written rings, never drops: a duplicate is + * cheaper than a missed ask. `Notification` and `StopFailure` are not + * deduplicated; each is a new event. + * * Nothing else rings: not a ๐Ÿ‘€ FYI, not a build report, not a hand-back whose * work continues (`running โ€”`, `after โ€”`), not a footer in none of * the forms, not text with no footer. Each alert goes once, at Pushover's high @@ -70,7 +84,9 @@ * anyone using it โ€” is an acceptable home for them. */ -import { basename } from "node:path"; +import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; +import { tmpdir } from "node:os"; +import { basename, join } from "node:path"; import { pathToFileURL } from "node:url"; export const PUSHOVER_URL = "https://api.pushover.net/1/messages.json"; @@ -197,7 +213,9 @@ export function repoName(input, env = {}) { /** * Pure: from a hook's input, the alert the event owes โ€” one only when the - * session is waiting on David โ€” or null. Kept free of I/O so the tests + * session is waiting on David โ€” or null; on `Stop`, also the `key` naming + * what it waits on, which `alertFor` deduplicates against. `clear: true` + * marks a hand-back that waits on nothing. Kept free of I/O so the tests * exercise the rule rather than the network. */ export function decide(input, env = {}) { @@ -209,20 +227,25 @@ export function decide(input, env = {}) { const text = withoutFences(input.last_assistant_message ?? ""); if (text.trim() === "") return null; - const ask = text.match(ASK_BANNER) ?? text.match(OPEN_ASK); - if (ask) return { ...WAITING, title: `๐Ÿ›‘ ${repo}: Claude needs you`, message: plain(ask[1]) }; + const banner = text.match(ASK_BANNER); + if (banner) return { ...WAITING, title: `๐Ÿ›‘ ${repo}: Claude needs you`, message: plain(banner[1]), key: `ask:${plain(text.slice(banner.index), Infinity)}` }; // the whole banner โ€” heading, options, recommendation โ€” so a changed decision under the same heading rings (Codex, PR #279 round 2) + if (OPEN_ASK.test(text)) return null; // an ask already rung, unchanged const f = footer(text); if (!f) return null; const form = nextForm(f.next); const needs = needsSomething(f.needsYou); - if (!needs && form !== "stopped" && form !== "nothing") return null; + if (!needs && form !== "stopped" && form !== "nothing") return { clear: true }; const parts = []; if (needs) parts.push(`Needs you: ${f.needsYou}`); if (f.done) parts.push(`Done: ${f.done}`); if (f.next && !needs) parts.push(`Next: ${f.next}`); const title = form === "nothing" && !needs ? `โœ… ${repo}: done โ€” over to you` : `๐Ÿ›‘ ${repo}: waiting on you`; - return { ...WAITING, title, message: plain(parts.join(" ยท ")) }; + // Keys use the whole text, never the lock-screen truncation, so asks that differ only past 240 characters stay distinct. + // A stopped or nothing hand-back keys on what was done as well: a second finished task is a new hand-back even when its + // Next: is worded the same (Codex, PR #279 round 1). + const key = needs ? `needs:${plain(f.needsYou, Infinity)}` : `next:${plain(f.next, Infinity)}|done:${plain(f.done ?? "", Infinity)}`; + return { ...WAITING, title, message: plain(parts.join(" ยท ")), key }; } if (event === "Notification") { if (!BLOCKING_NOTIFICATIONS.has(input.notification_type)) return null; @@ -238,6 +261,48 @@ export function decide(input, env = {}) { return null; } +/** Where one session's last-rung key lives. */ +export function statePath(input, env = {}) { + const dir = env.NEEDS_YOU_ALERT_STATE_DIR || join(tmpdir(), "needs-you-alert"); + const id = String(input?.session_id || basename(String(input?.transcript_path || ""), ".jsonl") || "session").replace(/[^A-Za-z0-9._-]/g, "_"); + return join(dir, `${id}.last`); +} + +function readKey(path) { + try { + return readFileSync(path, "utf8"); + } catch { + return null; + } +} + +function writeKey(path, key) { + try { + mkdirSync(join(path, ".."), { recursive: true }); + writeFileSync(path, key); + } catch {} +} + +/** + * The alert the event owes after deduplication, or null. A `Stop` whose key + * matches the last one rung in this session owes nothing; a hand-back that + * waits on nothing forgets the last key. + */ +export function alertFor(input, env = {}) { + const d = decide(input, env); + if (!d) return null; + const path = statePath(input, env); + if (d.clear) { + writeKey(path, ""); + return null; + } + if (!d.key) return d; + if (readKey(path) === d.key) return null; + writeKey(path, d.key); + const { key, ...alert } = d; + return alert; +} + /** The Pushover form for one alert. */ export function pushoverBody(alert, env) { const body = new URLSearchParams({ @@ -287,7 +352,7 @@ async function main() { } catch { return; } - const alert = decide(input, process.env); + const alert = alertFor(input, process.env); if (!alert) return; const failure = await send(alert, process.env); if (failure) {