diff --git a/AGENTS.md b/AGENTS.md index a4ba5cc5f..e31d30724 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -195,7 +195,7 @@ The check enumerates submodule contents and fails on what it finds there, becaus ### Architecture Docs Sync -- Channel forwarders deliver to the destination captured when a run was admitted, never to a per-channel last-active slot. `src/messaging/run-pin.ts` builds the identity block (origin/requestId/scope/sessionId/remoteKey/target) that every `agent_done` carries, and `resolveForwarderTarget` refuses an event with no destination or one addressed to another channel. Slack, Discord and Telegram forwarders no longer accept a `getLastTarget`/`getLastChatId` option, so web and CLI turns are not mirrored into chat rooms. Heartbeat destinations are complete or held: a Slack destination needs a thread or an explicit `scope: "channel_root"`, an absent destination sends nothing, and `authorizeExplicitTarget` vouches for a send without rewriting its address. See `structure/telegram.md` and `structure/server_api.md`. +- Channel forwarders deliver to the destination captured when a run was admitted, never to a per-channel last-active slot. `src/messaging/run-pin.ts` builds the identity block (origin/requestId/scope/sessionId/remoteKey/target) that every `agent_done` carries, and `resolveForwarderTarget` refuses an event with no destination or one addressed to another channel. Slack, Discord and Telegram forwarders no longer accept a `getLastTarget`/`getLastChatId` option, so web and CLI turns are not mirrored into chat rooms. Heartbeat destinations are complete or held: a Slack destination needs a thread or an explicit `scope: "channel_root"`, an absent destination sends nothing, and `authorizeExplicitTarget` vouches for a send without rewriting its address. Slack progress cards end their live loop on `message_not_found`/`cant_update_message` or three consecutive failures rather than retrying a dead message. See `structure/telegram.md` and `structure/server_api.md`. - Auto (`permissions:auto`) grants qualified direct-local Jaw API authority across supported runtimes, independently of per-turn secrets. Keep actual/effective loopback, exact browser origin, proxy provenance, explicit outbound destinations and server-only resource options. Safe/custom keep existing scoped/operator paths; full API authority is instance-wide, distinct from provider Safe and task scope. Preserve no-descendant/read-only assignments, captured worker context and honest capability/receipt evidence. See `docs/slack-tools.md` and `structure/server_api.md`. - Slack group DMs use `message.mpim` and optional `mpim:history`; exact `channel_type: mpim` mentions retain channel allowlist and thread policy, never the one-to-one DM bypass. An install without `mpim:history` receives no group-DM traffic at all; that gap is reported in `missingCapabilities` and logged as a reception limitation rather than failing credential validation. An absent scope header is unknown and a present empty header is a known empty grant. Keep `structure/telegram.md` and the validation API docs synchronized. diff --git a/CLAUDE.md b/CLAUDE.md index b3df7083c..86e3e11d2 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,7 +24,7 @@ Native Code interruption seals callbacks before persisting accepted buffered con ## Current Runtime Notes -- Channel forwarders deliver to the destination captured when a run was admitted, never to a per-channel last-active slot. `src/messaging/run-pin.ts` builds the identity block (origin/requestId/scope/sessionId/remoteKey/target) that every `agent_done` carries, and `resolveForwarderTarget` refuses an event with no destination or one addressed to another channel. Slack, Discord and Telegram forwarders no longer accept a `getLastTarget`/`getLastChatId` option, so web and CLI turns are not mirrored into chat rooms. Heartbeat destinations are complete or held: a Slack destination needs a thread or an explicit `scope: "channel_root"`, an absent destination sends nothing, and `authorizeExplicitTarget` vouches for a send without rewriting its address. See `structure/telegram.md` and `structure/server_api.md`. +- Channel forwarders deliver to the destination captured when a run was admitted, never to a per-channel last-active slot. `src/messaging/run-pin.ts` builds the identity block (origin/requestId/scope/sessionId/remoteKey/target) that every `agent_done` carries, and `resolveForwarderTarget` refuses an event with no destination or one addressed to another channel. Slack, Discord and Telegram forwarders no longer accept a `getLastTarget`/`getLastChatId` option, so web and CLI turns are not mirrored into chat rooms. Heartbeat destinations are complete or held: a Slack destination needs a thread or an explicit `scope: "channel_root"`, an absent destination sends nothing, and `authorizeExplicitTarget` vouches for a send without rewriting its address. Slack progress cards end their live loop on `message_not_found`/`cant_update_message` or three consecutive failures rather than retrying a dead message. See `structure/telegram.md` and `structure/server_api.md`. - File sends across Slack, Telegram and Discord share one confirmation vocabulary. A send the vendor will not name is refused rather than reported as delivered (Slack keeps its `files[]` echo requirement, Telegram requires `message_id` > 0, Discord requires a readable Create Message body): those are `ok:false` with `confirmation: 'unconfirmed'`, replacing the older `ok:true, ambiguous:true` no consumer read. Anything forwarding a file result must preserve `confirmation`, or the caption posts twice. See `structure/infra.md` and `structure/telegram.md`. diff --git a/src/slack/progress.ts b/src/slack/progress.ts index 7f26cc1dc..994184acf 100644 --- a/src/slack/progress.ts +++ b/src/slack/progress.ts @@ -5,6 +5,7 @@ import type { DraftStreamOptions } from '../messaging/draft-stream.js'; import type { RemoteTarget } from '../messaging/types.js'; import { log } from '../core/logger.js'; import { redactOutboundPayload } from '../messaging/redact.js'; +import { inc } from '../messaging/metrics.js'; import { t } from '../core/i18n.js'; import { createSlackActivity, projectSlackPrintTool, @@ -15,6 +16,13 @@ import { export type { SlackProgressOutcome, SlackProgressPhase } from './progress-activity.js'; const NATIVE_INTERVAL_MS = 1000; const FALLBACK_INTERVAL_MS = 3200; +/** Consecutive failed live edits before the card stops trying. + * + * Not a cap on updating: a stream that expires mid-job keeps its message and + * keeps editing it, which is how a long run stays visible. This bounds the + * case where those edits stop landing — a transport that has rejected the same + * request three times running is not about to accept the fourth. */ +const MAX_CONSECUTIVE_LIVE_FAILURES = 3; // chat.appendStream is Tier 4 (100+/minute). Reserve at most 90/minute // across this process's streams sharing one credential, including tool updates. const APPEND_SPACING_MS = 667; @@ -118,6 +126,10 @@ export async function startSlackProgress( let state: Ready = { mode: 'none', ts: null }; let closed = false; let remoteEnded = false; + // A status card whose message is gone: Slack will refuse every later edit + // for the same reason, so there is nothing left to try. + let messageGone = false; + let consecutiveLiveFailures = 0; let confirmed = false; let dirty = false; let lastAttemptAt = -Infinity; @@ -143,6 +155,22 @@ export async function startSlackProgress( if (idleTimer) clearTimer(idleTimer); timer = idleTimer = null; }; + + /** Stop live updating this card, once, for a stated reason. + * + * The idle loop re-dirtied the snapshot every 3.2s and `schedule()` sent it + * again, so a card whose message had been deleted produced 47 consecutive + * `chat.update` → `message_not_found` calls over two and a half minutes and + * would have kept going for the life of the job (#744). Ending the live loop + * is the whole fix: the card freezes at its last known state, which is + * honest, and the answer still arrives by its own path. */ + function endLive(reason: string, gone: boolean): void { + if (remoteEnded) return; + if (gone) messageGone = true; + remoteEnded = true; + clearScheduled(); + inc('slack.progress.stream_state_lost', { channel: 'slack', result: reason }); + } function abortProgress(): void { closed = true; clearScheduled(); @@ -237,16 +265,37 @@ export async function startSlackProgress( if (!response.attempted) { dirty = true; return; } if (response.result.ok) { lastSignature = signature; lastSnapshot = snapshot; } if (response.result.status === 429 || response.result.error === 'ratelimited' || response.result.error === 'rate_limited') dirty = true; - if (method === 'chat.appendStream' && response.result.error === 'message_not_in_streaming_state') useMessageUpdates(); - if (response.result.error === 'stopped_by_user') { - remoteEnded = true; - clearScheduled(); + const error = response.result.error ?? ''; + if (response.result.ok) consecutiveLiveFailures = 0; + // The message itself is unreachable. Retrying cannot bring it back, and + // posting a replacement would put a second status card in the thread. + if (error === 'message_not_found' || error === 'cant_update_message') { + endLive(error, true); + return; + } + if (error === 'stopped_by_user') { endLive(error, false); return; } + if (method === 'chat.appendStream' && error === 'message_not_in_streaming_state') { + // Slack expires a stream while a long job continues. Keep the message + // this stream already owns and edit it from here on, so the card stays + // current instead of freezing at the five-minute mark. + useMessageUpdates(); + return; + } + // A rate limit is a "later", not a "no": the embargo already spaces it out + // and `dirty` was set above so the same content is retried. + if (!response.result.ok && response.result.status !== 429 + && error !== 'ratelimited' && error !== 'rate_limited') { + if (++consecutiveLiveFailures >= MAX_CONSECUTIVE_LIVE_FAILURES) { + endLive(error || 'repeated_update_failure', false); + } } } function startIdle(): void { if (closed || remoteEnded || !state.ts) return; idleTimer = setTimer(() => { idleTimer = null; + // The flag can be raised while this timer is pending. + if (closed || remoteEnded) return; dirty = true; schedule(); startIdle(); @@ -322,6 +371,10 @@ export async function startSlackProgress( await ready; await inFlight; if (!state.ts || controller.signal.aborted) return; + // A card whose message is gone takes no terminal edit either. + // `message_not_found` already counted as confirmation before; + // the difference now is that no request is spent proving it. + if (messageGone) { confirmed = true; return; } const snapshot = model.snapshot(); let response = await call(state.mode === 'native' ? 'chat.stopStream' : 'chat.update', { channel: address.targetId, ts: state.ts, diff --git a/structure/str_func.md b/structure/str_func.md index dd9993cd2..613859274 100644 --- a/structure/str_func.md +++ b/structure/str_func.md @@ -380,7 +380,7 @@ cli-jaw/ │ │ ├── scope-status.ts ← OAuth grant drift 단일 소유자 (auth.test의 x-oauth-scopes를 manifest 요구 집합과 대조, 미관측을 '이상 없음'과 구분, doctor·health·identity 경고가 공유) (191L) ✨ │ │ ├── allowlist-audit.ts ← channelIds 변경 방향 분류 + 축소 감사 기록 (게이트 리더 기준 정규화, route·settings watcher 양쪽이 공유) (125L) ✨ │ │ ├── hot-notify.ts ← CLI 설정 변경 후 실행 중 서버 hot-reload 통지 (loopback PUT /api/settings → transport 재시작, version skew 감지) (41L) -│ │ ├── progress.ts ← native plan stream + explicit unsupported fallback, bounded IO/Retry-After and terminal receipt (346L) +│ │ ├── progress.ts ← native plan stream + explicit unsupported fallback, bounded IO/Retry-After and terminal receipt (399L) │ │ ├── progress-activity.ts ← safe fixed-category activity projection, bounded recent observations and delivery receipt (244L) │ │ ├── progress-files.ts ← shared bounded file target projection under captured working directory (61L) │ │ ├── progress-detail.ts ← explicit purposes and finite safe command action summaries (319L) diff --git a/tests/unit/slack-progress-stream.test.ts b/tests/unit/slack-progress-stream.test.ts index 502c8559f..83b080dac 100644 --- a/tests/unit/slack-progress-stream.test.ts +++ b/tests/unit/slack-progress-stream.test.ts @@ -291,6 +291,49 @@ test('a stream closed after five minutes continues editing its own status throug assert.equal(h.clock.timers.size, 0); }); +test('SPS-744: an expired stream whose message is gone stops instead of retrying forever', async () => { + // The live incident: the stream expired, the card fell back to chat.update, + // and the message was no longer there. Nothing promoted that to "stop", so + // the idle loop re-dirtied the snapshot every 3.2s and sent the same doomed + // update 47 times across two and a half minutes (#744). + const h = harness(c => c.method === 'chat.appendStream' + ? { payload: { ok: false, error: 'message_not_in_streaming_state' } } + : c.method === 'chat.update' + ? { payload: { ok: false, error: 'message_not_found' } } + : ok()); + const p = await startSlackProgress(h.token, target, '', h.options); + await p.ready(); + await h.clock.advance(60000); + const afterFirstMinute = h.calls.filter(c => c.method === 'chat.update').length; + assert.equal(afterFirstMinute, 1, 'one edit discovers the message is gone'); + + for (let minute = 0; minute < 5; minute++) await h.clock.advance(60000); + assert.equal(h.calls.filter(c => c.method === 'chat.update').length, afterFirstMinute, + 'a gone message is never edited again'); + assert.equal(h.calls.filter(c => c.method === 'chat.postMessage').length, 0, + 'a dead card is not replaced by a second one'); + assert.equal(h.clock.timers.size, 0, 'the idle loop is stopped, not merely skipped'); + + const before = h.calls.length; + await p.finish('complete', { bodyDelivered: true }); + assert.equal(h.calls.length, before, 'finalizing spends no request on a message known to be gone'); + assert.equal(p.terminalConfirmed(), true); +}); + +test('SPS-744: three consecutive failed edits stop the live loop', async () => { + // Not every failure names itself. A transport rejecting the same request over + // and over is the same storm wearing a different error string. + const h = harness(c => c.method === 'chat.appendStream' || c.method === 'chat.update' + ? { payload: { ok: false, error: 'internal_error' } } : ok()); + const p = await startSlackProgress(h.token, target, '', h.options); + await p.ready(); + for (let minute = 0; minute < 5; minute++) await h.clock.advance(60000); + const attempts = h.calls.filter(c => c.method === 'chat.appendStream' || c.method === 'chat.update').length; + assert.ok(attempts <= 3, `live edits bounded, saw ${attempts}`); + assert.equal(h.clock.timers.size, 0); + await p.finish('complete', { bodyDelivered: true }); +}); + test('stream closure first discovered at finalization updates the same message once', async () => { const h = harness(c => c.method === 'chat.stopStream' ? { payload: { ok: false, error: 'message_not_in_streaming_state' } } : ok());