diff --git a/src/agent/spawn.ts b/src/agent/spawn.ts index c012221cb..7412e6f42 100644 --- a/src/agent/spawn.ts +++ b/src/agent/spawn.ts @@ -91,7 +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 { runPinFields, sameRunConversation } 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'; @@ -798,6 +798,12 @@ export function canSteerAgent(scopeKey: string): boolean { return typeof run?.steerTurnInBand === 'function' || typeof run?.replaceTurn === 'function'; } +/** Native replacement owns a stricter mismatch contract than queue fallback: + * malformed cross-conversation metadata must fail before it is persisted. */ +export function hasActiveMainReplacement(scopeKey: string): boolean { + return typeof activeMainProcesses.get(scopeKey)?.replaceTurn === 'function'; +} + export type SteerOutcome = 'steered' | 'fallback-queue' | 'new-run' | 'cancelled' | 'retired'; export async function steerAgent( @@ -866,6 +872,19 @@ export async function steerAgent( sessionId: chatSessionId, reason: outcome.reason, requestId: capturedMeta.requestId })); return 'fallback-queue'; } + if (run && !sameRunConversation( + { origin: run.meta.origin, remoteKey: run.meta.remoteKey + ?? (isRemoteTarget(run.meta.target) ? buildRemoteBindingKey(run.meta.target) : undefined) }, + { origin: source, remoteKey: meta?.remoteKey + ?? (isRemoteTarget(meta?.target) ? buildRemoteBindingKey(meta.target) : undefined) }, + )) { + // In-band and kill-steer mutate the turn already owned by `run`. + // Different remote keys are different conversations, even when a legacy + // scope collapse put them in the same process slot. Let the caller queue + // a separate follow-up instead of giving this run another user's input + // and delivery address (#743). + return 'fallback-queue'; + } if (typeof run?.steerTurnInBand === 'function') { // codex-app same-turn steer. The user row is written only AFTER the // server accepts — a fallback must not leave a duplicate insert for the diff --git a/src/agent/spawn/queue.ts b/src/agent/spawn/queue.ts index 7f23e1070..530dafba4 100644 --- a/src/agent/spawn/queue.ts +++ b/src/agent/spawn/queue.ts @@ -9,6 +9,7 @@ import { sessionLanes, type SessionLanes } from '../../orchestrator/session-lane import { scopeForChatSession } from '../../orchestrator/scope.js'; import { settleOnce } from '../../orchestrator/request-registry.js'; import { readSlackWorkflowMetadata, type SlackWorkflowMetadata } from '../../slack/workflow.js'; +import { runPinFields } from '../../messaging/run-pin.js'; type QueueItem = { slackWorkflow?: SlackWorkflowMetadata; @@ -452,7 +453,14 @@ export function createQueueController( console.log(`[queue] +1 (${messageQueue.length} pending)`); deps.broadcast('queue_update', { ...queueUpdatePayload(item.scope), - ...(item.requestId ? { requestId: item.requestId, origin: item.source || 'web' } : {}), + ...(item.requestId ? runPinFields({ + requestId: item.requestId, + origin: item.source || 'web', + scope: item.scope, + sessionId: item.chatSessionId, + remoteKey: item.remoteKey, + target: item.target, + }) : {}), }); void processQueue(item.scope); return item.id; @@ -519,7 +527,10 @@ export function createQueueController( return; } const collectedItems = multiSessionEnabled && item!.collect - ? messageQueue.filter(candidate => normalizeScope(candidate.scope) === itemScope && candidate.collect === true && !scheduledItemIds.has(candidate.id)) + ? messageQueue.filter(candidate => normalizeScope(candidate.scope) === itemScope + && candidate.collect === true + && candidate.remoteKey === item!.remoteKey + && !scheduledItemIds.has(candidate.id)) : []; const runItems = [item!, ...collectedItems]; const runIds = new Set(runItems.map(candidate => candidate.id)); @@ -585,6 +596,7 @@ export function createQueueController( // turns with the same duplicate to prevent. if (requestId) deps.broadcast('queued_run_started', stripUndefined({ requestId, origin, scope: item.scope, target, sessionId: effectiveSessionId, + remoteKey: item.remoteKey, slackWorkflow: item.slackWorkflow, })); deps.broadcast('queue_update', queueUpdatePayload(item.scope)); @@ -614,7 +626,11 @@ export function createQueueController( } catch (err: unknown) { const msg = (err as Error).message; console.error('[queue:orchestrate]', msg); - deps.broadcast('orchestrate_done', { text: `[error] ${msg}`, error: true, origin, chatId, target, requestId, replyViaTarget, fromQueue: true, ...(eventScope || {}) }); + deps.broadcast('orchestrate_done', { + ...runPinFields({ requestId, origin, scope: item.scope, sessionId: effectiveSessionId, + remoteKey: item.remoteKey, target }), + text: `[error] ${msg}`, error: true, chatId, replyViaTarget, fromQueue: true, + }); // The pipeline threw before reaching its own settle site, so // this is the last place that can answer the caller. settleOnce(requestId, 'failed', { error: msg }); @@ -625,8 +641,12 @@ export function createQueueController( if (!inserted) { messageQueue.unshift(...runItems); } else { - deps.broadcast('orchestrate_done', { text: `[error] setup failed: ${(setupErr as Error).message}`, error: true, origin, chatId, target, requestId, replyViaTarget, fromQueue: true, - ...(multiSessionEnabled ? { scope: item.scope, sessionId: effectiveSessionId } : {}) }); + deps.broadcast('orchestrate_done', { + ...runPinFields({ requestId, origin, scope: item.scope, sessionId: effectiveSessionId, + remoteKey: item.remoteKey, target }), + text: `[error] setup failed: ${(setupErr as Error).message}`, error: true, + chatId, replyViaTarget, fromQueue: true, + }); // Re-queued items settle on their eventual run; these do not get // another chance, so answer the caller here. settleOnce(requestId, 'failed', { error: `setup failed: ${(setupErr as Error).message}` }); diff --git a/src/messaging/run-pin.ts b/src/messaging/run-pin.ts index e85e88613..840a9052d 100644 --- a/src/messaging/run-pin.ts +++ b/src/messaging/run-pin.ts @@ -15,7 +15,7 @@ // 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'; +import { isRemoteTarget, type RemoteTarget } from './types.js'; export interface RunPin { origin?: string | undefined; @@ -41,7 +41,7 @@ export function runPinFields(pin: RunPin): Record { 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) { + if (isRemoteTarget(pin.target)) { fields['target'] = { ...pin.target }; } return fields; @@ -63,3 +63,35 @@ export function sameRunConversation(a: RunPin, b: RunPin): boolean { if (a.remoteKey || b.remoteKey) return a.remoteKey === b.remoteKey; return (a.origin ?? '') === (b.origin ?? ''); } + +function sameTarget(expected: RemoteTarget, actual: unknown): boolean { + return isRemoteTarget(actual) + && actual.channel === expected.channel + && actual.targetKind === expected.targetKind + && actual.peerKind === expected.peerKind + && actual.targetId === expected.targetId + && actual.threadId === expected.threadId + && actual.guildId === expected.guildId + && actual.parentTargetId === expected.parentTargetId; +} + +/** + * Match a terminal/control event to the run waiting for it. + * + * requestId, origin, scope and sessionId are the minimum identity. Treating an + * absent field as agreement is the bug: an old print terminal omitted all four + * except origin, and a newly installed Slack waiter could adopt it (#743). + * Remote conversation and destination become mandatory when the waiter owns + * them. This lets local/web runs keep their smaller identity while remote runs + * fail closed on the extra fields that separate one conversation from another. + */ +export function matchesRunPin(expected: RunPin, actual: Record): boolean { + if (!expected.requestId || !expected.origin || !expected.scope || !expected.sessionId) return false; + if (actual['requestId'] !== expected.requestId + || actual['origin'] !== expected.origin + || actual['scope'] !== expected.scope + || actual['sessionId'] !== expected.sessionId) return false; + if (expected.remoteKey !== undefined && actual['remoteKey'] !== expected.remoteKey) return false; + if (expected.target !== undefined && !sameTarget(expected.target, actual['target'])) return false; + return true; +} diff --git a/src/orchestrator/collect.ts b/src/orchestrator/collect.ts index 50e86f853..43a3bcabd 100644 --- a/src/orchestrator/collect.ts +++ b/src/orchestrator/collect.ts @@ -16,6 +16,8 @@ import { settings } from '../core/config.js'; import { getActiveChatSession } from '../core/chat-sessions.js'; import { currentSessionScope } from '../core/session-context.js'; import { resolveExecutionBinding } from './scope.js'; +import { matchesRunPin } from '../messaging/run-pin.js'; +import { isRemoteTarget } from '../messaging/types.js'; export interface CollectedOrchestrateResult { text: string; @@ -51,6 +53,16 @@ export function orchestrateAndCollectData( const runMeta = { ...meta, ...binding, origin: meta['origin'] || 'web', _onRuntimeActivity: onRuntimeActivity }; const requestId = meta['requestId'] || undefined; + const strictTerminalPin = runMeta.origin === 'slack' && requestId + ? { + requestId, + origin: 'slack', + scope: binding.scope, + sessionId: binding.chatSessionId, + remoteKey: meta['remoteKey'], + target: isRemoteTarget(meta['target']) ? { ...meta['target'] } : undefined, + } + : null; let collected = ''; let ownTerminalDiagnostic = ''; let nativeSeen = false; @@ -131,11 +143,14 @@ export function orchestrateAndCollectData( // must differ — a steer carrying our own id is not a supersession. if (type === 'steer_started' && data['scope'] === binding.scope - && (data['sessionId'] === undefined || data['sessionId'] === binding.chatSessionId) + && data['sessionId'] === binding.chatSessionId + && data['origin'] === runMeta.origin + && (meta['remoteKey'] === undefined || data['remoteKey'] === meta['remoteKey']) && (!requestId || data['requestId'] !== requestId)) { superseded = true; } if (type === 'orchestrate_done') { + if (strictTerminalPin && !matchesRunPin(strictTerminalPin, data)) return; // Filter by requestId (strongest), then origin, then chatId if (meta?.["requestId"] && data?.["requestId"] && data["requestId"] !== meta["requestId"]) return; if (meta?.["origin"] && data?.["origin"] && data["origin"] !== meta["origin"]) return; diff --git a/src/orchestrator/gateway.ts b/src/orchestrator/gateway.ts index 260520a07..7c507f78e 100644 --- a/src/orchestrator/gateway.ts +++ b/src/orchestrator/gateway.ts @@ -3,7 +3,7 @@ // Replaces duplicated intent/queue/orchestrate logic in server.ts + bot.ts. import { randomUUID } from 'node:crypto'; -import { isAgentBusy, enqueueMessage, killActiveAgent, messageQueue, purgeQueueOnStop, steerAgent } from '../agent/spawn.js'; +import { getCurrentMainMeta, hasActiveMainReplacement, isAgentBusy, enqueueMessage, killActiveAgent, messageQueue, purgeQueueOnStop, steerAgent } from '../agent/spawn.js'; import { hasBlockingWorkers } from './worker-registry.js'; import { getSession, insertMessage } from '../core/db.js'; import { resolveMainCli, type MainSessionRecord } from '../core/main-session.js'; @@ -19,12 +19,13 @@ import { } from './pipeline.js'; import { getState } from './state-machine.js'; import { channelGateOn, resolveOrcScope } from './scope.js'; -import type { RuntimeOrigin, RemoteTarget } from '../messaging/types.js'; +import { isRemoteTarget, type RuntimeOrigin, type RemoteTarget } from '../messaging/types.js'; import { buildRemoteBindingKey, normalizedThreadId, type SessionScope } from '../messaging/session-key.js'; import { sessionLanes } from './session-lanes.js'; import { admitRequest, settleOnce } from './request-registry.js'; import { beginSteerInput } from '../agent/steer-input-guard.js'; import type { SlackWorkflowMetadata } from '../slack/workflow.js'; +import { sameRunConversation } from '../messaging/run-pin.js'; export type SubmitResult = { action: 'started' | 'queued' | 'rejected'; @@ -102,6 +103,27 @@ function applyMidRunPolicy( }; if (policy === 'steer') { + const owner = getCurrentMainMeta(ctx.scopeKey); + if (owner && !sameRunConversation( + { origin: owner.origin, remoteKey: owner.remoteKey + ?? (isRemoteTarget(owner.target) ? buildRemoteBindingKey(owner.target) : undefined) }, + { origin: ctx.meta.origin, remoteKey: ctx.remoteKey + ?? (isRemoteTarget(ctx.meta.target) ? buildRemoteBindingKey(ctx.meta.target) : undefined) }, + )) { + // Native replacement validates every owner dimension and returns a + // typed failure. Queueing here would preserve the owner's + // scope/session/target while swapping only remoteKey, which stores + // another conversation's prompt in the owner's transcript. + if (hasActiveMainReplacement(ctx.scopeKey)) { + // Fall through to steerAgent; its immutable owner check rejects. + } else { + // Steer changes the turn that is already running. A different + // remoteKey is a different conversation, so applying its text to + // this owner would also hand it the owner's progress and terminal + // delivery. Queue it as its own follow-up instead (#743). + return queue(); + } + } // 'steer' means the message steers the agent — never a silent queue. // A steerable Codex App turn receives in-band input. Native Cursor/Grok // use their cancel-reprompt hooks; other runtimes take the kill-steer @@ -171,7 +193,8 @@ export function __resetSubmitDedupForTest(): void { function runDetached( task: Promise, label: string, - meta: { origin: RuntimeOrigin; target?: RemoteTarget; chatId?: string | number; requestId?: string; replyViaTarget?: boolean; eventScope?: { scope: string; sessionId: string } }, + meta: { origin: RuntimeOrigin; target?: RemoteTarget; chatId?: string | number; requestId?: string; + remoteKey?: string; replyViaTarget?: boolean; eventScope?: { scope: string; sessionId: string } }, ) { task.catch((err: unknown) => { const msg = (err as Error)?.message || String(err); @@ -182,6 +205,7 @@ function runDetached( target: meta.target, chatId: meta.chatId, requestId: meta.requestId, + ...(meta.remoteKey ? { remoteKey: meta.remoteKey } : {}), replyViaTarget: meta.replyViaTarget, ...meta.eventScope, error: true, @@ -231,7 +255,12 @@ export function submitMessage( // Admit the request the moment its id exists. Every exit below then settles // through settleOnce(), so a caller holding this id always hears exactly one // terminal event — including on paths that never emit orchestrate_done. - admitRequest(requestId, scope); + admitRequest(requestId, scope, Date.now(), { + origin: meta.origin, + sessionId: chatSessionId, + ...(remoteKey ? { remoteKey } : {}), + ...(meta.target ? { target: meta.target } : {}), + }); try { meta.onAdmitted?.({ requestId, scope, chatSessionId }); } catch { settleOnce(requestId, 'failed', { error: 'slack_tool_context_unavailable' }); return { action: 'rejected', reason: 'slack_tool_context_unavailable', requestId }; } // OFF-mode byte-compat: only expose resolved identity when multi-session is on — @@ -239,7 +268,9 @@ export function submitMessage( const sessionContext = multiSessionEnabled ? { scope, chatSessionId, ...(remoteKey ? { remoteKey } : {}) } : undefined; - const eventScope = multiSessionEnabled ? { scope, sessionId: chatSessionId } : undefined; + const eventScope = multiSessionEnabled || meta.origin === 'slack' + ? { scope, sessionId: chatSessionId } + : undefined; // Reject before recording input, steering, interrupting or enqueueing. The // synchronous response must not advertise a rejected admission as steered. diff --git a/src/orchestrator/pipeline.ts b/src/orchestrator/pipeline.ts index f32fd1977..8a67cc042 100644 --- a/src/orchestrator/pipeline.ts +++ b/src/orchestrator/pipeline.ts @@ -7,6 +7,8 @@ import { resolve } from 'node:path'; import { broadcast } from '../core/bus.js'; import { readSlackWorkflowMetadata, isWorkflowReplyUnconfirmed } from '../slack/workflow.js'; import { settings } from '../core/config.js'; +import { stripUndefined } from '../core/strip-undefined.js'; +import { isRemoteTarget } from '../messaging/types.js'; import { clearAllEmployeeSessions, getRecentMessagesLite, @@ -72,7 +74,11 @@ function captureExecutionMeta(meta: Record & { remoteKey?: stri activeChatSessionId: getActiveChatSession(), multiSessionEnabled: settings['multiSession']?.enabled === true, }); - return { ...meta, ...binding }; + return { + ...meta, + ...(isRemoteTarget(meta['target']) ? { target: { ...meta['target'] } } : {}), + ...binding, + }; } function runtimeActivityLifecycle(meta: Record) { @@ -286,6 +292,28 @@ export async function drainPendingReplays( // ─── orchestrate (PABCD sole entry point) ─────────── +/** Identity a terminal exposes to its request owner. + * + * Slack always needs scope/session/remoteKey even with multi-session disabled: + * its reply waiter is conversation-scoped and must fail closed on an event that + * cannot name its owner (#743). Other legacy web/CLI terminals keep their + * existing compact shape unless multi-session or strict ownership asks for the + * same fields. */ +function terminalEventIdentity( + meta: Record, + origin: string, + scope: string, + sessionId: string, +): Record { + const scoped = origin === 'slack' + || settings["multiSession"]?.enabled === true + || meta["_strictRequestOwnership"] === true; + return stripUndefined({ + ...(scoped ? { scope, sessionId } : {}), + remoteKey: meta["remoteKey"], + }); +} + export async function orchestrate( prompt: string, meta: Record = {}, @@ -350,7 +378,7 @@ export async function orchestrate( requestId, replyViaTarget, ...(fromQueue ? { fromQueue: true } : {}), - ...((settings["multiSession"]?.enabled === true || meta["_strictRequestOwnership"] === true) ? { scope, sessionId: meta["chatSessionId"] || getActiveChatSession() } : {}), + ...terminalEventIdentity(meta, origin, scope, meta["chatSessionId"] || getActiveChatSession()), }); settleOnce(requestId, 'completed', { scope }); return; @@ -726,8 +754,8 @@ export async function orchestrate( ? { executionFailed: true } : {}), ...(fromQueue ? { fromQueue: true } : {}), ...(fromSteer ? { fromSteer: true } : {}), - ...(nativeOutcome ? { scope, sessionId: chatSessionId } - : (settings["multiSession"]?.enabled === true || meta["_strictRequestOwnership"] === true) ? { scope, sessionId: meta["chatSessionId"] || getActiveChatSession() } : {}), + ...(nativeOutcome ? { scope, sessionId: chatSessionId, ...(meta["remoteKey"] ? { remoteKey: meta["remoteKey"] } : {}) } + : terminalEventIdentity(meta, origin, scope, meta["chatSessionId"] || getActiveChatSession())), ...(typeof result['agyPlannerOnly'] === 'boolean' ? { agyPlannerOnly: result['agyPlannerOnly'] } : {}), ...(typeof result['agyCheckpointSeen'] === 'boolean' ? { agyCheckpointSeen: result['agyCheckpointSeen'] } : {}), ...(elicitationSpecs.length > 0 ? { elicitationSpecs } : {}), @@ -768,7 +796,7 @@ export async function orchestrateContinue( requestId, replyViaTarget, ...(meta["_fromQueue"] === true ? { fromQueue: true } : {}), - ...((settings["multiSession"]?.enabled === true || meta["_strictRequestOwnership"] === true) ? { scope, sessionId: meta["chatSessionId"] || getActiveChatSession() } : {}), + ...terminalEventIdentity(meta, origin, scope, meta["chatSessionId"] || getActiveChatSession()), }); settleOnce(requestId, 'completed', { scope, text: 'No pending work to continue.' }); } @@ -810,7 +838,7 @@ export async function orchestrateReset( requestId, replyViaTarget, ...(meta["_fromQueue"] === true ? { fromQueue: true } : {}), - ...((settings["multiSession"]?.enabled === true || meta["_strictRequestOwnership"] === true) ? { scope, sessionId: meta["chatSessionId"] || getActiveChatSession() } : {}), + ...terminalEventIdentity(meta, origin, scope, meta["chatSessionId"] || getActiveChatSession()), }); settleOnce(requestId, 'completed', { scope, text: 'Reset complete.' }); return; @@ -825,7 +853,7 @@ export async function orchestrateReset( requestId, replyViaTarget, ...(meta["_fromQueue"] === true ? { fromQueue: true } : {}), - ...((settings["multiSession"]?.enabled === true || meta["_strictRequestOwnership"] === true) ? { scope, sessionId: meta["chatSessionId"] || getActiveChatSession() } : {}), + ...terminalEventIdentity(meta, origin, scope, meta["chatSessionId"] || getActiveChatSession()), }); settleOnce(requestId, 'completed', { scope, text: 'Reset complete.' }); } diff --git a/src/orchestrator/request-registry.ts b/src/orchestrator/request-registry.ts index 055fefec1..db43e5706 100644 --- a/src/orchestrator/request-registry.ts +++ b/src/orchestrator/request-registry.ts @@ -28,6 +28,7 @@ import { revokeSlackToolGrant } from '../slack/tool-context.js'; */ import { broadcast } from '../core/bus.js'; import type { RuntimeTurnOutcome } from '../shared/runtime-contract.js'; +import { runPinFields, type RunPin } from '../messaging/run-pin.js'; export type SettleOutcome = /** Ran to completion; `text` carries the answer. */ @@ -45,7 +46,7 @@ export type SettleOutcome = /** Accepted but deliberately not orchestrated. */ | 'skipped'; -export interface SettleDetail { +export interface SettleDetail extends Omit { text?: string; error?: string; mergedInto?: string; @@ -60,14 +61,31 @@ interface PendingRequest { requestId: string; scope: string; admittedAt: number; + origin?: string; + sessionId?: string; + remoteKey?: string; + target?: RunPin['target']; } const pending = new Map(); /** Called where the requestId is minted, before any work is dispatched. */ -export function admitRequest(requestId: string, scope = 'default', now = Date.now()): void { +export function admitRequest( + requestId: string, + scope = 'default', + now = Date.now(), + pin: Omit = {}, +): void { if (!requestId) return; - pending.set(requestId, { requestId, scope, admittedAt: now }); + pending.set(requestId, { + requestId, + scope, + admittedAt: now, + ...(pin.origin ? { origin: pin.origin } : {}), + ...(pin.sessionId ? { sessionId: pin.sessionId } : {}), + ...(pin.remoteKey ? { remoteKey: pin.remoteKey } : {}), + ...(pin.target ? { target: { ...pin.target } } : {}), + }); } /** @@ -93,14 +111,19 @@ export function settleOnce( // applies exactly as it does to orchestrate_done — this is not a wider // audience for the same content. broadcast('request_settled', { - requestId, + ...runPinFields({ + requestId, + origin: detail.origin ?? entry.origin, + scope: detail.scope ?? entry.scope, + sessionId: detail.sessionId ?? entry.sessionId, + remoteKey: detail.remoteKey ?? entry.remoteKey, + target: detail.target ?? entry.target, + }), outcome, - scope: detail.scope ?? entry.scope, ...(detail.text !== undefined ? { text: detail.text } : {}), ...(detail.error !== undefined ? { error: detail.error } : {}), ...(detail.mergedInto !== undefined ? { mergedInto: detail.mergedInto } : {}), ...(detail.reason !== undefined ? { reason: detail.reason } : {}), - ...(detail.sessionId !== undefined ? { sessionId: detail.sessionId } : {}), ...(detail.runtimeFinality !== undefined ? { runtimeFinality: detail.runtimeFinality } : {}), ...(detail.runtimeStatus !== undefined ? { runtimeStatus: detail.runtimeStatus } : {}), }); diff --git a/src/slack/bot.ts b/src/slack/bot.ts index b3c9556fb..c771a478a 100644 --- a/src/slack/bot.ts +++ b/src/slack/bot.ts @@ -24,6 +24,7 @@ import { } from '../messaging/runtime.js'; import { slackTargetFromId, resolveSlackThreadPlacement } from '../messaging/slack-target.js'; import { isRemoteTarget, type RemoteTarget } from '../messaging/types.js'; +import { matchesRunPin } from '../messaging/run-pin.js'; import { sessionLanes } from '../orchestrator/session-lanes.js'; import { createSlackReplyDeliveryLedger } from './reply-delivery.js'; import { buildMediaPromptMany } from '../agent/spawn.js'; @@ -459,16 +460,14 @@ const SLACK_PENDING_STEER_MAX = 256; const SLACK_PENDING_STEER_TTL_MS = 300_000; function matchesSlackReply(options: SlackReplyOptions, data: Record): boolean { - const target = data['target']; - return data['requestId'] === options.requestId - && (data['scope'] === undefined || data['scope'] === options.session.scope) - && (data['sessionId'] === undefined || data['sessionId'] === options.session.chatSessionId) - && (data['origin'] === undefined || data['origin'] === 'slack') - && (data['remoteKey'] === undefined || data['remoteKey'] === options.session.remoteKey) - && (target === undefined || (isRemoteTarget(target) && target.channel === 'slack' - && target.targetId === options.target.targetId && target.threadId === options.target.threadId - && target.guildId === options.target.guildId && target.targetKind === options.target.targetKind - && target.peerKind === options.target.peerKind && target.parentTargetId === options.target.parentTargetId)); + return matchesRunPin({ + requestId: options.requestId, + origin: 'slack', + scope: options.session.scope, + sessionId: options.session.chatSessionId, + remoteKey: options.session.remoteKey, + target: options.target, + }, data); } function rememberPendingSteer(options: SlackReplyOptions): void { diff --git a/src/slack/commands.ts b/src/slack/commands.ts index f497db11a..2b782bb3a 100644 --- a/src/slack/commands.ts +++ b/src/slack/commands.ts @@ -160,7 +160,12 @@ export async function handleSlackSlashCommand(payload: Record): const granted = workspace && workspace.teamId === payload['team_id'] && actorId ? reserveSlackToolGrant({ teamId: workspace.teamId, actorId, destination: target, credentialKey: slackCredentialKey(token) }, { requestId, scope, chatSessionId }) : false; - admitRequest(requestId, scope); + admitRequest(requestId, scope, Date.now(), { + origin: 'slack', + sessionId: chatSessionId, + ...(remoteKey ? { remoteKey } : {}), + target, + }); let reply: string; try { reply = String(await withSessionScope(sessionScope, () => orchestrateAndCollect(steerPrompt, { requestId, ...(granted ? { _strictRequestOwnership: true } : {}), diff --git a/src/slack/progress-lifecycle.ts b/src/slack/progress-lifecycle.ts index 66d6d4b06..443a46a06 100644 --- a/src/slack/progress-lifecycle.ts +++ b/src/slack/progress-lifecycle.ts @@ -5,6 +5,7 @@ import { subscribeRuntimeLiveness } from '../agent/runtime/liveness.js'; import { projectSlackPrintTool, projectSlackRuntimeTool, type SlackActivityTool } from './progress-activity.js'; import { startSlackProgress, type SlackProgressHandle, type SlackProgressOutcome, type SlackProgressPhase } from './progress.js'; import type { RemoteTarget } from '../messaging/types.js'; +import { matchesRunPin } from '../messaging/run-pin.js'; import type { RuntimeLivenessIdentity } from '../shared/runtime-contract.js'; export type SlackProgressFinishOptions = { reason?: 'merged' | 'removed'; bodyDelivered?: boolean }; @@ -58,10 +59,12 @@ export function createSlackProgressLifecycle(input: SlackProgressLifecycleOption let unsubscribeEvents = () => {}; const enabled = Boolean(config.requestId && config.scope && config.sessionId); const notify = (fn: () => void) => { try { fn(); } catch { log.warn('[slack:progress] observer callback failed'); } }; - const matches = (data: Record): boolean => data['requestId'] === config.requestId - && (data['scope'] === undefined || data['scope'] === config.scope) - && (data['sessionId'] === undefined || data['sessionId'] === config.sessionId) - && (data['origin'] === undefined || data['origin'] === 'slack'); + const matches = (data: Record): boolean => matchesRunPin({ + requestId: config.requestId, + origin: 'slack', + scope: config.scope, + sessionId: config.sessionId, + }, data); function clearNativeBuffer(): void { nativePending = []; pendingGaps.clear(); diff --git a/structure/str_func.md b/structure/str_func.md index 613859274..64874ae5c 100644 --- a/structure/str_func.md +++ b/structure/str_func.md @@ -145,9 +145,9 @@ cli-jaw/ │ │ ├── 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 (4096L) +│ │ ├── 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 (4115L) │ │ ├── spawn/ ← spawn 서브모듈 (3 files) -│ │ │ ├── queue.ts ← QueueItem persistent DB queue + processQueue race fix + enqueue/dequeue + drainRecoveredQueue (부팅 시 복구 큐 기동, server.ts가 transport 준비 후 호출) + `_fromQueue` 표식 (대기자 없는 턴을 채널이 답할 수 있게) (703L) +│ │ │ ├── queue.ts ← QueueItem persistent DB queue + processQueue race fix + enqueue/dequeue + drainRecoveredQueue (부팅 시 복구 큐 기동, server.ts가 transport 준비 후 호출) + `_fromQueue` 표식 (대기자 없는 턴을 채널이 답할 수 있게) (723L) │ │ │ ├── resume.ts ← session resume logic + stale resume detection (117L) │ │ │ └── process-kill.ts ← child process kill helper (195L) │ │ ├── events/ ← NDJSON 이벤트 파서 모듈 분리 (12 files) @@ -236,11 +236,11 @@ cli-jaw/ │ │ └── slack-target.ts ← Slack target helper (74L) │ ├── orchestrator/ ← 직원 오케스트레이션 + 인터페이스 통합 (19 files) │ │ ├── state-machine.ts ← IPABCD 상태 머신 (I=Interview pre-plan) + broadcast(state,title) + worklog 타이틀 파싱 + employee terminology + OrcContext.workingDir + OrcContext.interview + Project root dispatch contract + Phase60 actor-aware canTransition(GateInput) form-only evidence gate + STATE_PROMPTS --attest instructions (806L) -│ │ ├── pipeline.ts ← IPABCD orchestration (explicit entry only) + interview first-turn detection + plan context persistence + memorySnapshot injection + reset clears boss session + OrcContext workingDir init + Approved Plan Project root guard + remote-channel elicitation guard + bounded delayed worker replay notice + Phase60 phase_attestation strip/fallback + no-state narration warn (831L) +│ │ ├── pipeline.ts ← IPABCD orchestration (explicit entry only) + interview first-turn detection + plan context persistence + memorySnapshot injection + reset clears boss session + OrcContext workingDir init + Approved Plan Project root guard + remote-channel elicitation guard + bounded delayed worker replay notice + Phase60 phase_attestation strip/fallback + no-state narration warn (859L) │ │ ├── distribute.ts ← runSingleAgent + buildPlanPrompt + parallel helpers + tiered findEmployee + employee resume diagnostics + virtual employee session-skip (518L) │ │ ├── parser.ts ← triage + subtask JSON + verdict 파싱 + isResetIntent (176L) -│ │ ├── gateway.ts ← submitMessage 통합 진입점 (WebUI+CLI+TG+Discord 공통) + working_dir scoped insertMessage (350L) -│ │ ├── collect.ts ← orchestrateAndCollect + orchestrateAndCollectData (191L) +│ │ ├── gateway.ts ← submitMessage 통합 진입점 (WebUI+CLI+TG+Discord 공통) + working_dir scoped insertMessage (381L) +│ │ ├── collect.ts ← orchestrateAndCollect + orchestrateAndCollectData (206L) │ │ ├── session-work.ts ← hasChatSessionWork — 세션 삭제 전 진행중 작업 관측 (활성 run·큐·replay는 정확 매칭, drain/retry/hold/worker/lane은 scope 단위 보수적 판정) (42L) ✨ │ │ ├── scope.ts ← remote binding key + channel gate + local session scope + captured execution binding; legacy findActiveScope만 default fallback (86L) │ │ ├── worker-monitor.ts ← Worker stall detection — activity timestamps + stall/disconnect/timeout callbacks (58L) @@ -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 (1936L) +│ │ ├── bot.ts ← Slack 봇 lifecycle + attachPort 성공 후 best-effort 자기선출/영속화 + envelope routing + orchestrate 경로 + queued-result waiter + top-level/thread 1회 context prefetch (1935L) │ │ ├── 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) @@ -368,7 +368,7 @@ cli-jaw/ │ │ ├── history.ts ← 동적 조회 (conversations.history/replies form-encoded 래퍼 + cursor 정규화 + 재시도 + 에이전트용 포맷/redact) (373L) │ │ ├── mention-watch.ts ← 가입 채널 backward mention scan + frontier/resume/round-robin/429 stop/60-channel overflow (369L) │ │ ├── attachment-recovery.ts ← app_mention 봉투에 없는 첨부를 channel+ts 재조회로 복구 (oldest+inclusive+limit=1) (53L) -│ │ ├── commands.ts ← slash command → 공유 parseCommand/executeCommand 파이프라인 (180L) +│ │ ├── commands.ts ← slash command → 공유 parseCommand/executeCommand 파이프라인 (185L) │ │ ├── slack-file.ts ← files.getUploadURLExternal → upload → completeUploadExternal 3단계 업로드 (파일명·캡션 모두 아웃바운드 마스킹) (158L) │ │ ├── ingress.ts ← 세션별 ingress lane + synthetic top-level followup override + admitSlackRun 동기 실행 예약(sessionLanes) + 전역 다운로드 세마포어 + shutdown abort/drain (304L) ✨ │ │ ├── inbound-file.ts ← 인바운드 첨부 단일 IO owner (files.info → 인증 스트리밍 다운로드 → saveUpload, 파일/메시지 바이트 예산, 고정 error code) (280L) ✨ @@ -384,7 +384,7 @@ cli-jaw/ │ │ ├── progress-activity.ts ← safe fixed-category activity projection, bounded recent observations and delivery receipt (244L) │ │ ├── progress-files.ts ← shared bounded file target projection under captured working directory (61L) │ │ ├── progress-detail.ts ← explicit purposes and finite safe command action summaries (319L) -│ │ ├── progress-lifecycle.ts ← request/native identity binding, safe buffers and owned terminal teardown (232L) +│ │ ├── progress-lifecycle.ts ← request/native identity binding, safe buffers and owned terminal teardown (235L) │ │ ├── progress-restore.ts ← generation-bound single-flight status recovery and abort ownership (86L) │ │ ├── reply-delivery.ts ← bounded start proof and atomic reply workflow claims independent of display expiry (91L) │ │ ├── verified-workspace.ts ← 토큰이 실제로 가리키는 team id (auth.test 1회 + 토큰별 캐시, settings.teamId 불신, 실패 시 null로 거부) (68L) ✨ diff --git a/tests/unit/codex-app-steer.test.ts b/tests/unit/codex-app-steer.test.ts index fbf33baba..76a796f82 100644 --- a/tests/unit/codex-app-steer.test.ts +++ b/tests/unit/codex-app-steer.test.ts @@ -129,18 +129,23 @@ type FakeRun = { starting: boolean; steering: boolean; ownerGeneration: number; - meta: { origin: string; cli: string; chatSessionId?: string }; + meta: { origin: string; cli: string; chatSessionId?: string; remoteKey?: string }; steerTurnInBand?: (text: string) => Promise<'steered' | 'unavailable' | 'rejected'>; }; -function installFakeCodexAppRun(scope: string, outcome: 'steered' | 'unavailable' | 'rejected'): { calls: string[] } { +function installFakeCodexAppRun( + scope: string, + outcome: 'steered' | 'unavailable' | 'rejected', + remoteKey?: string, +): { calls: string[] } { const calls: string[] = []; const run: FakeRun = { process: null, starting: false, steering: false, ownerGeneration: 0, - meta: { origin: 'web', cli: 'codex-app', chatSessionId: 'cs-spawn' }, + meta: { origin: remoteKey ? 'slack' : 'web', cli: 'codex-app', chatSessionId: 'cs-spawn', + ...(remoteKey ? { remoteKey } : {}) }, steerTurnInBand: async (text: string) => { calls.push(text); return outcome; }, }; activeMainProcesses.set(scope, run as never); @@ -160,7 +165,7 @@ test('CS-007: canSteerAgent is true while a codex-app steer hook is installed', test('CS-008: steerAgent routes in-band for codex-app and inserts the user row once accepted', async () => { const { calls } = installFakeCodexAppRun('cs008', 'steered'); try { - const outcome = await steerAgent('cs008', 'remember the context', 'test', { chatSessionId: 'cs-spawn' }); + const outcome = await steerAgent('cs008', 'remember the context', 'web', { chatSessionId: 'cs-spawn' }); assert.equal(outcome, 'steered'); assert.deepEqual(calls, ['remember the context']); const rows = getRecentMessagesAll.all('cs-spawn', 5) as Array<{ role: string; content: string }>; @@ -193,3 +198,17 @@ test('CS-010: raced steer (turn ended) falls back to queue', async () => { activeMainProcesses.delete('cs010'); } }); + +test('CS-011: another remote conversation cannot steer the active turn', async () => { + const { calls } = installFakeCodexAppRun('cs011', 'steered', 'jaw:slack:channel:C_A:thread:1.1'); + try { + const outcome = await steerAgent('cs011', 'belongs to B', 'slack', { + chatSessionId: 'cs-spawn', + remoteKey: 'jaw:slack:channel:C_B:thread:2.2', + }); + assert.equal(outcome, 'fallback-queue'); + assert.deepEqual(calls, [], 'foreign input never reaches the active turn'); + } finally { + activeMainProcesses.delete('cs011'); + } +}); diff --git a/tests/unit/native-delivery-producers.test.ts b/tests/unit/native-delivery-producers.test.ts index c4441ec7c..fb2caadcf 100644 --- a/tests/unit/native-delivery-producers.test.ts +++ b/tests/unit/native-delivery-producers.test.ts @@ -337,7 +337,8 @@ for (const channel of ['slack', 'discord', 'telegram']) { const pending = run(channel); await drain(); operations.length = 0; optionsSeen.length = 0; - broadcast('orchestrate_done', { origin: channel, requestId: activeRequest, text: ' \n', + broadcast('orchestrate_done', { origin: channel, requestId: activeRequest, + scope: 'default', sessionId: 'default', text: ' \n', runtimeFinality: 'present', runtimeStatus: 'done', fromQueue: true, target: target(channel) }); await pending; await drain(); assert.equal(optionsSeen.length, 0); @@ -362,6 +363,7 @@ for (const channel of ['slack', 'discord', 'telegram']) { const pending = run(channel); await drain(); operations.length = 0; optionsSeen.length = 0; broadcast('orchestrate_done', { origin: channel, requestId: activeRequest, + scope: 'default', sessionId: 'default', text: 'queued answer', fromQueue: true, target: target(channel), ...tags }); await pending; await drain(); return [...operations]; @@ -381,6 +383,7 @@ for (const channel of ['slack', 'discord', 'telegram']) { const pending = run(channel); await drain(); operations.length = 0; optionsSeen.length = 0; eraseBody = true; broadcast('orchestrate_done', { origin: channel, requestId: activeRequest, + scope: 'default', sessionId: 'default', text: 'format removes this', fromQueue: true, target: target(channel), runtimeFinality: 'absent', runtimeStatus: 'error' }); if (channel === 'telegram') await assert.rejects(pending, /empty_message/); diff --git a/tests/unit/native-orchestration-terminal.test.ts b/tests/unit/native-orchestration-terminal.test.ts index 168a9e15b..e16298a83 100644 --- a/tests/unit/native-orchestration-terminal.test.ts +++ b/tests/unit/native-orchestration-terminal.test.ts @@ -209,13 +209,16 @@ for (const finalText of ['', ' \n\t']) { test(`native ${JSON.stringify(finalText)} gets existing direct noResponse but queued completion remains empty`, async () => { const result = await orchestrateAndCollectData('native task', { ...options({ text: finalText, code: 0, runtimeOutcome: { status: 'done', finalText, partialText: 'not final' } }), - _fromQueue: true, replyViaTarget: true, target: { channel: 'slack', targetId: 'C-test' }, + _fromQueue: true, replyViaTarget: true, + target: { channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C-test' }, }, 'en'); assert.equal(result.text, t('tg.noResponse', {}, 'en')); assert.equal(result.data['text'], ''); assert.equal(result.data['fromQueue'], true); assert.equal(result.data['replyViaTarget'], true); - assert.deepEqual(result.data['target'], { channel: 'slack', targetId: 'C-test' }); + assert.deepEqual(result.data['target'], { + channel: 'slack', targetKind: 'channel', peerKind: 'channel', targetId: 'C-test', + }); assert.equal(result.data['runtimeFinality'], 'present'); assert.equal(result.data['runtimeStatus'], 'done'); }); diff --git a/tests/unit/queue-v2-migration-restart.test.ts b/tests/unit/queue-v2-migration-restart.test.ts index 5ec534831..cd9efb064 100644 --- a/tests/unit/queue-v2-migration-restart.test.ts +++ b/tests/unit/queue-v2-migration-restart.test.ts @@ -180,15 +180,19 @@ test('v2 restart preserves A/B capture and ignores a later global session switch ); }); -test('collect merges only not-yet-started items from the same scope', async () => { +test('collect merges only not-yet-started items from the same scope and remote conversation', async () => { db.prepare("INSERT INTO chat_sessions (id, seq, label) VALUES ('queue-v2-collect-a', 811, 'A')").run(); db.prepare("INSERT INTO chat_sessions (id, seq, label) VALUES ('queue-v2-collect-b', 812, 'B')").run(); const blocked = new Set(['A', 'B']); const runs: Array<{ prompt: string; meta: Record }> = []; const controller = makeController({ busy: scope => blocked.has(scope), runs }); - controller.enqueueMessage('queue-v2-collect-a2', 'web', { scope: 'A', chatSessionId: 'queue-v2-collect-a', collect: true }); - controller.enqueueMessage('queue-v2-collect-a3', 'web', { scope: 'A', chatSessionId: 'queue-v2-collect-a', collect: true }); + controller.enqueueMessage('queue-v2-collect-a2', 'slack', { scope: 'A', chatSessionId: 'queue-v2-collect-a', + remoteKey: 'jaw:slack:channel:C_A:thread:1.1', collect: true }); + controller.enqueueMessage('queue-v2-collect-a3', 'slack', { scope: 'A', chatSessionId: 'queue-v2-collect-a', + remoteKey: 'jaw:slack:channel:C_A:thread:1.1', collect: true }); + controller.enqueueMessage('queue-v2-collect-a-foreign', 'slack', { scope: 'A', chatSessionId: 'queue-v2-collect-a', + remoteKey: 'jaw:slack:channel:C_FOREIGN:thread:2.2', collect: true }); controller.enqueueMessage('queue-v2-collect-b1', 'web', { scope: 'B', chatSessionId: 'queue-v2-collect-b', collect: true }); blocked.clear(); @@ -200,10 +204,12 @@ test('collect merges only not-yet-started items from the same scope', async () = assert.deepEqual(runs.map(run => ({ prompt: run.prompt, scope: run.meta.scope })), [ { prompt: 'queue-v2-collect-a2\n\nqueue-v2-collect-a3', scope: 'A' }, { prompt: 'queue-v2-collect-b1', scope: 'B' }, + { prompt: 'queue-v2-collect-a-foreign', scope: 'A' }, ]); const rows = db.prepare("SELECT content, session_id FROM messages WHERE content LIKE 'queue-v2-collect-%' ORDER BY session_id").all(); assert.deepEqual(rows, [ { content: 'queue-v2-collect-a2\n\nqueue-v2-collect-a3', session_id: 'queue-v2-collect-a' }, + { content: 'queue-v2-collect-a-foreign', session_id: 'queue-v2-collect-a' }, { content: 'queue-v2-collect-b1', session_id: 'queue-v2-collect-b' }, ]); }); diff --git a/tests/unit/request-settle-contract.test.ts b/tests/unit/request-settle-contract.test.ts index 78bdfb577..a93405841 100644 --- a/tests/unit/request-settle-contract.test.ts +++ b/tests/unit/request-settle-contract.test.ts @@ -9,6 +9,7 @@ import { settleOnce, sweepStaleRequests, } from '../../src/orchestrator/request-registry.ts'; +import { addBroadcastListener, removeBroadcastListener } from '../../src/core/bus.ts'; // #276 prerequisite. POST /api/message always returned a requestId, but that id // could not tell a caller when the request was DONE: an accepted mid-run steer @@ -20,6 +21,43 @@ import { test.beforeEach(() => { resetRequestRegistryForTest(); }); +test('a settlement carries the immutable run pin captured at admission', () => { + const target = { + channel: 'slack' as const, + targetKind: 'channel' as const, + peerKind: 'channel' as const, + targetId: 'C_PIN', + threadId: '1.1', + }; + const seen: Record[] = []; + const listener = (type: string, data: Record) => { + if (type === 'request_settled') seen.push(data); + }; + addBroadcastListener(listener); + try { + admitRequest('pinned', 'scope-pin', 100, { + origin: 'slack', + sessionId: 'session-pin', + remoteKey: 'jaw:slack:channel:C_PIN:thread:1.1', + target, + }); + // Mutation after admission cannot rewrite the settlement address. + target.targetId = 'C_MUTATED'; + settleOnce('pinned', 'completed'); + } finally { + removeBroadcastListener(listener); + } + assert.deepEqual(seen, [{ + requestId: 'pinned', + origin: 'slack', + scope: 'scope-pin', + sessionId: 'session-pin', + remoteKey: 'jaw:slack:channel:C_PIN:thread:1.1', + target: { ...target, targetId: 'C_PIN' }, + outcome: 'completed', + }]); +}); + test('settleOnce is idempotent — a request cannot settle twice', () => { admitRequest('r1', 'default'); assert.equal(settleOnce('r1', 'completed', { text: 'hi' }), true); diff --git a/tests/unit/run-pin.test.ts b/tests/unit/run-pin.test.ts new file mode 100644 index 000000000..936c75c2c --- /dev/null +++ b/tests/unit/run-pin.test.ts @@ -0,0 +1,64 @@ +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { matchesRunPin, sameRunConversation } from '../../src/messaging/run-pin.ts'; +import type { RunPin } from '../../src/messaging/run-pin.ts'; + +const expected: RunPin = { + requestId: 'request-1', + origin: 'slack', + scope: 'scope-1', + sessionId: 'session-1', + remoteKey: 'jaw:slack:channel:C1:thread:1.1', + target: { + channel: 'slack', + targetKind: 'channel', + peerKind: 'channel', + targetId: 'C1', + threadId: '1.1', + }, +}; + +test('RP-001 a complete identical pin matches', () => { + assert.equal(matchesRunPin(expected, { ...expected, target: { ...expected.target } }), true); +}); + +test('RP-002 every core identity field is required', () => { + for (const field of ['requestId', 'origin', 'scope', 'sessionId'] as const) { + const actual = { ...expected, target: { ...expected.target } }; + delete actual[field]; + assert.equal(matchesRunPin(expected, actual), false, field); + } +}); + +test('RP-003 a remote waiter requires the same remoteKey and target', () => { + assert.equal(matchesRunPin(expected, { + ...expected, + remoteKey: 'jaw:slack:channel:C2:thread:2.2', + target: { ...expected.target }, + }), false); + assert.equal(matchesRunPin(expected, { + ...expected, + target: { ...expected.target, threadId: '2.2' }, + }), false); +}); + +test('RP-004 a local waiter does not invent remote fields', () => { + const local: RunPin = { + requestId: 'request-local', + origin: 'web', + scope: 'scope-local', + sessionId: 'session-local', + }; + assert.equal(matchesRunPin(local, { ...local }), true); +}); + +test('RP-005 remote conversation equality follows remoteKey, not reply placement', () => { + assert.equal(sameRunConversation( + { origin: 'slack', remoteKey: 'jaw:slack:channel:C1', target: { ...expected.target!, threadId: '1.1' } }, + { origin: 'slack', remoteKey: 'jaw:slack:channel:C1', target: { ...expected.target!, threadId: '2.2' } }, + ), true, 'synthetic reply placement must not mint another conversation'); + assert.equal(sameRunConversation( + { origin: 'slack', remoteKey: 'jaw:slack:channel:C1' }, + { origin: 'slack', remoteKey: 'jaw:slack:channel:C2' }, + ), false); +}); diff --git a/tests/unit/slack-agent-done-destination.test.ts b/tests/unit/slack-agent-done-destination.test.ts new file mode 100644 index 000000000..a5aca321b --- /dev/null +++ b/tests/unit/slack-agent-done-destination.test.ts @@ -0,0 +1,99 @@ +import '../setup/isolated-home.ts'; +import test from 'node:test'; +import assert from 'node:assert/strict'; +import { createSlackForwarder } from '../../src/slack/forwarder.ts'; +import { setLastActiveTarget } from '../../src/messaging/runtime.ts'; +import type { RemoteTarget } from '../../src/messaging/types.ts'; + +const runTarget: RemoteTarget = { + channel: 'slack', + targetKind: 'channel', + peerKind: 'channel', + targetId: 'C_RUN', + threadId: '1710000000.000200', +}; + +const newestConversation: RemoteTarget = { + channel: 'slack', + targetKind: 'channel', + peerKind: 'channel', + targetId: 'C_NEWEST', + threadId: '1710000000.000300', +}; + +test('SAD-742: agent_done stays on its captured thread after last-active moves', async t => { + const posts: Array> = []; + t.mock.method(globalThis, 'fetch', async (url: string | URL | Request, init?: RequestInit) => { + assert.equal(String(url), 'https://slack.com/api/chat.postMessage'); + posts.push(JSON.parse(String(init?.body)) as Record); + return new Response(JSON.stringify({ ok: true, ts: '1710000000.000201' }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }); + }); + + // The exact incident order: a run owns one conversation, then somebody + // speaks elsewhere before that run finishes. The global slot now points at + // the new conversation, but the terminal event still belongs to the first. + setLastActiveTarget('slack', runTarget); + // A new mention arrives while the first run is still working. + setLastActiveTarget('slack', newestConversation); + const forward = createSlackForwarder({ getToken: () => 'xoxb-fixture' }); + await forward('agent_done', { + origin: 'web', + requestId: 'request-run', + scope: 'scope-run', + sessionId: 'session-run', + remoteKey: 'jaw:slack:channel:C_RUN:thread:1710000000.000200', + target: runTarget, + text: 'answer for the run', + }); + + assert.equal(posts.length, 1); + assert.equal(posts[0]?.['channel'], runTarget.targetId); + assert.equal(posts[0]?.['thread_ts'], runTarget.threadId); + assert.equal(posts[0]?.['text'], 'answer for the run'); + assert.notEqual(posts[0]?.['channel'], newestConversation.targetId); +}); + +test('SAD-742: a targetless terminal never borrows last-active', async t => { + let fetches = 0; + t.mock.method(globalThis, 'fetch', async () => { + fetches++; + throw new Error('targetless completion must not reach the transport'); + }); + + setLastActiveTarget('slack', newestConversation); + const forward = createSlackForwarder({ getToken: () => 'xoxb-fixture' }); + await forward('agent_done', { + origin: 'web', + requestId: 'request-web', + scope: 'scope-web', + sessionId: 'session-web', + text: 'web-only answer', + }); + + assert.equal(fetches, 0); +}); + +test('SAD-742: a terminal addressed to another transport never reaches Slack', async t => { + let fetches = 0; + t.mock.method(globalThis, 'fetch', async () => { + fetches++; + throw new Error('cross-channel completion must not reach Slack'); + }); + + const forward = createSlackForwarder({ getToken: () => 'xoxb-fixture' }); + await forward('agent_done', { + origin: 'web', + target: { + channel: 'discord', + targetKind: 'channel', + peerKind: 'channel', + targetId: 'D_RUN', + }, + text: 'discord answer', + }); + + assert.equal(fetches, 0); +}); diff --git a/tests/unit/slack-delivery-helper.test.ts b/tests/unit/slack-delivery-helper.test.ts index 4b0bc3c21..7a05a3227 100644 --- a/tests/unit/slack-delivery-helper.test.ts +++ b/tests/unit/slack-delivery-helper.test.ts @@ -162,7 +162,7 @@ test('the direct path and the queued tracker deliver through the same helper', a await pending; operations.length = 0; broadcast('orchestrate_done', { - origin: 'slack', requestId: activeRequest, text: 'answer', + origin: 'slack', requestId: activeRequest, scope: 'default', sessionId: 'default', text: 'answer', fromQueue: true, target: target(), }); await drain(); diff --git a/tests/unit/slack-progress-stream.test.ts b/tests/unit/slack-progress-stream.test.ts index 83b080dac..f1aef55ed 100644 --- a/tests/unit/slack-progress-stream.test.ts +++ b/tests/unit/slack-progress-stream.test.ts @@ -4,6 +4,7 @@ import { fileURLToPath } from 'node:url'; import { loadLocales } from '../../src/core/i18n.ts'; import { startSlackProgress } from '../../src/slack/progress.ts'; import type { RemoteTarget } from '../../src/messaging/types.ts'; +import { __resetMessagingMetricsForTests, snapshotMetrics } from '../../src/messaging/metrics.ts'; loadLocales(fileURLToPath(new URL('../../public/locales', import.meta.url))); async function settle() { for (let i = 0; i < 20; i++) await Promise.resolve(); } @@ -292,6 +293,7 @@ test('a stream closed after five minutes continues editing its own status throug }); test('SPS-744: an expired stream whose message is gone stops instead of retrying forever', async () => { + __resetMessagingMetricsForTests(); // The live incident: the stream expired, the card fell back to chat.update, // and the message was no longer there. Nothing promoted that to "stop", so // the idle loop re-dirtied the snapshot every 3.2s and sent the same doomed @@ -318,6 +320,12 @@ test('SPS-744: an expired stream whose message is gone stops instead of retrying await p.finish('complete', { bodyDelivered: true }); assert.equal(h.calls.length, before, 'finalizing spends no request on a message known to be gone'); assert.equal(p.terminalConfirmed(), true); + assert.deepEqual(snapshotMetrics().counters.filter(row => + row.name === 'slack.progress.stream_state_lost'), [{ + name: 'slack.progress.stream_state_lost', + labels: { channel: 'slack', result: 'message_not_found' }, + value: 1, + }], 'one lost card produces one metric, not one per idle tick'); }); test('SPS-744: three consecutive failed edits stop the live loop', async () => { diff --git a/tests/unit/slack-workflow-routing.test.ts b/tests/unit/slack-workflow-routing.test.ts index e2a03cb74..7ba2fb584 100644 --- a/tests/unit/slack-workflow-routing.test.ts +++ b/tests/unit/slack-workflow-routing.test.ts @@ -135,7 +135,9 @@ function done(index: number, extra: RecordData = {}): RecordData { const admission = admissions[index]!; const session = admission.result.sessionContext!; return { origin: 'slack', requestId: admission.result.requestId, scope: session.scope, - sessionId: session.chatSessionId, target, fromQueue: true, text: 'queued answer', ...extra }; + sessionId: session.chatSessionId, target, + ...(session.remoteKey ? { remoteKey: session.remoteKey } : {}), + fromQueue: true, text: 'queued answer', ...extra }; } test.beforeEach(async context => { diff --git a/tests/unit/steer-acp-inband-not-superseded.test.ts b/tests/unit/steer-acp-inband-not-superseded.test.ts index 6b1606be3..c578ae6d5 100644 --- a/tests/unit/steer-acp-inband-not-superseded.test.ts +++ b/tests/unit/steer-acp-inband-not-superseded.test.ts @@ -27,7 +27,11 @@ test('ACP in-band steer with real text still delivers the answer', async () => { // cancel-reprompt: a later request steers in-band, then the SAME turn answers. broadcast('steer_started', { origin: 'slack', scope: 'default', sessionId: 'default', requestId: 'req-later', mode: 'cancel-reprompt', localDispatch: true }); - broadcast('orchestrate_done', { ...lastMeta, text: '두 질문에 대한 답변' }); + broadcast('orchestrate_done', { + ...lastMeta, + sessionId: lastMeta['chatSessionId'], + text: '두 질문에 대한 답변', + }); const result = await pending; assert.equal(result.text, '두 질문에 대한 답변', diff --git a/tests/unit/steer-input-handoff.test.ts b/tests/unit/steer-input-handoff.test.ts index 963de12d7..8bba6d8f8 100644 --- a/tests/unit/steer-input-handoff.test.ts +++ b/tests/unit/steer-input-handoff.test.ts @@ -11,6 +11,8 @@ const queued: unknown[][] = []; const steered: unknown[][] = []; test.mock.module('../../src/agent/spawn.js', { namedExports: { isAgentBusy: () => true, messageQueue: [], purgeQueueOnStop: () => {}, + getCurrentMainMeta: () => null, + hasActiveMainReplacement: () => false, canSteerAgent: () => assert.fail('gateway must delegate capability choice, not pre-queue'), killActiveAgent: () => assert.fail('gateway must delegate steer interruption to steerAgent'), enqueueMessage: (...args: unknown[]) => { queued.push(args); return 'fixture-queued'; }, diff --git a/tests/unit/steer-superseded-delivery.test.ts b/tests/unit/steer-superseded-delivery.test.ts index a8991c4d4..2bedbe330 100644 --- a/tests/unit/steer-superseded-delivery.test.ts +++ b/tests/unit/steer-superseded-delivery.test.ts @@ -10,7 +10,6 @@ import { broadcast } from '../../src/core/bus.ts'; // 1. the killed turn must not resolve to the placeholder, and // 2. the follow-up terminal must be marked so the standing forwarder delivers it. -let lastMeta: Record = {}; // The locale table is not loaded in this harness, so `t()` yields the raw key. // Asserting on the key keeps the test about WHICH string is chosen, not its wording. const NO_RESPONSE = 'tg.noResponse'; @@ -20,7 +19,12 @@ const STOPPED = 'tg.stopped'; // on purpose, so a payload that forgets these fields does not fail — it hangs. // Bounding the wait turns that into a readable assertion instead of an // eight-minute shard timeout. -const NATIVE_IDENTITY = { origin: 'slack', scope: 'default', sessionId: 'default' }; +const terminalPin = (requestId: string) => ({ + requestId, + origin: 'slack', + scope: 'default', + sessionId: 'default', +}); async function settled(pending: Promise, label: string): Promise { let timer: ReturnType | undefined; const guard = new Promise((_, reject) => { @@ -40,7 +44,7 @@ test('a native run that was stopped says so, instead of reporting no response', const pending = orchestrateAndCollectData('질문', { origin: 'slack', requestId: 'req-stopped', scope: 'default', chatSessionId: 'default', }); - broadcast('orchestrate_done', { ...lastMeta, ...NATIVE_IDENTITY, text: '', runtimeFinality: 'absent', runtimeStatus: 'stopped' }); + broadcast('orchestrate_done', { ...terminalPin('req-stopped'), text: '', runtimeFinality: 'absent', runtimeStatus: 'stopped' }); const result = await settled(pending, 'native stopped'); assert.equal(result.text, STOPPED); @@ -52,7 +56,7 @@ test('a legacy interrupted run says so too', async () => { const pending = orchestrateAndCollectData('질문', { origin: 'slack', requestId: 'req-interrupted', scope: 'default', chatSessionId: 'default', }); - broadcast('orchestrate_done', { ...lastMeta, text: '', executionInterrupted: true }); + broadcast('orchestrate_done', { ...terminalPin('req-interrupted'), text: '', executionInterrupted: true }); const result = await settled(pending, 'legacy interrupted'); assert.equal(result.text, STOPPED); @@ -65,7 +69,7 @@ test('a native run that finished with nothing to say keeps the no-response copy' const pending = orchestrateAndCollectData('질문', { origin: 'slack', requestId: 'req-done-empty', scope: 'default', chatSessionId: 'default', }); - broadcast('orchestrate_done', { ...lastMeta, ...NATIVE_IDENTITY, text: '', runtimeFinality: 'absent', runtimeStatus: 'done' }); + broadcast('orchestrate_done', { ...terminalPin('req-done-empty'), text: '', runtimeFinality: 'absent', runtimeStatus: 'done' }); const result = await settled(pending, 'native done empty'); assert.equal(result.text, NO_RESPONSE); @@ -79,7 +83,7 @@ test('a steer still wins over the stopped copy, because the follow-up owns the a origin: 'slack', requestId: 'req-steered-stop', scope: 'default', chatSessionId: 'default', }); broadcast('steer_started', { origin: 'slack', scope: 'default', sessionId: 'default', requestId: 'req-next' }); - broadcast('orchestrate_done', { ...lastMeta, ...NATIVE_IDENTITY, text: '', runtimeFinality: 'absent', runtimeStatus: 'stopped' }); + broadcast('orchestrate_done', { ...terminalPin('req-steered-stop'), text: '', runtimeFinality: 'absent', runtimeStatus: 'stopped' }); const result = await settled(pending, 'steer beats stopped'); assert.equal(result.text, ''); @@ -92,9 +96,7 @@ test.mock.module('../../src/orchestrator/pipeline.ts', { isResetIntent: () => false, orchestrateContinue: () => undefined, orchestrateReset: () => undefined, - orchestrate: (_prompt: string, meta: Record) => { - lastMeta = meta; - }, + orchestrate: (_prompt: string, _meta: Record) => undefined, }, }); @@ -106,7 +108,7 @@ test('a turn retired by a steer resolves empty, never the no-response placeholde // A LATER request steers this scope: the running process is killed, so its // terminal carries no text. broadcast('steer_started', { origin: 'slack', scope: 'default', sessionId: 'default', requestId: 'req-steer' }); - broadcast('orchestrate_done', { ...lastMeta, text: '' }); + broadcast('orchestrate_done', { ...terminalPin('req-original'), text: '' }); const result = await pending; assert.equal(result.text, '', 'a superseded turn must produce no user-visible text'); @@ -119,7 +121,7 @@ test('an ordinary empty terminal still reports no response', async () => { const pending = orchestrateAndCollectData('원본 질문', { origin: 'slack', requestId: 'req-plain', scope: 'default', chatSessionId: 'default', }); - broadcast('orchestrate_done', { ...lastMeta, text: '' }); + broadcast('orchestrate_done', { ...terminalPin('req-plain'), text: '' }); const result = await pending; assert.equal(result.text, NO_RESPONSE, 'a genuinely empty turn keeps its existing diagnostic'); @@ -132,7 +134,7 @@ test("another scope's steer does not retire this turn", async () => { origin: 'slack', requestId: 'req-scoped', scope: 'default', chatSessionId: 'default', }); broadcast('steer_started', { origin: 'slack', scope: 'other-scope', sessionId: 'other', requestId: 'req-elsewhere' }); - broadcast('orchestrate_done', { ...lastMeta, text: '' }); + broadcast('orchestrate_done', { ...terminalPin('req-scoped'), text: '' }); const result = await pending; assert.equal(result.text, NO_RESPONSE); diff --git a/tests/unit/steer-superseded-sink.test.ts b/tests/unit/steer-superseded-sink.test.ts index 8a77054b9..322097e18 100644 --- a/tests/unit/steer-superseded-sink.test.ts +++ b/tests/unit/steer-superseded-sink.test.ts @@ -59,7 +59,7 @@ test('SS-001: a turn retired by a real kill-steer event hands the sink nothing', prompt: 'replacement question', source: 'slack', scopeKey: 'default', chatSessionId: 'default', meta: { requestId: 'req-steer' }, mode: 'kill-steer', })); - broadcast('orchestrate_done', { ...lastMeta, text: '' }); + broadcast('orchestrate_done', { ...lastMeta, sessionId: lastMeta['chatSessionId'], text: '' }); const result = await pending; sink.send(result.text); @@ -76,7 +76,7 @@ test('SS-002: the same sink DOES receive a genuinely empty turn', async () => { const pending = orchestrateAndCollectData('original question', { origin: 'slack', requestId: 'req-plain', scope: 'default', chatSessionId: 'default', }); - broadcast('orchestrate_done', { ...lastMeta, text: '' }); + broadcast('orchestrate_done', { ...lastMeta, sessionId: lastMeta['chatSessionId'], text: '' }); const result = await pending; sink.send(result.text); diff --git a/tests/unit/stop-clears-queue.test.ts b/tests/unit/stop-clears-queue.test.ts index 0a7849889..568ac4140 100644 --- a/tests/unit/stop-clears-queue.test.ts +++ b/tests/unit/stop-clears-queue.test.ts @@ -174,7 +174,7 @@ test('Fix C3: non-stop kill reasons do not clear main live-run snapshots', async test('Fix B2: enqueueMessage returns the queue id and gateway threads it into SubmitResult.queuedId', () => { const enqueueIdx = queueSrc.indexOf('function enqueueMessage'); - const enqueue = queueSrc.slice(enqueueIdx, enqueueIdx + 1800); + const enqueue = queueSrc.slice(enqueueIdx, enqueueIdx + 2600); assert.ok(/\): string \{/.test(enqueue), 'enqueueMessage must declare string return type'); assert.ok(/return item\.id/.test(enqueue), 'enqueueMessage must return the queue item id'); diff --git a/tests/unit/submit-message.test.ts b/tests/unit/submit-message.test.ts index 864425e56..b5b5c923b 100644 --- a/tests/unit/submit-message.test.ts +++ b/tests/unit/submit-message.test.ts @@ -214,3 +214,16 @@ test('SM-019: collect and interrupt remain queued without disposition', () => { assert.ok(queueReturn); assert.doesNotMatch(queueReturn, /disposition/); }); + +test('SM-020: a steer policy queues input from another remote conversation', () => { + const policy = gatewaySrc.slice( + gatewaySrc.indexOf('function applyMidRunPolicy'), + gatewaySrc.indexOf('// ── 5s dedup window'), + ); + const steer = policy.slice(policy.indexOf("if (policy === 'steer')")); + const guard = steer.indexOf('sameRunConversation('); + const dispatch = steer.indexOf('steerAgent('); + assert.ok(guard >= 0 && dispatch > guard, + 'cross-conversation guard must run before any steer input is dispatched'); + assert.match(steer.slice(guard, dispatch), /return queue\(\)/); +});