From 8c305bcb179dfbcd95dfb6021ed00f1e0aaab95f Mon Sep 17 00:00:00 2001 From: suji lee Date: Fri, 11 Sep 2026 23:21:07 +0900 Subject: [PATCH 1/6] test(slack): reproduce last-active overwrite during agent_done delivery --- .../unit/slack-agent-done-destination.test.ts | 99 +++++++++++++++++++ 1 file changed, 99 insertions(+) create mode 100644 tests/unit/slack-agent-done-destination.test.ts 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); +}); From 9ef0a7da342a241dbefa687e2940227194617977 Mon Sep 17 00:00:00 2001 From: suji lee Date: Fri, 11 Sep 2026 23:36:20 +0900 Subject: [PATCH 2/6] fix(orchestrator): fail closed on cross-conversation run identity A Slack waiter used to require requestId but treat every absent scope, session, origin, remoteKey, and target as agreement. The print terminal that triggered the September 11 incident omitted those fields, so a new waiter could adopt an older run's control or completion event. Run-pin matching now requires requestId/origin/scope/session and, when the waiter owns them, the exact remoteKey and target. Producers are filled first: request settlement retains the admission pin, queue start/update and failure events carry it, and Slack terminals expose scope/session/remoteKey even when multi-session mode is off. The collector applies the same rule to Slack orchestrate_done events. A steer can mutate only the conversation that owns the active run. Another remoteKey is queued as a separate follow-up before in-band or kill-steer input is dispatched, and collect batches no longer join different remote keys that happen to share a scope. Preserves the existing boot-drain/orphan target-reply contract and synthetic thread keying. Refs #743 --- src/agent/spawn.ts | 13 +++- src/agent/spawn/queue.ts | 30 +++++++-- src/messaging/run-pin.ts | 34 +++++++++- src/orchestrator/collect.ts | 12 ++++ src/orchestrator/gateway.ts | 29 +++++++-- src/orchestrator/pipeline.ts | 35 ++++++++-- src/orchestrator/request-registry.ts | 35 ++++++++-- src/slack/bot.ts | 19 +++--- src/slack/commands.ts | 7 +- src/slack/progress-lifecycle.ts | 11 ++-- tests/unit/codex-app-steer.test.ts | 27 ++++++-- tests/unit/queue-v2-migration-restart.test.ts | 12 +++- tests/unit/request-settle-contract.test.ts | 38 +++++++++++ tests/unit/run-pin.test.ts | 64 +++++++++++++++++++ tests/unit/steer-superseded-delivery.test.ts | 26 ++++---- tests/unit/submit-message.test.ts | 13 ++++ 16 files changed, 348 insertions(+), 57 deletions(-) create mode 100644 tests/unit/run-pin.test.ts diff --git a/src/agent/spawn.ts b/src/agent/spawn.ts index c012221cb..3ce319564 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'; @@ -866,6 +866,17 @@ 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 }, + { origin: source, remoteKey: meta?.remoteKey }, + )) { + // 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..c44309e30 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; @@ -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..669a05532 100644 --- a/src/orchestrator/collect.ts +++ b/src/orchestrator/collect.ts @@ -16,6 +16,7 @@ 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'; export interface CollectedOrchestrateResult { text: string; @@ -51,6 +52,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: meta['target'], + } + : null; let collected = ''; let ownTerminalDiagnostic = ''; let nativeSeen = false; @@ -136,6 +147,7 @@ export function orchestrateAndCollectData( 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..01dcff834 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, 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'; @@ -25,6 +25,7 @@ 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,17 @@ function applyMidRunPolicy( }; if (policy === 'steer') { + const owner = getCurrentMainMeta(ctx.scopeKey); + if (owner && !sameRunConversation( + { origin: owner.origin, remoteKey: owner.remoteKey }, + { origin: ctx.meta.origin, remoteKey: ctx.remoteKey }, + )) { + // 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 +183,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 +195,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 +245,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 +258,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..2a8cff473 100644 --- a/src/orchestrator/pipeline.ts +++ b/src/orchestrator/pipeline.ts @@ -7,6 +7,7 @@ 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 { clearAllEmployeeSessions, getRecentMessagesLite, @@ -286,6 +287,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 +373,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 +749,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 +791,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 +833,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 +848,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/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/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/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/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\(\)/); +}); From 4bd31463b8a20cf07f6db4975a9e2b873028a17a Mon Sep 17 00:00:00 2001 From: suji lee Date: Fri, 11 Sep 2026 23:47:30 +0900 Subject: [PATCH 3/6] test(orchestrator): pin queued and steered terminal fixtures Update queued Slack and collector fixtures to carry the production terminal pin, and lock the new behavior: a foreign remoteKey waits as its own turn, collect does not join another remote conversation, and request settlement preserves the admission target even if its caller mutates the original object. --- tests/unit/native-delivery-producers.test.ts | 5 ++++- tests/unit/native-grok-spawn.test.ts | 22 +++++++++++++++---- .../native-orchestration-terminal.test.ts | 7 ++++-- .../steer-acp-inband-not-superseded.test.ts | 6 ++++- tests/unit/steer-superseded-sink.test.ts | 4 ++-- 5 files changed, 34 insertions(+), 10 deletions(-) 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-grok-spawn.test.ts b/tests/unit/native-grok-spawn.test.ts index 9c15709c7..02cb987f5 100644 --- a/tests/unit/native-grok-spawn.test.ts +++ b/tests/unit/native-grok-spawn.test.ts @@ -295,7 +295,10 @@ test('real CLI steer propagates fatal dispatch without its follow-up queue path' }); for (const conflict of ['chat', 'remoteKey', 'target', 'guild', 'targetKind'] as const) { - test(`gateway rejects foreign ${conflict} before replacement dispatch, row insertion or queueing`, { timeout: 5000 }, async t => { + test(conflict === 'remoteKey' + ? 'gateway queues a foreign remoteKey as its own turn before replacement dispatch' + : `gateway rejects foreign ${conflict} before replacement dispatch, row insertion or queueing`, + { timeout: 5000 }, async t => { const { submitMessage } = await import('../../src/orchestrator/gateway.ts'); const target: RemoteTarget = { channel: 'slack', peerKind: 'channel', targetKind: 'channel', targetId: 'COWNER', threadId: '1.001', guildId: 'TOWNER' }; @@ -313,14 +316,25 @@ for (const conflict of ['chat', 'remoteKey', 'target', 'guild', 'targetKind'] as remoteKey: conflict === 'remoteKey' ? 'jaw:slack:channel:CFOREIGN' : opts.remoteKey, target: suppliedTarget }); await new Promise(resolve => setImmediate(resolve)); const settled = events.filter(e => e.type === 'request_settled' && e.data['requestId'] === submitted.requestId); - assert.equal(settled.length, 1); assert.equal(settled[0]!.data['outcome'], 'failed'); - assert.equal(settled[0]!.data['error'], 'native_replacement_owner_mismatch'); + if (conflict === 'remoteKey') { + // Another conversation is valid work, just not a steer of this + // owner. It waits as its own turn instead of being failed or + // injected into the run currently on the wire (#743). + assert.equal(submitted.action, 'queued'); + assert.equal(settled.length, 0); + assert.equal(messageQueue.some(item => item.id === submitted.queuedId + && item.remoteKey === 'jaw:slack:channel:CFOREIGN'), true); + } else { + assert.equal(settled.length, 1); assert.equal(settled[0]!.data['outcome'], 'failed'); + assert.equal(settled[0]!.data['error'], 'native_replacement_owner_mismatch'); + } assert.equal(hook.mock.callCount(), 0); assert.deepEqual(db.prepare('SELECT COUNT(*) AS n FROM messages').get(), before); - assert.equal(messageQueue.some(item => item.scope === opts.scopeKey), false); + assert.equal(messageQueue.some(item => item.scope === opts.scopeKey), conflict === 'remoteKey'); assert.equal(wire().filter(e => e['method'] === 'session/prompt').length, 1); assert.equal(wire().some(e => e['method'] === 'session/cancel'), false); assert.equal(events.some(e => e.type === 'steer_started' || e.type === 'steer_rejected' || e.type === 'new_message'), false); + if (conflict === 'remoteKey' && submitted.queuedId) removeQueuedMessage(submitted.queuedId); } finally { off(); killActiveAgent(opts.scopeKey, 'user'); await run.promise; } }); } 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/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-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); From 58648b8ac90c06c82a28105a1040c3296dcfb185 Mon Sep 17 00:00:00 2001 From: suji lee Date: Fri, 11 Sep 2026 23:51:15 +0900 Subject: [PATCH 4/6] test(slack): align queued producer fixtures with strict run pins --- tests/unit/slack-delivery-helper.test.ts | 2 +- tests/unit/slack-workflow-routing.test.ts | 4 +++- tests/unit/steer-input-handoff.test.ts | 1 + tests/unit/stop-clears-queue.test.ts | 2 +- 4 files changed, 6 insertions(+), 3 deletions(-) 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-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-input-handoff.test.ts b/tests/unit/steer-input-handoff.test.ts index 963de12d7..b9bbd19a0 100644 --- a/tests/unit/steer-input-handoff.test.ts +++ b/tests/unit/steer-input-handoff.test.ts @@ -11,6 +11,7 @@ const queued: unknown[][] = []; const steered: unknown[][] = []; test.mock.module('../../src/agent/spawn.js', { namedExports: { isAgentBusy: () => true, messageQueue: [], purgeQueueOnStop: () => {}, + getCurrentMainMeta: () => null, 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/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'); From 02bfb1548bc40ea264f2cedceb39da316ca9f359 Mon Sep 17 00:00:00 2001 From: suji lee Date: Fri, 11 Sep 2026 23:53:43 +0900 Subject: [PATCH 5/6] test(slack): count a lost progress stream once --- tests/unit/slack-progress-stream.test.ts | 8 ++++++++ 1 file changed, 8 insertions(+) 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 () => { From d51889ab582e299de420a1945bd2808e0691b050 Mon Sep 17 00:00:00 2001 From: suji lee Date: Sat, 12 Sep 2026 00:37:31 +0900 Subject: [PATCH 6/6] fix(orchestrator): preserve native owner mismatch across remote keys Independent review found that the new gateway follow-up guard ran before native replaceTurn validation. A foreign remoteKey was queued with the native owner's scope, session, and target, which could store another conversation's prompt in the owner transcript. Native replacements now retain their typed owner-mismatch failure. Non-native in-band and kill-steer paths still queue another remote conversation, deriving the owner key from its captured target when remoteKey is absent. Terminal targets are snapshotted, incomplete targets are omitted from pins, and steer supersession requires the exact origin/session/remoteKey. Refs #743 --- src/agent/spawn.ts | 12 ++++++++++-- src/messaging/run-pin.ts | 2 +- src/orchestrator/collect.ts | 7 +++++-- src/orchestrator/gateway.ts | 20 +++++++++++++++----- src/orchestrator/pipeline.ts | 7 ++++++- structure/str_func.md | 16 ++++++++-------- tests/unit/native-grok-spawn.test.ts | 22 ++++------------------ tests/unit/steer-input-handoff.test.ts | 1 + 8 files changed, 50 insertions(+), 37 deletions(-) diff --git a/src/agent/spawn.ts b/src/agent/spawn.ts index 3ce319564..7412e6f42 100644 --- a/src/agent/spawn.ts +++ b/src/agent/spawn.ts @@ -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( @@ -867,8 +873,10 @@ export async function steerAgent( return 'fallback-queue'; } if (run && !sameRunConversation( - { origin: run.meta.origin, remoteKey: run.meta.remoteKey }, - { origin: source, remoteKey: meta?.remoteKey }, + { 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 diff --git a/src/messaging/run-pin.ts b/src/messaging/run-pin.ts index c44309e30..840a9052d 100644 --- a/src/messaging/run-pin.ts +++ b/src/messaging/run-pin.ts @@ -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; diff --git a/src/orchestrator/collect.ts b/src/orchestrator/collect.ts index 669a05532..43a3bcabd 100644 --- a/src/orchestrator/collect.ts +++ b/src/orchestrator/collect.ts @@ -17,6 +17,7 @@ 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; @@ -59,7 +60,7 @@ export function orchestrateAndCollectData( scope: binding.scope, sessionId: binding.chatSessionId, remoteKey: meta['remoteKey'], - target: meta['target'], + target: isRemoteTarget(meta['target']) ? { ...meta['target'] } : undefined, } : null; let collected = ''; @@ -142,7 +143,9 @@ 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; } diff --git a/src/orchestrator/gateway.ts b/src/orchestrator/gateway.ts index 01dcff834..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 { getCurrentMainMeta, 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,7 +19,7 @@ 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'; @@ -105,14 +105,24 @@ function applyMidRunPolicy( if (policy === 'steer') { const owner = getCurrentMainMeta(ctx.scopeKey); if (owner && !sameRunConversation( - { origin: owner.origin, remoteKey: owner.remoteKey }, - { origin: ctx.meta.origin, remoteKey: ctx.remoteKey }, + { 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(); + 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 diff --git a/src/orchestrator/pipeline.ts b/src/orchestrator/pipeline.ts index 2a8cff473..8a67cc042 100644 --- a/src/orchestrator/pipeline.ts +++ b/src/orchestrator/pipeline.ts @@ -8,6 +8,7 @@ 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, @@ -73,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) { 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/native-grok-spawn.test.ts b/tests/unit/native-grok-spawn.test.ts index 02cb987f5..9c15709c7 100644 --- a/tests/unit/native-grok-spawn.test.ts +++ b/tests/unit/native-grok-spawn.test.ts @@ -295,10 +295,7 @@ test('real CLI steer propagates fatal dispatch without its follow-up queue path' }); for (const conflict of ['chat', 'remoteKey', 'target', 'guild', 'targetKind'] as const) { - test(conflict === 'remoteKey' - ? 'gateway queues a foreign remoteKey as its own turn before replacement dispatch' - : `gateway rejects foreign ${conflict} before replacement dispatch, row insertion or queueing`, - { timeout: 5000 }, async t => { + test(`gateway rejects foreign ${conflict} before replacement dispatch, row insertion or queueing`, { timeout: 5000 }, async t => { const { submitMessage } = await import('../../src/orchestrator/gateway.ts'); const target: RemoteTarget = { channel: 'slack', peerKind: 'channel', targetKind: 'channel', targetId: 'COWNER', threadId: '1.001', guildId: 'TOWNER' }; @@ -316,25 +313,14 @@ for (const conflict of ['chat', 'remoteKey', 'target', 'guild', 'targetKind'] as remoteKey: conflict === 'remoteKey' ? 'jaw:slack:channel:CFOREIGN' : opts.remoteKey, target: suppliedTarget }); await new Promise(resolve => setImmediate(resolve)); const settled = events.filter(e => e.type === 'request_settled' && e.data['requestId'] === submitted.requestId); - if (conflict === 'remoteKey') { - // Another conversation is valid work, just not a steer of this - // owner. It waits as its own turn instead of being failed or - // injected into the run currently on the wire (#743). - assert.equal(submitted.action, 'queued'); - assert.equal(settled.length, 0); - assert.equal(messageQueue.some(item => item.id === submitted.queuedId - && item.remoteKey === 'jaw:slack:channel:CFOREIGN'), true); - } else { - assert.equal(settled.length, 1); assert.equal(settled[0]!.data['outcome'], 'failed'); - assert.equal(settled[0]!.data['error'], 'native_replacement_owner_mismatch'); - } + assert.equal(settled.length, 1); assert.equal(settled[0]!.data['outcome'], 'failed'); + assert.equal(settled[0]!.data['error'], 'native_replacement_owner_mismatch'); assert.equal(hook.mock.callCount(), 0); assert.deepEqual(db.prepare('SELECT COUNT(*) AS n FROM messages').get(), before); - assert.equal(messageQueue.some(item => item.scope === opts.scopeKey), conflict === 'remoteKey'); + assert.equal(messageQueue.some(item => item.scope === opts.scopeKey), false); assert.equal(wire().filter(e => e['method'] === 'session/prompt').length, 1); assert.equal(wire().some(e => e['method'] === 'session/cancel'), false); assert.equal(events.some(e => e.type === 'steer_started' || e.type === 'steer_rejected' || e.type === 'new_message'), false); - if (conflict === 'remoteKey' && submitted.queuedId) removeQueuedMessage(submitted.queuedId); } finally { off(); killActiveAgent(opts.scopeKey, 'user'); await run.promise; } }); } diff --git a/tests/unit/steer-input-handoff.test.ts b/tests/unit/steer-input-handoff.test.ts index b9bbd19a0..8bba6d8f8 100644 --- a/tests/unit/steer-input-handoff.test.ts +++ b/tests/unit/steer-input-handoff.test.ts @@ -12,6 +12,7 @@ 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'; },