Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -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`.

Expand Down
61 changes: 57 additions & 4 deletions src/slack/progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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();
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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,
Expand Down
2 changes: 1 addition & 1 deletion structure/str_func.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
43 changes: 43 additions & 0 deletions tests/unit/slack-progress-stream.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down