diff --git a/AGENTS.md b/AGENTS.md index c9e440e09..a4ba5cc5f 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -195,6 +195,8 @@ 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`. + - 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 20352ac64..b3df7083c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -24,6 +24,8 @@ 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`. + - 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`. The Classic permission selector offers Auto (YOLO) / Safe choices, stored as literal `auto` / `safe`. Server startup preserves the saved policy; never reintroduce the obsolete safe-to-auto coercion. Existing runtime-specific policy support and settings invalidation still apply. diff --git a/package-lock.json b/package-lock.json index dc79689ef..ffffa2d01 100644 --- a/package-lock.json +++ b/package-lock.json @@ -160,9 +160,6 @@ "cpu": [ "arm64" ], - "libc": [ - "glibc" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -176,9 +173,6 @@ "cpu": [ "arm64" ], - "libc": [ - "musl" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -192,9 +186,6 @@ "cpu": [ "x64" ], - "libc": [ - "glibc" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -208,9 +199,6 @@ "cpu": [ "x64" ], - "libc": [ - "musl" - ], "license": "SEE LICENSE IN LICENSE.md", "optional": true, "os": [ @@ -2670,9 +2658,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2690,9 +2675,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -2710,9 +2692,6 @@ "ppc64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2730,9 +2709,6 @@ "s390x" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2750,9 +2726,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MIT", "optional": true, "os": [ @@ -2770,9 +2743,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MIT", "optional": true, "os": [ @@ -6603,9 +6573,6 @@ "arm64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -6627,9 +6594,6 @@ "arm64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -6651,9 +6615,6 @@ "x64" ], "dev": true, - "libc": [ - "glibc" - ], "license": "MPL-2.0", "optional": true, "os": [ @@ -6675,9 +6636,6 @@ "x64" ], "dev": true, - "libc": [ - "musl" - ], "license": "MPL-2.0", "optional": true, "os": [ diff --git a/src/agent/claude-runtime-run.ts b/src/agent/claude-runtime-run.ts index 13071e3d7..8094e03b3 100644 --- a/src/agent/claude-runtime-run.ts +++ b/src/agent/claude-runtime-run.ts @@ -9,6 +9,7 @@ import { acquireClaudeRuntime } from './runtime-pool.js'; import { ClaudeAcquireFailure } from './claude-runtime-pool.js'; import { handleAgentExit, type ExitHandlerParams } from './lifecycle-handler.js'; import { handoffRuntimeOutcome } from './runtime/outcome.js'; +import { runPinFields } from '../messaging/run-pin.js'; import { RuntimeProjection, type RuntimeEnd } from './runtime/projection.js'; import { recordRuntimeEvent } from './runtime/events.js'; import { reserveClaudeRun } from './runtime/claude-run-controls.js'; @@ -118,9 +119,10 @@ export function startClaudeNativeRun(input: ClaudeNativeRunOptions): { child: nu ensureFallbackStarted(); if (!ctx.runtimeTerminalAttempted && !selected) { ctx.runtimeTerminalAttempted = true; - broadcast('agent_done', { traceRunId, scope: base.scopeKey, sessionId: base.chatSessionId, origin: base.origin, - cli: 'claude', ...(worker ? { isEmployee: true } : {}), - ...(base.opts.requestId ? { requestId: base.opts.requestId } : {}), + broadcast('agent_done', { ...runPinFields({ origin: base.origin, requestId: base.opts.requestId, + scope: base.scopeKey, sessionId: base.chatSessionId, + remoteKey: base.opts.remoteKey, target: base.opts.target }), + traceRunId, cli: 'claude', ...(worker ? { isEmployee: true } : {}), text: final.status === 'stopped' ? '' : `❌ ${diagnostic()}`, error: true, runtimeStatus: final.status, runtimeFinality: final.finalText === null ? 'absent' : 'present' }, input.audience); } diff --git a/src/agent/lifecycle-handler.ts b/src/agent/lifecycle-handler.ts index b8bb10e5c..6f3d5d6c5 100644 --- a/src/agent/lifecycle-handler.ts +++ b/src/agent/lifecycle-handler.ts @@ -28,6 +28,7 @@ import type { RuntimeEventBody, RuntimeTransport, RuntimeTurnOutcome } from '../ import { handoffRuntimeOutcome, lifecycleRuntimeOutcome, runtimeOutcomeExitCode } from './runtime/outcome.js'; import type { ToolEntry } from '../types/agent.js'; import type { RemoteTarget } from '../messaging/types.js'; +import { runPinFields } from '../messaging/run-pin.js'; import { resolveSpawnOutputText } from './events/helpers.js'; import { isKiroPlainTextCli, isKiroResumeDegradedOutput } from './kiro-runtime.js'; import { @@ -163,6 +164,9 @@ type LifecycleSpawnOptions = { scopeKey?: string; chatSessionId?: string; remoteKey?: string; + /** Destination captured when the run was admitted. Survives `...opts` + * through fallback and retry respawns so the pin cannot be lost. */ + target?: RemoteTarget; cli?: string; model?: string; _heartbeatAnchorId?: number; @@ -348,6 +352,11 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise const code = runtimeOutcomeExitCode(nativeOutcome, processCode); const nativeRequestId = ctx.requestId ?? opts.requestId; if (mainManaged) revokeSlackToolGrant(nativeRequestId); + // This run's identity and destination, carried on every terminal event it + // emits below. See src/messaging/run-pin.ts for why it is carried and not + // looked up (#742/#743). + const donePin = runPinFields({ origin, requestId: nativeRequestId, scope: scopeKey, + sessionId: chatSessionId, remoteKey: opts.remoteKey, target: opts.target }); const nativeTraceRunId = ctx.traceRunId; const effectiveProvider = params.effectiveProvider; const runtimeCli = cli; @@ -438,7 +447,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise ...opts, _isSmokeContinuation: true, _skipInsert: true, }); contPromise.then((r) => resolve(r)).catch(() => { - broadcast('agent_done', { ...runTag(ctx), + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ Smoke continuation failed. Original: ${ctx.fullText.slice(0, 200)}`, error: true, origin, ...empTag, @@ -646,7 +655,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise _skipInsert: true, }); retryP.then(resolve).catch(() => { - broadcast('agent_done', { ...runTag(ctx), + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: '❌ kiro stale resume and fresh retry failed', error: true, origin, @@ -707,6 +716,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise handoffRuntimeOutcome(ctx, { ...nativeOutcome, finalText: finalContent }); ctx.runtimeTerminalAttempted = true; broadcast('agent_done', { + ...donePin, ...(nativeTraceRunId ? { traceRunId: nativeTraceRunId } : {}), text: runtimeCompatibilityText(finalContent), runtimeFinality: finalContent === null ? 'absent' : 'present', @@ -818,7 +828,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise try { linkTraceRunToMessage(ctx.traceRunId, messageId); } catch { console.warn('[trace] print link failed'); } } - broadcast('agent_done', { ...runTag(ctx), text: finalContent, toolLog: sanitizedToolLog, origin, ...empTag, ...(wasSteer ? { steered: true } : {}) }); + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: finalContent, toolLog: sanitizedToolLog, origin, ...empTag, ...(wasSteer ? { steered: true } : {}) }); if (opts._heartbeatAnchorId) { try { @@ -868,7 +878,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise // Classified like the retry-exhausted site below: a watchdog kill is // the one failure the channel MUST show, and the forwarder gate // drops anything without errorKind (#519 round 2). - { ...runTag(ctx), text: `❌ ${errMsg}`, error: true, errorKind, cli: runtimeCli, origin, ...empTag }, + { ...runTag(ctx), ...donePin, text: `❌ ${errMsg}`, error: true, errorKind, cli: runtimeCli, origin, ...empTag }, isEmployee ? 'internal' : 'public', ); finalizeRun('error', errMsg); @@ -946,7 +956,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise _skipInsert: true, }) as { promise: Promise<{ text: string; code: number }> }; retryP.then(resolve).catch(() => { - broadcast('agent_done', { ...runTag(ctx), text: `❌ ${errMsg} (fresh-session retry failed)`, error: true, origin, ...empTag, ...(wasSteer ? { steered: true } : {}) }, isEmployee ? 'internal' : 'public'); + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ ${errMsg} (fresh-session retry failed)`, error: true, origin, ...empTag, ...(wasSteer ? { steered: true } : {}) }, isEmployee ? 'internal' : 'public'); resolve({ text: '', code: 1 }); if (mainManaged && !opts.internal) processQueue(scopeKey); }); @@ -969,7 +979,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise } insertMessage.run('assistant', `⏱️ ${errMsg}`, cli, model, settings["workingDir"] || null, chatSessionId); } - broadcast('agent_done', { ...runTag(ctx), text: `❌ ${errMsg}`, error: true, errorKind, cli: runtimeCli, origin, ...empTag, ...(wasSteer ? { steered: true } : {}) }, isEmployee ? 'internal' : 'public'); + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ ${errMsg}`, error: true, errorKind, cli: runtimeCli, origin, ...empTag, ...(wasSteer ? { steered: true } : {}) }, isEmployee ? 'internal' : 'public'); finalizeRun('error', errMsg); resolve({ text: '', code: 1 }); if (mainManaged && !opts.internal) processQueue(scopeKey); @@ -1006,7 +1016,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise ...opts, _retryAttempt: mainAttempt + 1, _skipInsert: true, }); retryP.then((r) => resolve(r)).catch(() => { - broadcast('agent_done', { ...runTag(ctx), text: `❌ ${errMsg} (재시도 실패, attempt ${mainAttempt + 1})`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ ${errMsg} (재시도 실패, attempt ${mainAttempt + 1})`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); resolve({ text: '', code: 1 }); if (mainManaged && !opts.internal) processQueue(scopeKey); }); @@ -1063,7 +1073,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise ...opts, cli: fallbackCli, _isFallback: true, _skipInsert: true, }); retryP.then((r) => resolve(r)).catch(() => { - broadcast('agent_done', { ...runTag(ctx), + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ Fallback (${fallbackCli}) failed`, error: true, origin, ...empTag, }, isEmployee ? 'internal' : 'public'); @@ -1073,10 +1083,10 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise return; } } - // The `{ ...runTag(ctx),` opening stays on this line: RID-001 in + // The `{ ...runTag(ctx), ...donePin,` opening stays on this line: RID-001 in // tests/unit/web-sse-replay-idempotency.test.ts matches that exact shape // to prove every agent_done carries its trace run id. - broadcast('agent_done', { ...runTag(ctx), + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ ${errMsg}`, error: true, // Classified here so a forwarder never re-parses Korean prose to @@ -1125,7 +1135,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise retryP.then((r) => resolve(r)).catch((retryErr: Error) => { const retryMessage = retryErr?.message ? `; retry=${retryErr.message}` : ''; const diagnostic = `${cls.message} (fresh employee session retry failed${retryMessage})`; - broadcast('agent_done', { ...runTag(ctx), text: `❌ ${diagnostic}`, error: true, origin, isEmployee: true }, 'internal'); + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ ${diagnostic}`, error: true, origin, isEmployee: true }, 'internal'); resolve({ text: '', code: 1, diagnostic }); }); return; @@ -1154,7 +1164,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise ...opts, _retryAttempt: empAttempt + 1, _skipInsert: true, _skipResume: true, }); retryP.then((r) => resolve(r)).catch(() => { - broadcast('agent_done', { ...runTag(ctx), text: `❌ ${cls.message} (재시도 실패, attempt ${empAttempt + 1})`, error: true, origin, isEmployee: true }, 'internal'); + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: `❌ ${cls.message} (재시도 실패, attempt ${empAttempt + 1})`, error: true, origin, isEmployee: true }, 'internal'); resolve({ text: '', code: 1, diagnostic: cls.message }); }); }, empDelayMs)); @@ -1198,7 +1208,7 @@ export async function handleAgentExit(params: ExitHandlerParams): Promise _skipSessionPersist: true, }); retryP.then(resolve).catch(() => { - broadcast('agent_done', { ...runTag(ctx), + broadcast('agent_done', { ...runTag(ctx), ...donePin, text: '❌ kiro resume empty and fresh retry failed', error: true, origin, diff --git a/src/agent/spawn.ts b/src/agent/spawn.ts index 4520c8c94..c012221cb 100644 --- a/src/agent/spawn.ts +++ b/src/agent/spawn.ts @@ -91,6 +91,7 @@ import { clearNativeStartFailure, nativeStartFailure, recordNativeStartFailure } import { asCliEventRecord, discriminate, fieldString, type CliEventRecord } from '../types/cli-events.js'; import { isRemoteTarget, type RemoteTarget } from '../messaging/types.js'; import { buildRemoteBindingKey } from '../messaging/session-key.js'; +import { runPinFields } from '../messaging/run-pin.js'; import { isRetiredCliSelection, retiredRuntimeDiagnostic } from '../types/cli-engine.js'; import { runBeforeSpawnChecks, type PolicyVerdict } from '../core/policy-hooks.js'; import { appendTraceEvent, createTraceId, finalizeTraceRun, stampTraceTool, startTraceRun, updateTraceToolRow } from '../trace/store.js'; @@ -1220,6 +1221,11 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { const scopeKey = binding.scope, chatSessionId = binding.chatSessionId; opts = stripUndefined({ ...opts, scopeKey, chatSessionId, ...(opts.remoteKey ? { remoteKey: opts.remoteKey } : {}) }); + // Captured once, at admission. Every terminal event this run emits carries + // it, so a forwarder never has to guess the destination from global state + // and a waiter can tell someone else's completion from its own (#742/#743). + const runPin = runPinFields({ origin, requestId: opts.requestId, scope: scopeKey, + sessionId: chatSessionId, remoteKey: opts.remoteKey, target: opts.target }); let mainRun = mainManaged ? activeMainProcesses.get(scopeKey) : undefined; if (mainManaged && mainRun && !opts._settingsGateWaited) { @@ -1289,8 +1295,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { settleOnce(opts.requestId, 'failed', { error: diagnostic, text: message, scope: scopeKey, sessionId: chatSessionId }); broadcast('agent_done', { - text: message, error: true, origin, cli, scope: scopeKey, sessionId: chatSessionId, - ...(opts.requestId ? { requestId: opts.requestId } : {}), ...empTag, + ...runPin, text: message, error: true, cli, ...empTag, }, isEmployee ? 'internal' : 'public'); try { opts.lifecycle?.onExit?.(78); } catch { console.warn('[runtime] retirement exit observer failed'); } resolve!({ text: message, code: 78 }); @@ -1325,8 +1330,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { const released = mainManaged && activeMainProcesses.get(scopeKey) === mainRun && releaseMainRun(scopeKey, null, ownerGeneration); broadcast('agent_done', { - text: message, error: true, origin, cli, scope: scopeKey, sessionId: chatSessionId, - ...(opts.requestId ? { requestId: opts.requestId } : {}), ...empTag, + ...runPin, text: message, error: true, cli, ...empTag, }, isEmployee ? 'internal' : 'public'); resolve!({ text: message, code: 78 }); if (released) void processQueue(scopeKey); @@ -1726,7 +1730,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { const msg = formatCliUnavailableMessage(cli, detected); console.error(`[jaw:${agentLabel}] ${msg}`); if (mainManaged) clearLiveRun(liveScope); - broadcast('agent_done', { text: `❌ ${msg}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); + broadcast('agent_done', { ...runPin, text: `❌ ${msg}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); resolve!({ text: '', code: 127 }); if (mainManaged) { releaseMainRun(scopeKey, null, ownerGeneration); @@ -1808,7 +1812,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { if (capturedRun && activeMainProcesses.get(scopeKey) === capturedRun) releaseMainRun(scopeKey, null, ownerGeneration); cleanupClaudeWorker(); const text = 'Claude native runtime could not be admitted.'; - broadcast('agent_done', { text, error: true, origin, cli, scope: scopeKey, sessionId: chatSessionId, ...empTag }, traceAudience); + broadcast('agent_done', { ...runPin, text, error: true, cli, ...empTag }, traceAudience); if (mainManaged && !activeMainProcesses.has(scopeKey)) void processQueue(scopeKey); return { child: null, promise: Promise.resolve({ text, code: 1 }) }; } @@ -1917,8 +1921,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { if (!nativeStarted && !runtimeEnded) startFailedRuntime(); if (!ctx.runtimeTerminalAttempted) { ctx.runtimeTerminalAttempted = true; - broadcast('agent_done', { traceRunId, scope: scopeKey, sessionId: chatSessionId, origin, cli, - ...(opts.requestId ? { requestId: opts.requestId } : {}), + broadcast('agent_done', { ...runPin, traceRunId, cli, text: selected.status === 'stopped' ? '' : `❌ ${diagnostic()}`, error: true, runtimeStatus: selected.status, runtimeFinality: selected.finalText === null ? 'absent' : 'present', }, traceAudience); @@ -2144,7 +2147,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { } else { activeProcesses.delete(agentLabel); } - broadcast('agent_done', { text: `❌ ${msg}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); + broadcast('agent_done', { ...runPin, text: `❌ ${msg}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); finishPrintActivity(ctx, { kind: 'turn-end', status: 'error', finalText: null, error: msg }); resolve!({ text: '', code: 1 }); if (mainManaged) void processQueue(scopeKey); @@ -2792,7 +2795,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { clearLiveRun(liveScope); try { broadcast('agent_status', { running: false, agentId: agentLabel }); - broadcast('agent_done', { text: `❌ Pi RPC acquire failed: ${err.message}`, error: true, origin }, 'public'); + broadcast('agent_done', { ...runPin, text: `❌ Pi RPC acquire failed: ${err.message}`, error: true, origin }, 'public'); } catch { console.warn('[jaw:pi] acquisition diagnostic delivery failed'); } releaseMainRun(scopeKey, null, ownerGeneration); } @@ -3369,7 +3372,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { if (ownsRun) { clearLiveRun(liveScope); broadcast('agent_status', { running: false, agentId: agentLabel }); - broadcast('agent_done', { text: `❌ Codex AppServer acquire failed: ${err.message}`, error: true, origin }, 'public'); + broadcast('agent_done', { ...runPin, text: `❌ Codex AppServer acquire failed: ${err.message}`, error: true, origin }, 'public'); releaseMainRun(scopeKey, null, ownerGeneration); } resolve!({ text: '', code: 1 }); @@ -3430,7 +3433,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { // later request for this scope would be rejected as "already running". console.error(`[jaw:${agentLabel}] ${decision.reason}`); if (mainManaged) clearLiveRun(liveScope); - broadcast('agent_done', { text: `❌ ${decision.reason}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); + broadcast('agent_done', { ...runPin, text: `❌ ${decision.reason}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); resolve!({ text: '', code: 126 }); if (mainManaged) { releaseMainRun(scopeKey, null, ownerGeneration); @@ -3512,7 +3515,7 @@ export function spawnAgent(prompt: string, opts: SpawnOpts = {}): SpawnResult { } else { activeProcesses.delete(agentLabel); } - broadcast('agent_done', { text: `❌ ${msg}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); + broadcast('agent_done', { ...runPin, text: `❌ ${msg}`, error: true, origin, ...empTag }, isEmployee ? 'internal' : 'public'); finishPrintActivity(ctx, { kind: 'turn-end', status: 'error', finalText: null, error: msg }); resolve!({ text: '', code: 127 }); if (mainManaged) void processQueue(scopeKey); diff --git a/src/core/config.ts b/src/core/config.ts index a370d5c19..e3b18b0cf 100644 --- a/src/core/config.ts +++ b/src/core/config.ts @@ -1463,6 +1463,10 @@ export interface HeartbeatDestination { channel: 'telegram' | 'discord' | 'slack'; targetId: string; threadId?: string; + /** Opt in to posting at the conversation root instead of inside a thread. + * Written explicitly so a job that simply has not been given a thread yet + * is distinguishable from one whose audience really is the channel (#745). */ + scope?: 'channel_root'; } /** Channels one mention-watch job may cover. @@ -1501,6 +1505,7 @@ export function isHeartbeatDestination(value: unknown): value is HeartbeatDestin if (d['channel'] !== 'telegram' && d['channel'] !== 'discord' && d['channel'] !== 'slack') return false; if (typeof d['targetId'] !== 'string' || !d['targetId'].trim()) return false; if (d['threadId'] !== undefined && typeof d['threadId'] !== 'string') return false; + if (d['scope'] !== undefined && d['scope'] !== 'channel_root') return false; return true; } export interface HeartbeatFile { jobs: HeartbeatJob[] } diff --git a/src/discord/bot.ts b/src/discord/bot.ts index f9e21e130..1c3b64ad1 100644 --- a/src/discord/bot.ts +++ b/src/discord/bot.ts @@ -12,7 +12,6 @@ import { saveUpload, buildMediaPromptMany } from '../agent/spawn.js'; import { setLastActiveTarget, setLatestSeenTarget, - getLastActiveTarget, transportStarted, transportNotStarted, type TransportStartOutcome, @@ -692,7 +691,6 @@ async function installDiscordGeneration( if (settings["discord"]?.forwardAll !== false) { const forwarder = createDiscordForwarder({ client, - getLastTarget: () => getLastActiveTarget('discord'), shouldSkip: (data) => shouldSkipForwarding(data, 'discord'), log: ({ channelId, preview }) => { log.info(`[discord:forward] → ${channelId}: ${preview}...`); diff --git a/src/discord/forwarder.ts b/src/discord/forwarder.ts index a7dffae5f..67feeb518 100644 --- a/src/discord/forwarder.ts +++ b/src/discord/forwarder.ts @@ -12,6 +12,7 @@ import { asSendable } from './channel-types.js'; import { sendDiscordFile } from './discord-file.js'; import { redactOutboundText, logErrorText } from '../messaging/redact.js'; import { renderAgentErrorBlock } from '../messaging/error-block.js'; +import { resolveForwarderTarget } from '../messaging/forwarder-origin.js'; export async function relayDiscordImages( client: Client, @@ -65,7 +66,6 @@ export function chunkDiscordMessage(text: string, limit = DISCORD_MESSAGE_LIMIT) export function createDiscordForwarder(opts: { client: Client; - getLastTarget: () => RemoteTarget | null; shouldSkip?: (data: Record) => boolean; log?: (info: { channelId: string; preview: string }) => void; prefix?: string; @@ -78,7 +78,7 @@ export function createDiscordForwarder(opts: { const errorBlock = data["error"] ? renderAgentErrorBlock(data) : null; if (data["error"] && !errorBlock) return; if (opts.shouldSkip?.(data)) return; - const target = opts.getLastTarget(); + const target = resolveForwarderTarget(data, 'discord'); if (!target?.targetId || !opts.client) return; try { const channel = await opts.client.channels.fetch(target.targetId); diff --git a/src/memory/heartbeat-destination.ts b/src/memory/heartbeat-destination.ts new file mode 100644 index 000000000..46440879c --- /dev/null +++ b/src/memory/heartbeat-destination.ts @@ -0,0 +1,74 @@ +// ─── Heartbeat destination binding ─────────────────── +// A scheduled report is written for one conversation. An operator picked it; +// nothing about the job says "wherever someone last spoke". Yet a job with no +// stored destination used to fall through to the active-channel path, and a job +// that stored only a channel was delivered to that channel's root with no way to +// tell an intentional root post from a half-filled form (#437, #745). +// +// So a destination is either complete or it is held. Complete means a thread — +// the conversation the operator pointed at — or an explicit `channel_root` +// saying the channel itself is the intended audience. A job that says neither +// has not been configured yet, and the honest response to that is silence with +// a reason in the log, not a guess that lands in someone else's thread. + +import { isHeartbeatDestination, type HeartbeatDestination } from '../core/config.js'; +import { targetFromChatId } from '../messaging/send.js'; +import type { RemoteTarget } from '../messaging/types.js'; + +export type HeartbeatHoldReason = + | 'unbound_destination' + | 'incomplete_destination' + | 'malformed_destination'; + +export type HeartbeatBinding = + | { state: 'bound'; target: RemoteTarget } + | { state: 'held'; reason: HeartbeatHoldReason }; + +/** True when a stored destination names a conversation precisely enough to send. + * + * Slack is the strict case because Slack is where threads exist: either a + * non-empty `threadId`, or `scope: 'channel_root'` written on purpose. An + * empty-string `threadId` is not a thread; it used to pass validation and then + * read as falsy at send time, quietly turning a threaded job into a root post. */ +export function isCompleteHeartbeatDestination(value: unknown): value is HeartbeatDestination { + if (!isHeartbeatDestination(value)) return false; + const dest = value as HeartbeatDestination; + if (dest.threadId !== undefined && !dest.threadId.trim()) return false; + if (dest.channel !== 'slack') return true; + return Boolean(dest.threadId?.trim()) || dest.scope === 'channel_root'; +} + +/** + * Resolve a stored destination into the one target this job may use. + * + * There is deliberately no "no destination" success case. The previous contract + * had one, and it is the reason a report reached a conversation that had never + * asked for it. + */ +export function resolveHeartbeatBinding(destination: unknown): HeartbeatBinding { + if (destination === undefined || destination === null) { + return { state: 'held', reason: 'unbound_destination' }; + } + if (!isHeartbeatDestination(destination)) { + return { state: 'held', reason: 'malformed_destination' }; + } + if (!isCompleteHeartbeatDestination(destination)) { + return { state: 'held', reason: 'incomplete_destination' }; + } + const dest = destination as HeartbeatDestination; + const base = targetFromChatId(dest.channel, dest.targetId); + const threadId = dest.threadId?.trim(); + return { state: 'bound', target: threadId ? { ...base, threadId } : base }; +} + +/** Operator-facing explanation. Never includes tokens or report content. */ +export function heartbeatHoldMessage(reason: HeartbeatHoldReason): string { + switch (reason) { + case 'unbound_destination': + return 'no destination configured — set a channel and thread before enabling'; + case 'incomplete_destination': + return 'destination names a channel but no thread — add a thread, or set scope:"channel_root" to post to the channel itself'; + case 'malformed_destination': + return 'destination is malformed'; + } +} diff --git a/src/memory/heartbeat.ts b/src/memory/heartbeat.ts index af0c7e83b..3f90e9bf3 100644 --- a/src/memory/heartbeat.ts +++ b/src/memory/heartbeat.ts @@ -13,9 +13,10 @@ import { hasPendingWorkerReplays } from '../orchestrator/worker-registry.js'; import { broadcast } from '../core/bus.js'; import { sendChannelOutput, targetFromChatId } from '../messaging/send.js'; import { nextDeliverySeq, wasSelfDelivered } from '../messaging/turn-delivery.js'; -import { isHeartbeatDestination, isHeartbeatMentionWatch } from '../core/config.js'; -import type { HeartbeatDestination, HeartbeatMentionWatch } from '../core/config.js'; +import { isHeartbeatMentionWatch } from '../core/config.js'; +import type { HeartbeatMentionWatch } from '../core/config.js'; import { runMentionWatchTick } from './heartbeat-mention-watch.js'; +import { resolveHeartbeatBinding, heartbeatHoldMessage, type HeartbeatBinding } from './heartbeat-destination.js'; import { watchNamespace } from './mention-watch-ledger.js'; import { detectLegacyMentionWatch, isQuarantined } from './legacy-mention-watch-quarantine.js'; import { verifiedSlackWorkspace } from '../slack/verified-workspace.js'; @@ -77,25 +78,19 @@ export function isHeartbeatQuietOutput(result: string, extraMarkers: string[] = return ['[SILENT]', ...extraMarkers].some(marker => marker.length > 0 && result.includes(marker)); } -/** Turn a stored destination into a send target. +/** Turn a stored destination into the one target this job may use. * - * The stored shape carries only what an operator can reasonably know: which - * transport, which conversation, and optionally which thread. `targetKind` and - * `peerKind` are derived from the id — Slack's C/D/G prefixes decide them — so - * `targetFromChatId` owns that mapping rather than the heartbeat file. + * `targetKind` and `peerKind` are derived from the id — Slack's C/D/G prefixes + * decide them — so `targetFromChatId` owns that mapping rather than the + * heartbeat file. * - * `pinned` distinguishes the two ways this returns no target, because they must - * not be delivered the same way. A job that never named a destination keeps the - * legacy active-channel path. A job that DID name one but wrote it wrong has - * stated an intent the resolver cannot satisfy — falling back there would send - * a report meant for one channel to whoever spoke last, which is the failure - * this whole change exists to stop (#437). */ -export function heartbeatTarget(destination: unknown): { pinned: boolean; target: RemoteTarget | null } { - if (destination === undefined || destination === null) return { pinned: false, target: null }; - if (!isHeartbeatDestination(destination)) return { pinned: true, target: null }; - const dest = destination as HeartbeatDestination; - const target = targetFromChatId(dest.channel, dest.targetId); - return { pinned: true, target: dest.threadId ? { ...target, threadId: dest.threadId } : target }; + * There is no longer a "no destination, send anyway" outcome. #437 closed the + * case where a malformed destination fell back to the active channel but left + * the absent one open, and an absent destination is the same failure wearing + * less: a report meant for somewhere specific delivered to whoever spoke last + * (#745). Every way of not naming a conversation now holds the send. */ +export function heartbeatTarget(destination: unknown): HeartbeatBinding { + return resolveHeartbeatBinding(destination); } function pendingSnapshot(reason?: HeartbeatPendingReason, policy?: HeartbeatPendingPolicy) { @@ -245,9 +240,12 @@ async function runMentionWatchJob(job: Record, watch: HeartbeatMent log.error(`[heartbeat:${job["name"]}] mention watch needs Slack enabled with a bot token`); return false; } - const { pinned, target } = heartbeatTarget(job["destination"]); - if (pinned && !target) { - log.error(`[heartbeat:${job["name"]}] malformed destination — mention watch not run`); + // A mention watch answers the thread it found, so it needs no destination of + // its own. A destination that IS stored still has to be readable: a broken + // one means the operator meant something this code cannot honour. + const binding = heartbeatTarget(job["destination"]); + if (binding.state === 'held' && binding.reason !== 'unbound_destination') { + log.error(`[heartbeat:${job["name"]}] refuse: ${binding.reason} — mention watch not run`); return false; } @@ -548,26 +546,20 @@ export async function runHeartbeatJob(job: Record) { log.info(`[heartbeat:${job["name"]}] response: ${result.slice(0, 80)}`); - // A job with a destination goes THERE and nowhere else. Without one the - // send falls back to whichever conversation spoke to the bot most - // recently, which is not a property of this job at all — that is how two - // scheduled reports landed in an unrelated design thread (#437). Jobs - // with no destination keep the legacy behaviour on purpose: defaulting - // them to "do not send" would silence every existing install. - const { pinned, target } = heartbeatTarget(job["destination"]); - if (pinned && !target) { - // Stated an intent we cannot honour. Refusing is the point: delivering - // to the active channel would put this report wherever the last - // conversation happened to be. - log.error(`[heartbeat:${job["name"]}] malformed destination — not delivered`); + // A job goes to the conversation it names and nowhere else. The + // active-channel fallback that used to stand in for a missing + // destination is gone: it delivered to whoever spoke to the bot most + // recently, which is not a property of this job at all (#437, #745). + const binding = heartbeatTarget(job["destination"]); + if (binding.state === 'held') { + log.error(`[heartbeat:${job["name"]}] refuse: ${binding.reason} — ${heartbeatHoldMessage(binding.reason)}`); } const sendResult = !decision.send ? { ok: true as const } - : pinned - ? (target - ? await sendChannelOutput({ channel: target.channel, type: 'text', text: formatted, target, allowActiveFallback: false }) - : { ok: false as const, error: 'invalid heartbeat destination' }) - : await sendChannelOutput({ channel: 'active', type: 'text', text: formatted }); + : binding.state === 'bound' + ? await sendChannelOutput({ channel: binding.target.channel, type: 'text', text: formatted, + target: binding.target, allowActiveFallback: false }) + : { ok: false as const, error: `heartbeat destination held: ${binding.reason}` }; if (!sendResult.ok) { log.error(`[heartbeat:${job["name"]}] send failed: ${sendResult.error}`); } @@ -577,7 +569,9 @@ export async function runHeartbeatJob(job: Record) { const now = Date.now(); try { insertHeartbeatAnchor.run( - job["id"], job["name"], settings["workingDir"], target?.channel ?? 'active', target?.targetId ?? null, + job["id"], job["name"], settings["workingDir"], + binding.state === 'bound' ? binding.target.channel : 'active', + binding.state === 'bound' ? binding.target.targetId : null, job["prompt"], decision.delivered ? formatted : `[quiet] ${formatted}`, now, decision.delivered ? now : null, ); } catch (e) { diff --git a/src/messaging/forwarder-origin.ts b/src/messaging/forwarder-origin.ts index 932234ff6..d7a8a2296 100644 --- a/src/messaging/forwarder-origin.ts +++ b/src/messaging/forwarder-origin.ts @@ -18,6 +18,8 @@ // `orchestrate_done`. So a job that decided it had nothing to say, or whose // output was meant to be suppressed by `reportPolicy`, had already leaked. +import type { MessengerChannel, RemoteTarget } from './types.js'; + /** Origins whose output is delivered by their own producer, not by a channel * forwarder. Keep this list small and specific: the forwarder failing open is * what makes web/CLI turns visible at all. */ @@ -37,3 +39,42 @@ export function shouldSkipForwarding( if (origin === ownChannel) return true; return PRODUCER_OWNED_ORIGINS.has(origin); } + +// ─── Request-scoped destination ────────────────────── +// A forwarder used to ask "where was this channel last spoken to?" and post +// there. That question has nothing to do with the run that just finished. On +// 2026-09-11 a mention arrived while an unrelated web run was working; the +// mention overwrote the last-active slot, the web run exited, and its answer — +// an internal ticket summary — was posted into the conversation that had just +// asked something else entirely (#742). +// +// The destination now travels WITH the run. `agent_done` carries the target +// captured when the run started, and a forwarder delivers to that target or to +// nowhere. A run that never had a destination on this channel (a web or CLI +// turn) is not homeless — its answer is already on the surface the user is +// looking at. Posting it to a chat room was always a guess. + +function isRemoteTargetLike(value: unknown): value is RemoteTarget { + if (!value || typeof value !== 'object') return false; + const candidate = value as Partial; + return typeof candidate.channel === 'string' + && typeof candidate.targetId === 'string' + && candidate.targetId.length > 0; +} + +/** + * The destination this `agent_done` is bound to on `ownChannel`, or null. + * + * Null means DO NOT SEND. It must never be read as "fall back to something": + * every fallback available here (last-active, latest-seen, configured home) is + * a global slot that a concurrent conversation can move, which is the bug. + */ +export function resolveForwarderTarget( + data: Record, + ownChannel: MessengerChannel, +): RemoteTarget | null { + const bound = data['target']; + if (!isRemoteTargetLike(bound)) return null; + if (bound.channel !== ownChannel) return null; + return bound; +} diff --git a/src/messaging/run-pin.ts b/src/messaging/run-pin.ts new file mode 100644 index 000000000..e85e88613 --- /dev/null +++ b/src/messaging/run-pin.ts @@ -0,0 +1,65 @@ +// ─── Run pin ───────────────────────────────────────── +// A run's identity, captured when it is admitted and carried on every terminal +// event it emits. Two separate failures made this necessary on 2026-09-11. +// +// The destination one: `agent_done` said what was said but not who it was for, +// so each channel forwarder answered "who for?" from a global last-active slot. +// A mention that arrived mid-run moved that slot, and an unrelated run's answer +// followed it into the wrong conversation (#742). +// +// The identity one: the print exit path broadcast `agent_done` with nothing but +// `origin`, and the Slack matcher treated every absent field as agreement. An +// event that cannot say which run produced it could therefore be adopted by a +// waiter that could (#743). +// +// Both are the same mistake in different clothes: inferring a run's context +// from process-global state instead of carrying it. A pin is carried. + +import type { RemoteTarget } from './types.js'; + +export interface RunPin { + origin?: string | undefined; + requestId?: string | undefined; + scope?: string | undefined; + sessionId?: string | undefined; + remoteKey?: string | undefined; + target?: RemoteTarget | undefined; +} + +/** + * The identity block to spread into a terminal event payload. + * + * Absent values are omitted rather than sent as `undefined`, because consumers + * distinguish "this run has no remote destination" from "this producer forgot + * to say". The target is copied: a payload that aliased the live run meta would + * change under a reader when the next turn overwrote it. + */ +export function runPinFields(pin: RunPin): Record { + const fields: Record = {}; + if (pin.origin) fields['origin'] = pin.origin; + if (pin.requestId) fields['requestId'] = pin.requestId; + if (pin.scope) fields['scope'] = pin.scope; + if (pin.sessionId) fields['sessionId'] = pin.sessionId; + if (pin.remoteKey) fields['remoteKey'] = pin.remoteKey; + if (pin.target && typeof pin.target.targetId === 'string' && pin.target.targetId) { + fields['target'] = { ...pin.target }; + } + return fields; +} + +/** True when the payload carries the execution identity a waiter can check. */ +export function hasRunIdentity(data: Record): boolean { + return typeof data['scope'] === 'string' && typeof data['sessionId'] === 'string'; +} + +/** + * True when two runs belong to the same conversation. + * + * A remote key is the conversation. If either side has one they must agree; a + * web turn (no key) and a Slack thread (key) are never the same conversation, + * which is what makes it wrong to steer one with the other. + */ +export function sameRunConversation(a: RunPin, b: RunPin): boolean { + if (a.remoteKey || b.remoteKey) return a.remoteKey === b.remoteKey; + return (a.origin ?? '') === (b.origin ?? ''); +} diff --git a/src/messaging/send.ts b/src/messaging/send.ts index ec438d033..a99ffee61 100644 --- a/src/messaging/send.ts +++ b/src/messaging/send.ts @@ -379,9 +379,11 @@ function authorizeExplicitTarget(target: RemoteTarget, channel: MessengerChannel // for a list nobody can parse (#406). if (channel !== 'slack' || slackAllowlist().ids.length) return null; for (const known of [getLastActiveTarget('slack'), getLatestSeenTarget('slack')]) { - if (known && sameSlackDestination(target, known)) { - return target.threadId == null && known.threadId != null ? known : target; - } + // Vouching decides WHETHER this send is allowed, never WHERE it goes. + // Returning `known` here rewrote an explicitly addressed channel-root + // send into whichever thread had spoken most recently — a caller that + // named its destination correctly still had it moved (#745). + if (known && sameSlackDestination(target, known)) return target; } // With no configured allowlist, the two slots above are the only conversations // this process can vouch for — and both hold whatever spoke MOST RECENTLY. So an diff --git a/src/routes/heartbeat.ts b/src/routes/heartbeat.ts index 311b48ffe..d5920f614 100644 --- a/src/routes/heartbeat.ts +++ b/src/routes/heartbeat.ts @@ -3,6 +3,7 @@ import type { AuthMiddleware } from './types.js'; import { loadHeartbeatFile, saveHeartbeatFile, isHeartbeatDestination, isHeartbeatMentionWatch, settings } from '../core/config.js'; import type { HeartbeatDestination, HeartbeatMentionWatch, HeartbeatJob } from '../core/config.js'; import { startHeartbeat } from '../memory/heartbeat.js'; +import { isCompleteHeartbeatDestination, resolveHeartbeatBinding } from '../memory/heartbeat-destination.js'; import { validateHeartbeatScheduleInput } from '../memory/heartbeat-schedule.js'; import { approveLegacyFreshStart, quarantineState, detectLegacyMentionWatch, isQuarantined } from '../memory/legacy-mention-watch-quarantine.js'; import { verifiedSlackWorkspace } from '../slack/verified-workspace.js'; @@ -33,9 +34,25 @@ export function resolveHeartbeatDestination( const raw = job['destination']; if (raw === null) return { ok: true, destination: undefined }; if (!isHeartbeatDestination(raw)) return { ok: false, error: 'invalid heartbeat destination' }; + // A supplied destination must say WHERE, not just which room. Inheritance + // above keeps existing half-filled jobs loadable; this rejects writing a new + // one, so the gap closes as jobs are edited instead of being re-saved + // forever with no thread (#745). + if (!isCompleteHeartbeatDestination(raw)) { + return { ok: false, error: 'heartbeat destination needs a threadId, or scope:"channel_root" to post at the conversation root' }; + } return { ok: true, destination: raw }; } +/** Surface a stored destination a tick will refuse, so an operator can see the + * hold instead of waiting for a report that never arrives. Mention-watch jobs + * answer the thread they find and need no destination of their own. */ +function heldForDestination(job: HeartbeatJob): (HeartbeatJob & { held: string }) | null { + if (job.mentionWatch) return null; + const binding = resolveHeartbeatBinding(job.destination); + return binding.state === 'held' ? { ...job, held: binding.reason } : null; +} + /** Resolve the mention-watch a PUT should persist. * * Same inheritance rule as `destination`, for the same reason: every shipped UI @@ -102,7 +119,7 @@ export function registerHeartbeatRoutes(app: Express, requireAuth: AuthMiddlewar ...file, jobs: file.jobs.map(job => (job.mentionWatch && job.id && isQuarantined(job.id) ? { ...job, held: 'unmigrated_mention_watch_ledger' as const } - : job)), + : heldForDestination(job) ?? job)), }); }); diff --git a/src/slack/bot.ts b/src/slack/bot.ts index 1adf135ed..b3c9556fb 100644 --- a/src/slack/bot.ts +++ b/src/slack/bot.ts @@ -18,7 +18,7 @@ import { orchestrateAndCollectData } from '../orchestrator/collect.js'; import { isResetIntent } from '../orchestrator/pipeline.js'; import { isContinueIntent } from '../orchestrator/parser.js'; import { - setLastActiveTarget, setLatestSeenTarget, getLastActiveTarget, + setLastActiveTarget, setLatestSeenTarget, revokeMessagingTransport, startMessagingTransport, transportStarted, transportNotStarted, type TransportInitContext, type TransportStartOutcome, } from '../messaging/runtime.js'; @@ -1821,7 +1821,6 @@ async function runSlackInit(ctx?: TransportInitContext): Promise getSlackSendClient().token, - getLastTarget: () => getLastActiveTarget('slack'), shouldSkip: (data) => shouldSkipForwarding(data, 'slack'), }); addBroadcastListener(forwarderHandler); diff --git a/src/slack/forwarder.ts b/src/slack/forwarder.ts index a6e051c4f..0c59b5af8 100644 --- a/src/slack/forwarder.ts +++ b/src/slack/forwarder.ts @@ -13,6 +13,7 @@ import { sendSlackFile } from './slack-file.js'; import { sendSlackText } from './send-only-client.js'; import { logErrorText } from '../messaging/redact.js'; import { renderAgentErrorBlock } from '../messaging/error-block.js'; +import { resolveForwarderTarget } from '../messaging/forwarder-origin.js'; export async function relaySlackImages( token: string, @@ -52,7 +53,6 @@ export async function relaySlackImages( export function createSlackForwarder(opts: { getToken: () => string | null; - getLastTarget: () => RemoteTarget | null; shouldSkip?: (data: Record) => boolean; log?: (info: { channelId: string; preview: string }) => void; prefix?: string; @@ -67,7 +67,9 @@ export function createSlackForwarder(opts: { const errorBlock = data["error"] ? renderAgentErrorBlock(data) : null; if (data["error"] && !errorBlock) return; if (opts.shouldSkip?.(data)) return; - const target = opts.getLastTarget(); + // The run's own destination, or nothing. No last-active fallback: that + // slot belongs to whoever spoke most recently, not to this run (#742). + const target = resolveForwarderTarget(data, 'slack'); const token = opts.getToken(); if (!target?.targetId || !token) return; try { diff --git a/src/telegram/bot.ts b/src/telegram/bot.ts index cfefc190b..8cb4b8fa3 100644 --- a/src/telegram/bot.ts +++ b/src/telegram/bot.ts @@ -28,7 +28,7 @@ import { applyRuntimeSettingsPatch } from '../core/runtime-settings.js'; import { resetEmployeeSessions, seedDefaultEmployees } from '../core/employees.js'; import { handleVoice } from './voice.js'; import { - getLastActiveTarget, registerTransport, setLastActiveTarget, setLatestSeenTarget, + registerTransport, setLastActiveTarget, setLatestSeenTarget, transportNotStarted, transportStarted, type TransportStartOutcome, } from '../messaging/runtime.js'; import { @@ -159,11 +159,6 @@ const telegramForwarderLifecycle = createForwarderLifecycle({ removeListener: removeBroadcastListener, buildForwarder: ({ bot }: Record) => createTelegramForwarder({ bot: bot as Bot, - getLastChatId: () => { - const chatIds = Array.from(telegramActiveChatIds); - return chatIds.length ? (chatIds[chatIds.length - 1] ?? null) : null; - }, - getLastTarget: () => getLastActiveTarget('telegram'), // Own origin: handled by tgOrchestrate already. Producer-owned origins // (heartbeat) deliver to their own destination — see forwarder-origin.ts. shouldSkip: (data: Record) => shouldSkipForwarding(data, 'telegram'), diff --git a/src/telegram/forwarder.ts b/src/telegram/forwarder.ts index 1840230c2..d624c6b62 100644 --- a/src/telegram/forwarder.ts +++ b/src/telegram/forwarder.ts @@ -109,6 +109,7 @@ import { stripUndefined } from '../core/strip-undefined.js'; import { extractLocalImagePaths } from '../messaging/extract-images.js'; import { threadIdNumber } from '../messaging/thread-target.js'; import type { RemoteTarget } from '../messaging/types.js'; +import { resolveForwarderTarget } from '../messaging/forwarder-origin.js'; import { assertSendFilePath } from '../security/path-guards.js'; import { sendTelegramMarkdown } from './rich-message.js'; import { sendTelegramFile, validateFileSize } from './telegram-file.js'; @@ -123,8 +124,6 @@ interface ForwarderLifecycleOptions { interface TelegramForwarderOptions { bot: Bot; - getLastChatId: () => string | number | null | undefined; - getLastTarget?: () => RemoteTarget | null; shouldSkip?: (data: Record) => boolean; log?: (info: { chatId: string | number; preview: string }) => void; prefix?: string; @@ -202,8 +201,6 @@ export function createForwarderLifecycle({ */ export function createTelegramForwarder({ bot, - getLastChatId, - getLastTarget, shouldSkip = (_data: Record) => false, log = (_info: { chatId: string | number; preview: string }) => { }, prefix = '📡 ', @@ -215,10 +212,11 @@ export function createTelegramForwarder({ if (data["error"] && !isUserSafeWatchdogDiagnostic(String(data["text"]))) return; if (shouldSkip(data)) return; - const candidateTarget = getLastTarget?.() ?? null; - const target = candidateTarget?.channel === 'telegram' ? candidateTarget : null; - const chatId = target?.targetId - ?? (typeof getLastChatId === 'function' ? getLastChatId() : null); + // The run's own destination only. `getLastTarget`/`getLastChatId` + // answer "who spoke here most recently", which a concurrent + // conversation moves out from under a running turn (#742). + const target = resolveForwarderTarget(data, 'telegram'); + const chatId = target?.targetId ?? null; if (!chatId) return; const text = String(data["text"]); diff --git a/structure/str_func.md b/structure/str_func.md index 31c54b4e4..dd9993cd2 100644 --- a/structure/str_func.md +++ b/structure/str_func.md @@ -38,7 +38,7 @@ cli-jaw/ │ └── mime-detect.ts ← MIME 타입 감지 헬퍼 (67L) ├── src/ │ ├── core/ ← 의존 0 인프라 계층 (31 files, 3847L) -│ │ ├── config.ts ← JAW_HOME, settings, APP_VERSION + migrateSettings legacy Claude model normalization + avatar settings deep merge + default `settings.pi` + corrupt settings backup + CLI 탐지 re-export hub (1571L) +│ │ ├── config.ts ← JAW_HOME, settings, APP_VERSION + migrateSettings legacy Claude model normalization + avatar settings deep merge + default `settings.pi` + corrupt settings backup + CLI 탐지 re-export hub (1576L) │ │ ├── cli-detection.ts ← CLI 탐지 + `pi` npm-exec fallback + `kiro-code`(`kiro-cli` binary) 탐지 + local package release/debug candidates (57L) │ │ ├── compact.ts ← compact 헬퍼 (COMPACT_MARKER_CONTENT, managed summary builder, cutoff logic, harvestGitGrep + harvestChatGrep 1KB/1KB budget split) (782L) │ │ ├── instance.ts ← 인스턴스 ID, node/jaw 경로, 유닛명 sanitize (61L) @@ -142,10 +142,10 @@ cli-jaw/ │ │ ├── merge-tool-log.ts ← exact run/item latest merge with terminal precedence and omission preservation (78L) │ │ ├── runtime-pool-contract.ts ← type-only shared store/lease/provider access contract (49L) │ │ ├── claude-runtime-pool.ts ← Claude acquisition and physical/logical retirement over shared stores (358L) -│ │ ├── claude-runtime-run.ts ← native Claude main adaptation to shared host/lifecycle and fallback terminal ordering (293L) +│ │ ├── claude-runtime-run.ts ← native Claude main adaptation to shared host/lifecycle and fallback terminal ordering (295L) │ │ ├── prompt-context.ts ← history/operational/partial boundaries and bounded accepted Cursor redirects (201L) │ │ ├── steer-input-guard.ts ← transient scoped Stop fence through fallback enqueue (33L) -│ │ ├── spawn.ts ← CLI spawn + ACP/Codex App/Pi RPC/AGY/Kiro plain text/log session capture + v2 SQLite session resume + 큐 + 메모리 flush + 429 retry timer + isAgentBusy/isSteerInProgress + buildHistoryBlock compact cutoff + working_dir scoping + enqueue→processQueue race fix + QueueItem persistent DB queue + makeCleanEnv PATH augment (4093L) +│ │ ├── spawn.ts ← CLI spawn + ACP/Codex App/Pi RPC/AGY/Kiro plain text/log session capture + v2 SQLite session resume + 큐 + 메모리 flush + 429 retry timer + isAgentBusy/isSteerInProgress + buildHistoryBlock compact cutoff + working_dir scoping + enqueue→processQueue race fix + QueueItem persistent DB queue + makeCleanEnv PATH augment (4096L) │ │ ├── spawn/ ← spawn 서브모듈 (3 files) │ │ │ ├── queue.ts ← QueueItem persistent DB queue + processQueue race fix + enqueue/dequeue + drainRecoveredQueue (부팅 시 복구 큐 기동, server.ts가 transport 준비 후 호출) + `_fromQueue` 표식 (대기자 없는 턴을 채널이 답할 수 있게) (703L) │ │ │ ├── resume.ts ← session resume logic + stale resume detection (117L) @@ -169,7 +169,7 @@ cli-jaw/ │ │ ├── agy-capabilities.ts ← AGY `--help`/`--version` capability probe + cached optional flag support map + legacy emit-all fallback marker (124L) │ │ ├── agy-transcript-watcher.ts ← AGY transcript/log watcher and session-id extraction support (291L) │ │ ├── pi-runtime.ts ← Pi profile 정규화 + isolated `PI_CODING_AGENT_DIR` models/settings 생성 + `pi --offline --list-models` discovery + `pi --mode rpc` JSONL parser/spawner (단일 launch/reader/writer 소유자) (1149L) ✨ -│ │ ├── lifecycle-handler.ts ← child lifecycle + fallback/retry + queue resume orchestration + clearEmployeeSession on resume failure + stale resume fresh retry + kickGoalContinuation export + clearGoalTimers + goal continuation boundary row (1424L) +│ │ ├── lifecycle-handler.ts ← child lifecycle + fallback/retry + queue resume orchestration + clearEmployeeSession on resume failure + stale resume fresh retry + kickGoalContinuation export + clearGoalTimers + goal continuation boundary row (1434L) │ │ ├── kiro-auth.ts ← Kiro CLI auth store reader (resolveKiroDataPath, readKiroAuthFromStore, resolveKiroProfileArn, regionFromProfileArn, listKiroConversationIdsForCwd, resolveKiroSessionIdAfterSpawn, extractKiroSessionIdFromV2Store) (314L) │ │ ├── kiro-models.ts ← Kiro live model inventory (KiroModelEntry, KiroModelInventory, parseKiroModelListJson, fetchKiroModelInventory) (98L) │ │ ├── kiro-runtime.ts ← Kiro plain-text stdout parser + session capture (isKiroPlainTextCli, processKiroStdoutChunk, flushKiroStdoutContext, appendKiroStdoutChunk, captureKiroSessionIdAfterExit, stripKiroAnsi, parseKiroAssistantText, isKiroStaleSessionOutput, isKiroResumeDegradedOutput, KiroStreamEvent, KiroStdoutContext) (447L) @@ -196,7 +196,7 @@ cli-jaw/ │ │ └── events.ts ← legacy re-export stub → events/ 모듈 (15L) │ ├── messaging/ ← 통합 메시징 런타임 (33 files) │ │ ├── runtime.ts ← 채널 lifecycle (init/shutdown/restart) + transport registry (333L) -│ │ ├── send.ts ← 통합 아웃바운드 메시지 라우팅 (ChannelSendRequest, 다중 채널 send 지원, 턴 주소 우선순위) (562L) +│ │ ├── send.ts ← 통합 아웃바운드 메시지 라우팅 (ChannelSendRequest, 다중 채널 send 지원, 턴 주소 우선순위) (564L) │ │ ├── turn-conversation.ts ← 턴이 답하는 대화 주소 encode/decode + 채널 매칭 (60L) ✨ │ │ ├── turn-delivery.ts ← 에이전트 자가 전송 claim (턴 앵커 + digest, 소비형) → dispatch 중복 게시 억제 (252L) ✨ │ │ ├── dedupe.ts ← 배달 중복 제거 (TTL seen-set, 미만료 항목 보존) (118L) ✨ @@ -224,7 +224,7 @@ cli-jaw/ │ │ ├── inbound-envelope.ts ← InboundEnvelope normalizers (229L) │ │ ├── ack-reaction.ts ← inbound ACK reaction lifecycle (serialized transitions + per-channel defaults + nested ack merge) (269L) │ │ ├── queue-notice.ts ← queue-notice lifecycle (deferred close + bind race drain + bounded shutdown registry) (208L) -│ │ ├── forwarder-origin.ts ← channel forwarder 공통 origin 필터 (own channel + producer-owned heartbeat skip) (39L) +│ │ ├── forwarder-origin.ts ← channel forwarder 공통 origin 필터 (own channel + producer-owned heartbeat skip) (80L) │ │ ├── native-body.ts ← native runtime terminal 태그 술어 + 배달 정책 래퍼 (3봇+collector 공유) (32L) ✨ │ │ ├── file-receipt.ts ← 파일 전송 confirmation 어휘 (confirmed|unconfirmed) + 채널별 unconfirmed 에러코드 (53L) ✨ │ │ ├── queue-notice-record.ts ← durable notice 기록 best-effort 래퍼 factory (channel+logPrefix) (59L) ✨ @@ -318,7 +318,7 @@ cli-jaw/ │ ├── memory/ ← 데이터 영속화 + advanced memory runtime (17 files) │ │ ├── advanced.ts ← Advanced Memory re-export stub (1L) │ │ ├── bootstrap.ts ← legacy memory/bootstrap import + structured root 초기화 (588L) -│ │ ├── heartbeat.ts ← Heartbeat 잡 스케줄 + cron/every timer orchestration + minute-slot dedupe + fs.watch (659L) +│ │ ├── heartbeat.ts ← Heartbeat 잡 스케줄 + cron/every timer orchestration + minute-slot dedupe + fs.watch (653L) │ │ ├── heartbeat-schedule.ts ← Heartbeat schedule normalize + cron validate/match + timezone validate + immediate cron loop helper (410L) │ │ ├── heartbeat-mention-watch.ts ← Slack mention 항목 loop + busy yield + server-owned thread send + WatchNamespace 경유 ledger 접근 (249L) │ │ ├── mention-watch-ledger.ts ← v2 ledger 단일 접근 경로 (WatchNamespace = job+workspace+user, 모든 SQL이 3파트 predicate 유지, A/B 대칭 테스트가 최종 보증) (104L) ✨ @@ -336,15 +336,15 @@ cli-jaw/ │ ├── telegram/ ← Telegram 인터페이스 (11 files) │ │ ├── reactions.ts ← ACK reaction transport + ReactionTypeEmoji allowlist + notice transport (186L) │ │ ├── ipv4-fetch.ts ← IPv4 fetch factory that honours init.signal (destroys on abort) (106L) -│ │ ├── bot.ts ← Telegram 봇 + forwarder lifecycle + origin 필터링 + channel-origin text/image reply + elicitation callback + voice 핸들러 등록 (1422L) +│ │ ├── bot.ts ← Telegram 봇 + forwarder lifecycle + origin 필터링 + channel-origin text/image reply + elicitation callback + voice 핸들러 등록 (1417L) │ │ ├── voice.ts ← 음성 메시지 → guarded download → STT → tgOrchestrate 파이프라인 (43L) -│ │ ├── forwarder.ts ← text 전송 뒤 guarded local-image photo relay + escape/chunk/createForwarder (247L) +│ │ ├── forwarder.ts ← text 전송 뒤 guarded local-image photo relay + escape/chunk/createForwarder (245L) │ │ ├── rich-message.ts ← Bot API 10.1 rich-first send (sendTelegramMarkdown, 32k chunk, HTML/plaintext fallback) (425L) │ │ ├── elicitation-buttons.ts ← single_select elicitation → inline keyboard + pending store + callback codec (110L) │ │ ├── hub-callback.ts ← hub-member callback URL SSRF guard (19L) │ │ └── telegram-file.ts ← Telegram 파일 전송 + 재시도 + 사이즈 검증 (231L) │ ├── discord/ ← Discord 인터페이스 (8 files) -│ │ ├── bot.ts ← Discord 봇 + transport 등록 + message/attachment 핸들러 + channel-origin image relay (1034L) +│ │ ├── bot.ts ← Discord 봇 + transport 등록 + message/attachment 핸들러 + channel-origin image relay (1032L) │ │ ├── reactions.ts ← ACK reaction transport + cancellable notice REST (122L) │ │ ├── commands.ts ← Discord slash command 등록 + 핸들러 (153L) │ │ ├── send-only-client.ts ← Discord send-only client (webhook/DM fallback) (257L) ✨ @@ -353,7 +353,7 @@ cli-jaw/ │ │ └── discord-file.ts ← Discord 파일 전송 (107L) │ ├── slack/ ← Slack 인터페이스 (41 files, Socket Mode + Web API, SDK 없음) │ │ ├── socket.ts ← Socket Mode client (apps.connections.open → wss, ack-before-work, envelope dedupe TTL, hello deadline, backoff 재연결) (437L) -│ │ ├── bot.ts ← Slack 봇 lifecycle + attachPort 성공 후 best-effort 자기선출/영속화 + envelope routing + orchestrate 경로 + queued-result waiter + top-level/thread 1회 context prefetch (1937L) +│ │ ├── bot.ts ← Slack 봇 lifecycle + attachPort 성공 후 best-effort 자기선출/영속화 + envelope routing + orchestrate 경로 + queued-result waiter + top-level/thread 1회 context prefetch (1936L) │ │ ├── api.ts ← Slack Web API fetch wrapper (HTTP 200 + ok:false를 실패로 처리, credential/URL redaction, Retry-After) (442L) │ │ ├── format.ts ← CommonMark → mrkdwn 변환 + code-fence 보존 chunking (222L) │ │ ├── blocks.ts ← Block Kit validation and per-table message splitting (143L) @@ -374,7 +374,7 @@ cli-jaw/ │ │ ├── inbound-file.ts ← 인바운드 첨부 단일 IO owner (files.info → 인증 스트리밍 다운로드 → saveUpload, 파일/메시지 바이트 예산, 고정 error code) (280L) ✨ │ │ ├── inbound-url.ts ← 인바운드 다운로드 URL 검증 (Slack host allowlist + https-only hop + 사설망 거부) (44L) ✨ │ │ ├── send-only-client.ts ← bot-token outbound with separate transport/verification receipts (228L) -│ │ ├── forwarder.ts ← agent_done 포워딩 + guarded local-image relay (filename caption) (87L) +│ │ ├── forwarder.ts ← agent_done 포워딩 + guarded local-image relay (filename caption) (89L) │ │ ├── send-handler.ts ← ChannelSendRequest → Slack Web API 어댑터 + 413 텍스트 다운그레이드 (70L) │ │ ├── manifest.ts ← Slack 앱 표시명 검증 + bot 표시명 결정적 파생을 포함한 매니페스트 single source (`jaw slack manifest`/`setup`이 사용) (162L) │ │ ├── scope-status.ts ← OAuth grant drift 단일 소유자 (auth.test의 x-oauth-scopes를 manifest 요구 집합과 대조, 미관측을 '이상 없음'과 구분, doctor·health·identity 경고가 공유) (191L) ✨ @@ -426,7 +426,7 @@ cli-jaw/ │ │ ├── types.ts ← `AuthMiddleware` shared type (3L) │ │ ├── static.ts ← root/uploads/widgets + guarded local image/video `/api/image` 서빙 (160L) │ │ ├── employees.ts ← employee CRUD 라우트 (123L) -│ │ ├── heartbeat.ts ← heartbeat read/write 라우트 (305L) +│ │ ├── heartbeat.ts ← heartbeat read/write 라우트 (322L) │ │ ├── skills.ts ← skill list/enable/disable/reset 라우트 (90L) │ │ ├── jaw-memory.ts ← jaw memory search/read/list/save/init/reflect/flush/soul/soul-activate/bootstrap 라우트 (362L) │ │ ├── jaw-ceo.ts ← Jaw CEO channel/session support routes (321L) ✨ diff --git a/structure/telegram.md b/structure/telegram.md index 83cb24462..e45c2e62b 100644 --- a/structure/telegram.md +++ b/structure/telegram.md @@ -24,6 +24,31 @@ Auto instances authorize qualified direct-local Jaw API calls without manually c ## 공통 메시징 레이어 +### Request-scoped destination (#742/#745) + +A completion event carries the conversation it belongs to. `src/messaging/run-pin.ts` +captures `origin`, `requestId`, `scope`, `sessionId`, `remoteKey` and the admitted +`target` once, at spawn; every `agent_done` spreads that block. `resolveForwarderTarget` +in `src/messaging/forwarder-origin.ts` reads the destination off the event and returns +null when it is missing or names another channel — null means do not send, never +"fall back to something". The Slack, Discord and Telegram forwarders take no +`getLastTarget`/`getLastChatId` option, so the last-active lookup cannot return. + +Consequence: a web or CLI turn is no longer mirrored into a chat room. Its answer is +on the surface that asked for it. Mirroring guessed a room from a global slot that any +concurrent conversation could move, which is how a web run's internal ticket summary +reached a Slack thread that had asked something else (#742). + +Heartbeat destinations follow the same rule through +`src/memory/heartbeat-destination.ts`: `resolveHeartbeatBinding` returns `bound` or +`held`. Slack needs a non-empty `threadId` or an explicit `scope: "channel_root"`; an +absent destination is `unbound_destination` and sends nothing. `GET /api/heartbeat` +surfaces the hold and `PUT` refuses to write a new incomplete Slack destination while +still inheriting an existing one. `authorizeExplicitTarget` in `src/messaging/send.ts` +vouches for a send without rewriting its address: it no longer returns the last-active +target's thread for an explicitly addressed channel-root post. + + ### Slack group DMs and scope observations The manifest subscribes to [`message.mpim`](https://docs.slack.dev/reference/events/message.mpim/) diff --git a/tests/telegram-forwarding.test.ts b/tests/telegram-forwarding.test.ts index 99ca313fd..4c94c4c2e 100644 --- a/tests/telegram-forwarding.test.ts +++ b/tests/telegram-forwarding.test.ts @@ -52,6 +52,13 @@ function createBotSpy({ failHtmlOnce = false } = {}) { }; } +/** The destination a run was admitted with. Forwarders read it off the event + * rather than asking the transport who spoke most recently (#742). */ +function tgTarget(targetId, threadId) { + return { channel: 'telegram', targetKind: 'channel', peerKind: 'group', + targetId: String(targetId), ...(threadId ? { threadId } : {}) }; +} + function flush() { return new Promise((resolve) => setImmediate(resolve)); } @@ -60,16 +67,15 @@ test('forwarder skips telegram-origin responses', async () => { const { bot, sent } = createBotSpy(); const forward = createTelegramForwarder({ bot, - getLastChatId: () => 123, shouldSkip: (data) => data.origin === 'telegram', }); - forward('agent_done', { text: 'A', origin: 'telegram' }); - forward('agent_done', { text: 'B', origin: 'web' }); + forward('agent_done', { text: 'A', origin: 'telegram', target: tgTarget(123) }); + forward('agent_done', { text: 'B', origin: 'web', target: tgTarget(123) }); await flush(); assert.equal(sent.length, 1); - assert.equal(sent[0].chatId, 123); + assert.equal(sent[0].chatId, '123'); assert.equal(sent[0].opts?.parse_mode, 'HTML'); assert.equal(sent[0].text, '📡 B'); }); @@ -78,10 +84,9 @@ test('forwarder skips error responses', async () => { const { bot, sent } = createBotSpy(); const forward = createTelegramForwarder({ bot, - getLastChatId: () => 123, }); - forward('agent_done', { text: 'error text', error: true, origin: 'web' }); + forward('agent_done', { text: 'error text', error: true, origin: 'web', target: tgTarget(123) }); await flush(); assert.equal(sent.length, 0); }); @@ -90,13 +95,13 @@ test('forwarder sends watchdog stall diagnostics even when marked error', async const { bot, sent } = createBotSpy(); const forward = createTelegramForwarder({ bot, - getLastChatId: () => 123, }); forward('agent_done', { text: '❌ ⏱️ 응답 없음 — unsafe AGY run_command broad home search', error: true, origin: 'web', + target: tgTarget(123), }); await flush(); @@ -109,10 +114,9 @@ test('forwarder falls back to plain text when HTML send fails', async () => { const { bot, sent } = createBotSpy({ failHtmlOnce: true }); const forward = createTelegramForwarder({ bot, - getLastChatId: () => 777, }); - forward('agent_done', { text: '**bold** ', origin: 'web' }); + forward('agent_done', { text: '**bold** ', origin: 'web', target: tgTarget(777) }); await flush(); assert.equal(sent.length, 2); @@ -127,14 +131,13 @@ test('forwarder handles mixed origin/error events deterministically', async () = const { bot, sent } = createBotSpy(); const forward = createTelegramForwarder({ bot, - getLastChatId: () => 456, shouldSkip: (data) => data.origin === 'telegram', }); - forward('agent_done', { text: 'skip telegram', origin: 'telegram' }); - forward('agent_done', { text: 'ok web', origin: 'web' }); - forward('agent_done', { text: 'skip error', origin: 'web', error: true }); - forward('agent_done', { text: 'ok cli', origin: 'cli' }); + forward('agent_done', { text: 'skip telegram', origin: 'telegram', target: tgTarget(456) }); + forward('agent_done', { text: 'ok web', origin: 'web', target: tgTarget(456) }); + forward('agent_done', { text: 'skip error', origin: 'web', error: true, target: tgTarget(456) }); + forward('agent_done', { text: 'ok cli', origin: 'cli', target: tgTarget(456) }); await flush(); assert.equal(sent.length, 2); @@ -146,16 +149,15 @@ test('forwarder chunks long messages into multiple sends', async () => { const { bot, sent } = createBotSpy(); const forward = createTelegramForwarder({ bot, - getLastChatId: () => 999, }); const longText = `**head**\n${'x'.repeat(5000)}`; - forward('agent_done', { text: longText, origin: 'web' }); + forward('agent_done', { text: longText, origin: 'web', target: tgTarget(999) }); await flush(); assert.equal(sent.length >= 2, true); assert.equal(sent.every((msg) => msg.opts?.parse_mode === 'HTML'), true); - assert.equal(sent.every((msg) => msg.chatId === 999), true); + assert.equal(sent.every((msg) => msg.chatId === '999'), true); assert.equal(sent[0].text.startsWith('📡 '), true); }); @@ -163,10 +165,10 @@ test('forwarder does nothing when type is not agent_done or chatId is missing', const { bot, sent } = createBotSpy(); const forward = createTelegramForwarder({ bot, - getLastChatId: () => null, }); - forward('agent_tool', { text: 'tool message', origin: 'web' }); + forward('agent_tool', { text: 'tool message', origin: 'web', target: tgTarget(123) }); + // No destination on the event: nothing to forward to. forward('agent_done', { text: 'done', origin: 'web' }); await flush(); @@ -182,20 +184,13 @@ test('image relay activation: agent_done broadcast sends text then sendPhoto', { const { bot, photos, events, photoSent } = createBotSpy(); const forward = createTelegramForwarder({ bot, - getLastChatId: () => 123, - getLastTarget: () => ({ - channel: 'telegram', - targetKind: 'channel', - peerKind: 'group', - targetId: '123', - threadId: '42', - }), }); addBroadcastListener(forward); try { broadcast('agent_done', { origin: 'web', text: `ready\n![generated](${imagePath})`, + target: tgTarget(123, '42'), }); await photoSent; assert.deepEqual(events, ['text', 'photo']); @@ -214,11 +209,12 @@ test('image relay guard skips and logs a path outside allowed roots', async () = fs.writeFileSync(imagePath, Buffer.from([0x89, 0x50, 0x4e, 0x47])); const { bot, sent, photos } = createBotSpy(); const before = drainLogRing().length; - const forward = createTelegramForwarder({ bot, getLastChatId: () => 123 }); + const forward = createTelegramForwarder({ bot }); try { const returnValue = forward('agent_done', { origin: 'web', text: `ready\n![denied](${imagePath})`, + target: tgTarget(123), }); assert.equal(returnValue, undefined, 'forwarder keeps a synchronous listener signature'); await flush(); diff --git a/tests/unit/codex-app-multiplex-spawn.test.ts b/tests/unit/codex-app-multiplex-spawn.test.ts index 41025c107..8ea055f66 100644 --- a/tests/unit/codex-app-multiplex-spawn.test.ts +++ b/tests/unit/codex-app-multiplex-spawn.test.ts @@ -15,6 +15,10 @@ test.mock.module('../../src/agent/runtime/events.js', { runtimeEvents.push(event); return event; }, + // A partial namedExports replaces the whole module, so every symbol the + // real projection imports has to be present here or the import throws + // before a single assertion runs. + recordRuntimeProjectionLoss: () => undefined, }, }); diff --git a/tests/unit/discord-forwarder.test.ts b/tests/unit/discord-forwarder.test.ts index 24b0af9a4..54a308d72 100644 --- a/tests/unit/discord-forwarder.test.ts +++ b/tests/unit/discord-forwarder.test.ts @@ -95,10 +95,10 @@ test('Discord forwarder sends text before a guarded local image attachment', asy try { const forward = createDiscordForwarder({ client: client as never, - getLastTarget: () => target, }); await forward('agent_done', { origin: 'web', + target, text: `ready\n![generated](${imagePath})`, }); assert.equal(sent.length, 2); diff --git a/tests/unit/heartbeat-destination-binding.test.ts b/tests/unit/heartbeat-destination-binding.test.ts new file mode 100644 index 000000000..5aea1c389 --- /dev/null +++ b/tests/unit/heartbeat-destination-binding.test.ts @@ -0,0 +1,79 @@ +import '../setup/isolated-home.ts'; +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { + resolveHeartbeatBinding, + isCompleteHeartbeatDestination, + heartbeatHoldMessage, +} from '../../src/memory/heartbeat-destination.ts'; + +// HDB — a scheduled report goes to the conversation an operator named, or it +// does not go. Every "held" case below used to be a send to somewhere nobody +// chose: the active channel, or a channel root standing in for a thread the +// form never collected (#437, #745). + +test('HDB-001 a channel and thread bind to exactly that thread', () => { + const binding = resolveHeartbeatBinding({ + channel: 'slack', targetId: 'C_REPORTS', threadId: '1787616871.254919' }); + assert.equal(binding.state, 'bound'); + assert.equal(binding.state === 'bound' && binding.target.targetId, 'C_REPORTS'); + assert.equal(binding.state === 'bound' && binding.target.threadId, '1787616871.254919'); + assert.equal(binding.state === 'bound' && binding.target.channel, 'slack'); +}); + +test('HDB-002 a missing destination is held, never resolved to an active channel', () => { + for (const absent of [undefined, null]) { + const binding = resolveHeartbeatBinding(absent); + assert.equal(binding.state, 'held'); + assert.equal(binding.state === 'held' && binding.reason, 'unbound_destination'); + } +}); + +test('HDB-003 a Slack channel without a thread is held, not posted to the root', () => { + const binding = resolveHeartbeatBinding({ channel: 'slack', targetId: 'C_REPORTS' }); + assert.equal(binding.state, 'held'); + assert.equal(binding.state === 'held' && binding.reason, 'incomplete_destination'); +}); + +test('HDB-004 channel_root is the explicit way to mean the channel itself', () => { + const binding = resolveHeartbeatBinding({ + channel: 'slack', targetId: 'C_REPORTS', scope: 'channel_root' }); + assert.equal(binding.state, 'bound'); + assert.equal(binding.state === 'bound' && binding.target.threadId, undefined); +}); + +test('HDB-005 a blank thread ts is not a thread', () => { + // It passed shape validation and then read as falsy at send time, which + // turned a threaded job into a root post with no diagnostic anywhere. + assert.equal(isCompleteHeartbeatDestination({ + channel: 'slack', targetId: 'C_REPORTS', threadId: ' ' }), false); + const binding = resolveHeartbeatBinding({ channel: 'slack', targetId: 'C_REPORTS', threadId: '' }); + assert.equal(binding.state, 'held'); + assert.equal(binding.state === 'held' && binding.reason, 'incomplete_destination'); +}); + +test('HDB-006 a malformed destination is distinguishable from an absent one', () => { + for (const bad of [{ channel: 'slack' }, { channel: 'irc', targetId: 'C_X' }, { targetId: 'C_X' }, + { channel: 'slack', targetId: 'C_X', scope: 'dm' }, 'slack', 42]) { + const binding = resolveHeartbeatBinding(bad); + assert.equal(binding.state, 'held', JSON.stringify(bad)); + assert.equal(binding.state === 'held' && binding.reason, 'malformed_destination', JSON.stringify(bad)); + } +}); + +test('HDB-007 non-Slack transports keep their conversation-level contract', () => { + // Telegram and Discord do not carry Slack's thread model, so requiring one + // there would hold every working job for a field it cannot supply. + for (const channel of ['telegram', 'discord'] as const) { + const binding = resolveHeartbeatBinding({ channel, targetId: '12345' }); + assert.equal(binding.state, 'bound', channel); + } +}); + +test('HDB-008 every hold reason explains itself without leaking anything', () => { + for (const reason of ['unbound_destination', 'incomplete_destination', 'malformed_destination'] as const) { + const message = heartbeatHoldMessage(reason); + assert.ok(message.length > 0); + assert.equal(/xox[bp]-|token|secret/i.test(message), false); + } +}); diff --git a/tests/unit/heartbeat-file.test.ts b/tests/unit/heartbeat-file.test.ts index b3aff2756..97a472118 100644 --- a/tests/unit/heartbeat-file.test.ts +++ b/tests/unit/heartbeat-file.test.ts @@ -86,10 +86,26 @@ test('an explicit null clears the destination', () => { test('a supplied destination replaces the stored one', () => { const result = resolveHeartbeatDestination( - { destination: { channel: 'slack', targetId: 'C_NEW' } }, - { channel: 'slack', targetId: 'C_OLD' }, + { destination: { channel: 'slack', targetId: 'C_NEW', threadId: '1787616871.254919' } }, + { channel: 'slack', targetId: 'C_OLD', threadId: '1787616871.111111' }, ); - assert.deepEqual(result, { ok: true, destination: { channel: 'slack', targetId: 'C_NEW' } }); + assert.deepEqual(result, { ok: true, + destination: { channel: 'slack', targetId: 'C_NEW', threadId: '1787616871.254919' } }); +}); + +test('a supplied Slack destination with no thread is rejected', () => { + // Inheritance keeps an existing half-filled job loadable, but writing one is + // how the gap would live forever: the UI cannot show the field, so every + // Save would re-commit "channel, no thread" (#745). + const result = resolveHeartbeatDestination( + { destination: { channel: 'slack', targetId: 'C_NEW' } }, undefined); + assert.equal(result.ok, false); +}); + +test('an explicit channel_root scope is an acceptable Slack destination', () => { + const destination = { channel: 'slack' as const, targetId: 'C_NEW', scope: 'channel_root' as const }; + assert.deepEqual(resolveHeartbeatDestination({ destination }, undefined), + { ok: true, destination }); }); test('a malformed destination is rejected rather than half-applied', () => { @@ -114,4 +130,3 @@ test('a destination written directly to the file survives load', () => { saveHeartbeatFile({ jobs: [] }); } }); - diff --git a/tests/unit/heartbeat-mention-watch-fresh-start.test.ts b/tests/unit/heartbeat-mention-watch-fresh-start.test.ts index b31dd6d2b..236515af9 100644 --- a/tests/unit/heartbeat-mention-watch-fresh-start.test.ts +++ b/tests/unit/heartbeat-mention-watch-fresh-start.test.ts @@ -429,6 +429,7 @@ test('a job with no hold carries no held marker', async () => { const jobId = 'not_held_listed'; saveHeartbeatFile({ jobs: [{ id: jobId, name: jobId, enabled: true, schedule: { kind: 'every', minutes: 10 }, prompt: 'x', + destination: { channel: 'slack', targetId: 'C_REPORTS', threadId: '1787616871.254919' }, }] }); await withHeartbeatServer(async baseUrl => { const response = await fetch(baseUrl + '/api/heartbeat'); diff --git a/tests/unit/heartbeat-runner-modes.test.ts b/tests/unit/heartbeat-runner-modes.test.ts index 94034b750..c3f30a8ab 100644 --- a/tests/unit/heartbeat-runner-modes.test.ts +++ b/tests/unit/heartbeat-runner-modes.test.ts @@ -88,7 +88,9 @@ test('non-planner main heartbeat runs once', async () => { test('busy employee produces warning delivery without running employee', async () => { employeeBusy = true; sent.length = 0; - await runHeartbeatJob({ id: 'busy', name: 'busy', runner: 'employee', employee: employee.name, reportPolicy: 'anomaly_only', schedule: { minutes: 5 }, prompt: 'check' }); + await runHeartbeatJob({ id: 'busy', name: 'busy', runner: 'employee', employee: employee.name, + reportPolicy: 'anomaly_only', schedule: { minutes: 5 }, prompt: 'check', + destination: { channel: 'slack', targetId: 'C_REPORTS', threadId: '1787616871.254919' } }); employeeBusy = false; assert.equal(sent.length, 1); assert.match(sent[0]!, /\[warning\].*skipped: employee busy/); @@ -135,22 +137,45 @@ test('a job with a destination sends there and forbids the active fallback', asy 'a pinned job must not be re-routed by whoever spoke last'); }); -test('a destination without a thread posts to the conversation root', async () => { +test('a destination that opts into the conversation root posts there', async () => { sent.length = 0; sentRequests.length = 0; await runHeartbeatJob({ id: 'root', name: 'root', enabled: true, schedule: { minutes: 5 }, prompt: 'check', - destination: { channel: 'slack', targetId: 'C_REPORTS' }, + destination: { channel: 'slack', targetId: 'C_REPORTS', scope: 'channel_root' }, }); assert.equal(sentRequests[0]?.['target']?.targetId, 'C_REPORTS'); assert.equal(sentRequests[0]?.['target']?.threadId, undefined); }); +test('a Slack destination with no thread and no root opt-in is held', async () => { + // "Channel but no thread" is indistinguishable from a form nobody finished. + // Guessing the root put scheduled reports at the bottom of channels their + // operator had pointed at a specific thread (#745). + sent.length = 0; sentRequests.length = 0; + await runHeartbeatJob({ + id: 'incomplete', name: 'incomplete', enabled: true, schedule: { minutes: 5 }, prompt: 'check', + destination: { channel: 'slack', targetId: 'C_REPORTS' }, + }); + + assert.equal(sentRequests.length, 0, 'an unfinished destination delivers nowhere'); +}); + +test('an empty-string thread is not a thread', async () => { + sent.length = 0; sentRequests.length = 0; + await runHeartbeatJob({ + id: 'blank-thread', name: 'blank-thread', enabled: true, schedule: { minutes: 5 }, prompt: 'check', + destination: { channel: 'slack', targetId: 'C_REPORTS', threadId: '' }, + }); + + assert.equal(sentRequests.length, 0, 'a blank ts used to pass validation then read as falsy at send time'); +}); + test('the derived target carries the kinds the operator never types', async () => { sent.length = 0; sentRequests.length = 0; await runHeartbeatJob({ id: 'kinds', name: 'kinds', enabled: true, schedule: { minutes: 5 }, prompt: 'check', - destination: { channel: 'slack', targetId: 'C_REPORTS' }, + destination: { channel: 'slack', targetId: 'C_REPORTS', scope: 'channel_root' }, }); // Stored form is three fields; targetKind/peerKind come from the id prefix. @@ -158,15 +183,16 @@ test('the derived target carries the kinds the operator never types', async () = assert.equal(sentRequests[0]?.['target']?.peerKind, 'channel'); }); -test('a job without a destination keeps the legacy active-channel behaviour', async () => { +test('a job without a destination delivers nowhere', async () => { + // This used to fall through to the active channel. That path is why a + // scheduled report could arrive in a conversation that had never asked for + // it: "active" is whoever spoke to the bot last, which is not a property of + // the job at all (#437, #745). Silence with a logged reason is the honest + // answer to an unconfigured destination. sent.length = 0; sentRequests.length = 0; await runHeartbeatJob({ id: 'legacy', name: 'legacy', enabled: true, schedule: { minutes: 5 }, prompt: 'check' }); - assert.equal(sentRequests.length, 1); - assert.equal(sentRequests[0]?.['channel'], 'active'); - assert.equal(sentRequests[0]?.['target'], undefined); - assert.equal(sentRequests[0]?.['allowActiveFallback'], undefined, - 'existing installs must not start failing to deliver'); + assert.equal(sentRequests.length, 0); }); test('a malformed destination is refused, not redirected to the active channel', async () => { @@ -199,7 +225,8 @@ test('a scheduled run survives a malformed destination without throwing', async destination: { targetId: 'C_X' }, }); sent.length = 0; sentRequests.length = 0; - await runHeartbeatJob({ id: 'after', name: 'after', enabled: true, schedule: { minutes: 5 }, prompt: 'check' }); + await runHeartbeatJob({ id: 'after', name: 'after', enabled: true, schedule: { minutes: 5 }, prompt: 'check', + destination: { channel: 'slack', targetId: 'C_REPORTS', threadId: '1787616871.254919' } }); assert.equal(sentRequests.length, 1, 'the next job still runs'); }); @@ -207,7 +234,7 @@ test('the anchor records where the report actually went', async () => { anchors.length = 0; await runHeartbeatJob({ id: 'anchored', name: 'anchored', enabled: true, schedule: { minutes: 5 }, prompt: 'check', - destination: { channel: 'slack', targetId: 'C_REPORTS' }, + destination: { channel: 'slack', targetId: 'C_REPORTS', scope: 'channel_root' }, }); // Routing and the record must not disagree: 'active' here would attribute the @@ -233,4 +260,3 @@ test('an employee heartbeat consumes its own worker replay', async () => { assert.equal(hasPendingWorkerReplays('default'), false, 'a heartbeat must not leave the default scope blocked on a replay'); }); - diff --git a/tests/unit/lifecycle-error-forwarding.test.ts b/tests/unit/lifecycle-error-forwarding.test.ts index e637914a3..a48847448 100644 --- a/tests/unit/lifecycle-error-forwarding.test.ts +++ b/tests/unit/lifecycle-error-forwarding.test.ts @@ -14,7 +14,7 @@ function stallParams(overrides: Partial = {}): ExitHandlerPar return { ctx: { fullText: '', sessionId: null, toolLog: [], traceLog: [], stderrBuf: '', stallReason: 'absolute timeout 2045s' }, code: 124, cli: 'codex', model: 'test', resumeKey: null, agentLabel: 'Boss', mainManaged: true, - origin: 'web', prompt: 'test', opts: {}, cfg: {}, ownerGeneration: 1, + origin: 'web', prompt: 'test', opts: { target: { channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C1' } }, cfg: {}, ownerGeneration: 1, persistenceOwner: { global: 0, scope: 0 }, forceNew: false, empSid: null, isResume: false, wasKilled: true, wasSteer: false, smokeResult: { isSmoke: false, confidence: 'low', matchedPattern: null, reason: '' }, effortDefault: 'medium', costLine: '', resolve: () => {}, activeProcesses: new Map(), scopeKey: 'lef-test', @@ -36,10 +36,7 @@ async function runStall(overrides: Partial = {}) { }) as typeof fetch; const seen: Record[] = []; const capture = (type: string, data: Record) => { if (type === 'agent_done') seen.push(data); }; - const forward = createSlackForwarder({ - getToken: () => 'xoxb-test', - getLastTarget: () => ({ channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C1' }), - }); + const forward = createSlackForwarder({ getToken: () => 'xoxb-test' }); const pending: Promise[] = []; const forwardListener = (type: string, data: Record) => { pending.push(forward(type, data)); }; addBroadcastListener(capture); @@ -72,7 +69,7 @@ test('LEF-001: a watchdog kill (wasKilled + stallReason) reaches Slack as a clas for (const finalText of [null, '', ' \n\t ']) { test('native empty compatibility terminal closes once without passive Slack send: ' + JSON.stringify(finalText), async () => { const { payloads, payload, fetches, ends, result } = await runStall({ - code: 0, wasKilled: false, opts: { _skipSessionPersist: true }, + code: 0, wasKilled: false, opts: { _skipSessionPersist: true, target: { channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C1' } }, ctx: { fullText: 'DO NOT DELIVER PARTIAL', liveOutputText: 'DO NOT DELIVER LIVE', requestId: 'native-request', sessionId: 'provider-session', toolLog: [], traceLog: [], stderrBuf: '', runtimeOutcome: { status: 'done', finalText, partialText: 'DO NOT DELIVER PARTIAL' } }, @@ -93,7 +90,7 @@ for (const finalText of [null, '', ' \n\t ']) { test('throwing legacy terminal observer does not duplicate or suppress existing final delivery', async () => { const { payload, fetches, ends, result } = await runStall({ - code: 0, wasKilled: false, costLine: '\nCOST', opts: { _skipSessionPersist: true }, + code: 0, wasKilled: false, costLine: '\nCOST', opts: { _skipSessionPersist: true, target: { channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C1' } }, ctx: { fullText: 'hiddenanswer', sessionId: null, toolLog: [], traceLog: [], stderrBuf: '' }, onRuntimeEnd: () => { throw new Error('projection unavailable'); }, }); diff --git a/tests/unit/print-activity-lifecycle.test.ts b/tests/unit/print-activity-lifecycle.test.ts index c081a262a..46815e5d9 100644 --- a/tests/unit/print-activity-lifecycle.test.ts +++ b/tests/unit/print-activity-lifecycle.test.ts @@ -36,7 +36,7 @@ function fixture() { setSpawnAgent(() => { respawns++; return { promise: Promise.resolve({ text: 'unexpected retry', code: 0 }) }; }); const params: ExitHandlerParams = { ctx, code: 0, cli: 'codex', model: 'fixture', resumeKey: null, agentLabel: 'fixture', mainManaged: true, - origin: 'web', prompt: 'fixture', opts: { _skipSessionPersist: true, _isSmokeContinuation: true }, cfg: {}, + origin: 'web', prompt: 'fixture', opts: { _skipSessionPersist: true, _isSmokeContinuation: true, target: { channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C-fixture' } }, cfg: {}, ownerGeneration: 1, persistenceOwner: { global: 0, scope: 0 }, forceNew: false, empSid: null, isResume: false, wasKilled: false, wasSteer: false, smokeResult: { isSmoke: false, confidence: 'low', matchedPattern: null, reason: '' }, @@ -61,8 +61,7 @@ for (const fault of ['none', 'append', 'terminal', 'link', 'finalize'] as const) t.mock.method(console, 'log', () => {}); t.mock.method(console, 'warn', () => {}); t.mock.method(console, 'error', () => {}); const events: BusEvent[] = [], legacy: Array<{ type: string; data: Record }> = []; const pending: Promise[] = []; - const forward = createSlackForwarder({ getToken: () => 'fixture-token', - getLastTarget: () => ({ channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C-fixture' }) }); + const forward = createSlackForwarder({ getToken: () => 'fixture-token' }); const listener = (type: string, data: Record) => { legacy.push({ type, data }); pending.push(forward(type, data)); }; addBroadcastListener(listener); const unsubscribe = subscribe(e => events.push(e)); const f = fixture(); diff --git a/tests/unit/runtime-messaging-isolation.test.ts b/tests/unit/runtime-messaging-isolation.test.ts index c71c06897..859677c2e 100644 --- a/tests/unit/runtime-messaging-isolation.test.ts +++ b/tests/unit/runtime-messaging-isolation.test.ts @@ -76,11 +76,9 @@ async function sinks(t: TestContext, exercise: (h: { return { message_id: 1 }; } } } as unknown as Parameters[0]['bot']; const forwarders = { - slack: createSlackForwarder({ getToken: () => 'fixture-token', getLastTarget: () => ({ - channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C-fixture' }) }), - discord: createDiscordForwarder({ client: discord, getLastTarget: () => ({ - channel: 'discord', targetKind: 'channel', peerKind: 'channel', targetId: 'D-fixture' }) }), - telegram: createTelegramForwarder({ bot: telegram, getLastChatId: () => 'T-fixture', prefix: '' }), + slack: createSlackForwarder({ getToken: () => 'fixture-token' }), + discord: createDiscordForwarder({ client: discord }), + telegram: createTelegramForwarder({ bot: telegram, prefix: '' }), }; const observers: BroadcastListener[] = [(type) => { legacy.push(type); }]; for (const channel of ['slack', 'discord', 'telegram'] as const) { @@ -105,17 +103,27 @@ async function sinks(t: TestContext, exercise: (h: { assert.deepEqual(fetches, []); assert.deepEqual(sends, []); }, legacyFinal: async replay => { + // A run with no remote destination has nowhere to be forwarded. + // It used to be posted to whichever conversation each channel had + // most recently spoken to, which put an unrelated run's answer in + // a stranger's thread (#742). Now it reaches no channel at all. broadcast('agent_done', { origin: 'web', text: FINAL }); await drain(); replay?.(); await drain(); assert.deepEqual(legacy, ['agent_done']); assert.deepEqual(invocations, ['slack', 'discord', 'telegram']); + assert.deepEqual(fetches, []); + assert.deepEqual(sends, []); + + // The same final, pinned to the Slack conversation that asked for + // it, lands there and ONLY there. Discord and Telegram see the + // event and decline it because it is not addressed to them. + broadcast('agent_done', { origin: 'web', text: FINAL, target: { + channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C-fixture' } }); + await drain(); assert.deepEqual(fetches, ['https://slack.com/api/chat.postMessage']); - assert.equal(sends.length, 3); - for (const channel of ['slack', 'discord', 'telegram']) { - assert.deepEqual(sends.filter(send => send.channel === channel), [{ channel, text: FINAL }]); - } + assert.deepEqual(sends, [{ channel: 'slack', text: FINAL }]); assert.equal(JSON.stringify(sends).includes(CANARY), false); }, }); diff --git a/tests/unit/send-validation.test.ts b/tests/unit/send-validation.test.ts index 1825ff8c2..67fb5985d 100644 --- a/tests/unit/send-validation.test.ts +++ b/tests/unit/send-validation.test.ts @@ -182,7 +182,7 @@ test('normalizeChannelSendRequest gives Slack-shaped channel values an actionabl ); }); -test('empty Slack allowlist permits the exact last-active chatId and preserves its current thread', async () => { +test('empty Slack allowlist permits the exact last-active chatId without moving it into that thread', async () => { await withIsolatedSlack(async capture => { const { sendChannelOutput } = await import('../../src/messaging/send.js'); const { setLastActiveTarget } = await import('../../src/messaging/runtime.js'); @@ -192,11 +192,15 @@ test('empty Slack allowlist permits the exact last-active chatId and preserves i assert.equal(result.ok, true); assert.equal(capture.requests.length, 1); - assert.deepEqual(capture.requests[0]?.target, slackTarget()); + // The last-active slot is evidence the bot belongs in this conversation, + // which is all it is asked for here. It used to also supply the address: + // a caller that named the channel had its post moved into whichever + // thread had spoken most recently (#745). Vouching is not addressing. + assert.deepEqual(capture.requests[0]?.target, slackTarget('C_CURRENT', '')); }); }); -test('empty Slack allowlist permits the exact last-active object target and preserves an omitted thread', async () => { +test('empty Slack allowlist permits the exact last-active object target and keeps its omitted thread omitted', async () => { await withIsolatedSlack(async capture => { const { sendChannelOutput } = await import('../../src/messaging/send.js'); const { setLastActiveTarget } = await import('../../src/messaging/runtime.js'); @@ -210,7 +214,7 @@ test('empty Slack allowlist permits the exact last-active object target and pres }); assert.equal(result.ok, true); - assert.deepEqual(capture.requests[0]?.target, slackTarget()); + assert.deepEqual(capture.requests[0]?.target, slackTarget('C_CURRENT', '')); }); }); @@ -240,7 +244,7 @@ test('latest-seen Slack target authorizes the same explicit chat when last-activ const result = await sendChannelOutput({ channel: 'slack', type: 'text', chatId: 'C_CURRENT' }); assert.equal(result.ok, true); - assert.deepEqual(capture.requests[0]?.target, slackTarget()); + assert.deepEqual(capture.requests[0]?.target, slackTarget('C_CURRENT', '')); }); }); diff --git a/tests/unit/slack-forwarding.test.ts b/tests/unit/slack-forwarding.test.ts index ed7b6add8..c7672aceb 100644 --- a/tests/unit/slack-forwarding.test.ts +++ b/tests/unit/slack-forwarding.test.ts @@ -34,11 +34,9 @@ test('SFW-001: text goes first, then each image is relayed with its filename as globalThis.fetch = fakeFetch(log); settings['workingDir'] = dir; try { - const forward = createSlackForwarder({ - getToken: () => 'xoxb-t', - getLastTarget: () => ({ channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C1' }), - }); - await forward('agent_done', { text: `done\n![chart](${img})` }); + const forward = createSlackForwarder({ getToken: () => 'xoxb-t' }); + await forward('agent_done', { text: `done\n![chart](${img})`, + target: { channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C1' } }); assert.match(log[0]!.url, /chat\.postMessage/, 'the answer text is posted first'); const complete = log.find((l) => /completeUploadExternal/.test(l.url)); assert.ok(complete, 'the image was relayed'); @@ -74,13 +72,13 @@ test('ordinary agent_done prose with a self-chosen table renders richly without }] })); }) as typeof fetch; try { + const dm = { channel: 'slack', targetKind: 'user', peerKind: 'direct', targetId: 'D1', threadId: '9.1' } as const; const forward = createSlackForwarder({ getToken: () => 'xoxb-fixture', - getLastTarget: () => ({ channel: 'slack', targetKind: 'user', peerKind: 'direct', targetId: 'D1', threadId: '9.1' }), log: info => success.push(info), }); // Only ordinary final text: no user request, blocks, table flag or skill invocation. - await forward('agent_done', { text: `8명이라면 B가 적합합니다.\n\n${table}\n\nA는 인원이 부족합니다.` }); + await forward('agent_done', { text: `8명이라면 B가 적합합니다.\n\n${table}\n\nA는 인원이 부족합니다.`, target: dm }); assert.equal(posts.length, 1); assert.deepEqual(posts[0]!['blocks'], [{ type: 'markdown', text: `8명이라면 B가 적합합니다.\n\n${table}\n\nA는 인원이 부족합니다.` }]); assert.equal(reads.length, 1); @@ -88,7 +86,7 @@ test('ordinary agent_done prose with a self-chosen table renders richly without assert.ok(posts.every(p => p['thread_ts'] === '9.1')); assert.equal(success.length, 1); omitStoredTable = true; - await forward('agent_done', { text: table }); + await forward('agent_done', { text: table, target: dm }); assert.equal(success.length, 2, 'forwarding reports transport success independently of rendering verification'); assert.equal(posts.length, 2, 'a verification mismatch never reposts the message'); assert.equal(reads.length, 2, 'both posted messages still undergo verification');