From df2be546dcb7ad3d053bda4b2ab1acdccd319c22 Mon Sep 17 00:00:00 2001 From: claudemm Date: Mon, 3 Aug 2026 16:52:22 +0300 Subject: [PATCH 1/2] A rejected decision is now answered in the room, not just logged petrus typed "/approve f2af1c66" from his tablet today and nothing happened. The owner guard rejected it -- correctly, that device posts as "@petrus-boox" rather than "petrus" -- wrote one line to stderr, and left the intent pending. He saw no error, assumed the approval had landed, and carried on. He had been filming the approval flow for a promo video minutes earlier. The guard is right and is not touched here. What was wrong is that the approval path failed silently, which is the one way it must never fail: the whole point of a confirmation is that the person knows what their tap did. "sender is not the owner" in a log nobody reads is not telling them. Both reject branches now post the reason back to the room: - not a recognised owner identity - unknown/expired intent id Both say explicitly that the decision was NOT recorded and the intent is still pending, because "ignored" alone leaves someone guessing whether it half-worked. Costs one request per rejected message, and the `seen` set makes that once rather than once per poll. Reply failures are caught and logged: not telling someone their tap was rejected is bad, dropping every later tap because one POST failed is worse. The replies cannot re-trigger the poller -- the /approve|/deny regex is anchored at the start of the message and these begin with a backtick. Does NOT change who may approve. Whether @petrus-boox should be able to settle intents is a separate decision, and it belongs to petrus: that handle is a device identity backed by an API key on a tablet, so granting it approval authority grants it to whoever holds that key. Co-Authored-By: Claude Opus 5 --- src/confirmations.mjs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/confirmations.mjs b/src/confirmations.mjs index 5658841..9385ea5 100644 --- a/src/confirmations.mjs +++ b/src/confirmations.mjs @@ -1073,6 +1073,31 @@ export function startChatReplyPoller({ apiKey, room, intervalMs = 5000, log }) { const emit = log || ((msg) => process.stderr.write(`[iak-mcp] ${msg}\n`)); const seen = new Set(); let primed = false; + // A dropped decision has to be VISIBLE, not merely logged. On 2026-08-03 + // petrus typed "/approve f2af1c66" from his tablet; the owner guard below + // rejected it because that device posts as "@petrus-boox" rather than + // "petrus", wrote one line to stderr, and left the intent pending. He saw + // no error, assumed the approval had landed, and moved on — the approval + // path failing in the one way it must never fail, silently. Every reject + // branch now answers in the room. Costs one request per rejected message, + // and `seen` guarantees that is once, not once per poll. + // + // No feedback loop: these replies never match the /approve|/deny regex + // below, which is anchored at the start of the message. + const reply = async (body) => { + try { + await fetch('https://groupmind.one/api/v1/messages', { + method: 'POST', + headers: { 'X-API-Key': apiKey, 'Content-Type': 'application/json' }, + body: JSON.stringify({ room, body }), + }); + } catch (e) { + // Never let a failed reply break the poll loop: not telling someone + // their tap was rejected is bad, but dropping every later tap on the + // floor because one POST failed is worse. + emit(`reply failed: ${e.message}`); + } + }; const poll = async () => { try { const url = `https://groupmind.one/api/v1/rooms/${encodeURIComponent(room)}/messages?limit=30`; @@ -1095,6 +1120,11 @@ export function startChatReplyPoller({ apiKey, room, intervalMs = 5000, log }) { const sender = String(m.from || '').replace(/^@/, '').toLowerCase(); if (sender !== 'petrus' && m.isHuman !== true) { emit(`${text} from ${m.from}: sender is not the owner — ignoring`); + await reply( + `\`${text}\` was NOT recorded — the intent is still pending. ` + + `Only the account owner can settle intents, and this arrived from ` + + `\`${m.from}\`, which is not a recognised owner identity.` + ); continue; } const decision = match[1].toLowerCase(); @@ -1102,6 +1132,10 @@ export function startChatReplyPoller({ apiKey, room, intervalMs = 5000, log }) { const intent = getIntent(id); if (!intent) { emit(`/${decision} ${id} from ${m.from}: unknown intent, ignoring`); + await reply( + `\`/${decision} ${id}\` was NOT recorded — no intent with that id. ` + + `It has probably expired or been settled already.` + ); continue; } const r = decideIntent(id, decision); From 96a9f9b8c9163bb25ce74ad61855dfcb4d6f504b Mon Sep 17 00:00:00 2001 From: claudemm Date: Mon, 3 Aug 2026 16:56:53 +0300 Subject: [PATCH 2/2] Only answer senders who plausibly are the owner claudeMB's review: replying to every rejected decision amplifies the exact misbehaviour the guard was written for. It was added because a fleet agent auto-replied "/approve " in a loop. Answering each attempt turns a silent log line into the daemon posting to the room -- and that room is petrus's phone notification surface. Worse, a bot that retries on being told "not recorded" ping-pongs indefinitely, and no `seen` set stops it because every round is a genuinely new message id. So the reply now fires only for senders who plausibly ARE the owner: `petrus`, `petrus-boox`, a future `petrus-watch`. That is exactly the case that broke today, and it keeps the whole benefit -- a human who tapped Approve is told it did not land. An agent emitting a spurious /approve does not need telling; the log line was always the right answer for it. `owner` is now a parameter defaulting to 'petrus' rather than another literal. Existing callers behave identically. This ships as a product and the owner is not always called petrus; the guard's own literal stays put until Part B, when who counts as an owner is actually decided. Also strips backticks from the interpolated handle. It is the one value in either string chosen by whoever registered it, and it sits inside a markdown code span that a single backtick escapes. `seen` growing unbounded is real but pre-existing and slow; left alone rather than widened into a bug-fix PR. Co-Authored-By: Claude Opus 5 --- src/confirmations.mjs | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/src/confirmations.mjs b/src/confirmations.mjs index 9385ea5..446a28e 100644 --- a/src/confirmations.mjs +++ b/src/confirmations.mjs @@ -1065,7 +1065,12 @@ export function composeAnnouncers(map) { // // Logs go to stderr only (stdout is the MCP stdio protocol channel — writing // there would corrupt it). Returns the interval handle so callers can stop it. -export function startChatReplyPoller({ apiKey, room, intervalMs = 5000, log }) { +// `owner` is the account handle decisions are accepted from. It also decides +// who is worth ANSWERING when a decision is rejected: see `ownerish` below. +// Defaulting it keeps existing callers behaving identically, but it is a +// parameter rather than a literal because this ships as a product and the +// owner is not always called petrus. +export function startChatReplyPoller({ apiKey, room, intervalMs = 5000, log, owner = 'petrus' }) { if (!apiKey || !room) { process.stderr.write('[iak-mcp] chat-reply poller: missing apiKey or room — disabled\n'); return null; @@ -1084,6 +1089,10 @@ export function startChatReplyPoller({ apiKey, room, intervalMs = 5000, log }) { // // No feedback loop: these replies never match the /approve|/deny regex // below, which is anchored at the start of the message. + // + // Backticks are stripped from interpolated handles: a handle is chosen by + // whoever registered it, and these strings put it inside a markdown code + // span, which one backtick would break out of. const reply = async (body) => { try { await fetch('https://groupmind.one/api/v1/messages', { @@ -1120,11 +1129,28 @@ export function startChatReplyPoller({ apiKey, room, intervalMs = 5000, log }) { const sender = String(m.from || '').replace(/^@/, '').toLowerCase(); if (sender !== 'petrus' && m.isHuman !== true) { emit(`${text} from ${m.from}: sender is not the owner — ignoring`); - await reply( - `\`${text}\` was NOT recorded — the intent is still pending. ` + - `Only the account owner can settle intents, and this arrived from ` + - `\`${m.from}\`, which is not a recognised owner identity.` - ); + // Answer only senders who plausibly ARE the owner (`petrus`, + // `petrus-boox`, a future `petrus-watch`). claudeMB's review caught + // that replying to everything amplifies the very misbehaviour this + // guard was written for: a fleet agent once retried `/approve` in a + // loop, and answering each attempt would turn a silent log line into + // the daemon spamming the room — which is petrus's phone notification + // surface. Worse, a bot that retries on being told "not recorded" + // ping-pongs forever, and no `seen` set stops that because every + // round is a genuinely new message id. + // + // A human who tapped Approve needs to know it did not land. An agent + // emitting a spurious `/approve` does not; the log line was always + // the right answer for it. + const ownerish = sender === owner || sender.startsWith(`${owner}-`); + if (ownerish) { + await reply( + `\`${text}\` was NOT recorded — the intent is still pending. ` + + `Only the account owner can settle intents, and this arrived from ` + + `\`${String(m.from || '').replace(/`/g, '')}\`, which is not a ` + + `recognised owner identity.` + ); + } continue; } const decision = match[1].toLowerCase();