diff --git a/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts b/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts index 885cdcea20..65ec8f1c33 100644 --- a/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts +++ b/apps/desktop/src/main/__tests__/import-tasks-settings-page.test.ts @@ -25,6 +25,7 @@ import { createRoot, type Root } from 'react-dom/client'; import { AstryxLocaleProvider, LocaleProvider } from '@maka/ui'; import type { DesktopRuntimeHostRef } from '../../preload/bridge-contract.js'; import type { DesktopExternalSessionCatalogItem } from '../../preload/external-session-catalog.js'; +import type { ExternalSessionImportFailureReason } from '../../preload/external-session-import-result.js'; import { ImportTasksSettingsPage } from '../../renderer/settings/import-tasks-settings-page.js'; import { RuntimeHostSettingsTarget } from '../../renderer/settings/runtime-host-settings-target.js'; @@ -108,6 +109,52 @@ describe('ImportTasksSettingsPage durable import state', () => { await act(async () => harness.root.unmount()); }); + it('shows an actionable banner and does not re-read the catalog when no model is usable', async () => { + const harness = await renderPage({ + catalog: catalog(externalSession()), + importResult: { ok: false, reason: 'no_model' }, + }); + + const importButton = buttonWithText(harness.container, 'Import'); + assert.ok(importButton); + await act(async () => { + importButton.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + + assert.match(harness.container.textContent, /No usable model connection/); + // A clean model failure is not a maybe-landed task: no recovery re-read, and + // none of the unknown-outcome copy. + assert.doesNotMatch(harness.container.textContent, /Check the import result/); + assert.deepEqual(harness.hostCalls(), [ + { operation: 'listSources', host: TEST_RUNTIME_HOST }, + { operation: 'list', host: TEST_RUNTIME_HOST }, + { operation: 'import', host: TEST_RUNTIME_HOST }, + ]); + + await act(async () => harness.root.unmount()); + }); + + it('shows a source-unreadable banner when the conversation cannot be converted', async () => { + const harness = await renderPage({ + catalog: catalog(externalSession()), + importResult: { ok: false, reason: 'source_unreadable' }, + }); + + const importButton = buttonWithText(harness.container, 'Import'); + assert.ok(importButton); + await act(async () => { + importButton.click(); + await Promise.resolve(); + await Promise.resolve(); + }); + + assert.match(harness.container.textContent, /could not be read or converted/); + + await act(async () => harness.root.unmount()); + }); + it('uses catalog in-flight state after remount to disable the source row', async () => { const harness = await renderPage({ catalog: { @@ -1202,14 +1249,14 @@ async function renderPage(options: { adapterIds?: string[]; bySource?: Record>>; importResult?: - | { ok: false; reason: 'commit_outcome_unknown' } - | Promise<{ ok: false; reason: 'commit_outcome_unknown' }>; + | { ok: false; reason: ExternalSessionImportFailureReason } + | Promise<{ ok: false; reason: ExternalSessionImportFailureReason }>; /** * Per-source answers for a batch: `ok` lands, `unknown` is the Host not * answering, `throw` is a rejection. Keyed by source session id, because a * batch is exactly the case where the ids must not share one answer. */ - importBySource?: Record; + importBySource?: Record; onOpenImported?: (sessionId: string) => void; locale?: 'en' | 'zh-CN'; }): Promise<{ @@ -1288,6 +1335,9 @@ async function renderPage(options: { const perSource = options.importBySource?.[sourceSessionId]; if (perSource === 'throw') throw new Error(`import-failed:${sourceSessionId}`); if (perSource === 'unknown') return { ok: false, reason: 'commit_outcome_unknown' }; + if (perSource === 'no_model' || perSource === 'source_unreadable') { + return { ok: false, reason: perSource }; + } if (perSource === 'ok') { return { ok: true, session: { id: `imported-${sourceSessionId}` } }; } @@ -1498,6 +1548,44 @@ describe('ImportTasksSettingsPage batch import', () => { assert.match(text, /unconfirmed|Unconfirmed|outcome/i); }); + it('counts code-classified batch failures as failed, not unconfirmed, and raises the model banner', async () => { + // Before the fix, no_model / source_unreadable were swept into the + // maybe-landed "unconfirmed" bucket alongside commit_outcome_unknown: no + // actionable banner, the recovery/retry path offered, and the summary could + // read as success. They are definite failures — counted as failed, never + // offered recovery. no_model additionally raises its actionable banner. + const { container } = await renderPage({ + catalog: { + sessions: [ + externalSession({ id: 'blocked', name: 'Blocked' }), + externalSession({ id: 'unreadable', name: 'Unreadable' }), + ], + nextCursor: null, + }, + importBySource: { blocked: 'no_model', unreadable: 'source_unreadable' }, + }); + + await tick(masterBox(container), true); + const run = buttonWithText(container, 'Import selected'); + assert.ok(run); + await act(async () => { + run.click(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + await Promise.resolve(); + }); + + const text = container.textContent ?? ''; + // Both are definite failures: the summary counts them, none imported. + assert.match(text, /No conversation was imported/); + assert.match(text, /2 more could not be imported/); + // Not the maybe-landed path: no unconfirmed/recovery banner is offered. + assert.doesNotMatch(text, /Check the import result/); + // The one globally-actionable reason surfaces its banner. + assert.match(text, /No usable model connection/); + }); + it('spins only the conversion in flight, not every queued row', async () => { // A spinner claims something is happening now. Marking every selected row // would put one on rows the batch has not reached, and on rows it already diff --git a/apps/desktop/src/main/__tests__/runtime-host-external-sessions-ipc-main.test.ts b/apps/desktop/src/main/__tests__/runtime-host-external-sessions-ipc-main.test.ts index d4d8fa388c..66da64ef5a 100644 --- a/apps/desktop/src/main/__tests__/runtime-host-external-sessions-ipc-main.test.ts +++ b/apps/desktop/src/main/__tests__/runtime-host-external-sessions-ipc-main.test.ts @@ -134,6 +134,94 @@ test('an uncertain commit still asks the shell to re-read the catalog', async () assert.deepEqual(events, [{ reason: 'created', sessionId: undefined }]); }); +test('maps a no-usable-model failure to a distinct, non-recovering reason', async () => { + const events: unknown[] = []; + const ipc = ipcHarness(); + registerRuntimeHostExternalSessionsIpc( + { + client: clientFixture({ + importExternalSession: async () => { + throw new RuntimeHostOperationError( + 'external-session.import', + 'model_unavailable', + 'No usable Session model connection is available for import', + ); + }, + }), + emitSessionsChanged: (reason, sessionId) => events.push({ reason, sessionId }), + }, + ipc, + ); + + assert.deepEqual( + await ipc.invoke('external-sessions:import', { + adapterId: 'codex', + sourceSessionId: 'source-1', + }), + { ok: false, reason: 'no_model' }, + ); + // A model-resolution failure never touched the catalog, so nothing to re-read. + assert.deepEqual(events, []); +}); + +test('maps a pre-commit conversion failure to source_unreadable', async () => { + const ipc = ipcHarness(); + registerRuntimeHostExternalSessionsIpc( + { + client: clientFixture({ + importExternalSession: async () => { + throw new RuntimeHostOperationError( + 'external-session.import', + 'source_unreadable', + 'External Session could not be read or converted', + ); + }, + }), + emitSessionsChanged() {}, + }, + ipc, + ); + + assert.deepEqual( + await ipc.invoke('external-sessions:import', { + adapterId: 'codex', + sourceSessionId: 'source-1', + }), + { ok: false, reason: 'source_unreadable' }, + ); +}); + +test('rethrows import failures that have no distinct renderer reason', async () => { + const ipc = ipcHarness(); + registerRuntimeHostExternalSessionsIpc( + { + client: clientFixture({ + importExternalSession: async () => { + // An unsupported adapter is a bad request, not a model or source + // problem — it must NOT be relabeled as `source_unreadable`; it falls + // through to the generic banner. + throw new RuntimeHostOperationError( + 'external-session.import', + 'invalid_request', + 'External Session source is unsupported', + ); + }, + }), + emitSessionsChanged() {}, + }, + ipc, + ); + + await assert.rejects( + () => + ipc.invoke('external-sessions:import', { + adapterId: 'codex', + sourceSessionId: 'source-1', + }), + /External Session source is unsupported/, + ); +}); + test('rejects malformed renderer requests before they reach the Host client', async () => { let calls = 0; const ipc = ipcHarness(); diff --git a/apps/desktop/src/main/runtime-host-external-sessions-ipc-main.ts b/apps/desktop/src/main/runtime-host-external-sessions-ipc-main.ts index 91e871f800..fec47c480a 100644 --- a/apps/desktop/src/main/runtime-host-external-sessions-ipc-main.ts +++ b/apps/desktop/src/main/runtime-host-external-sessions-ipc-main.ts @@ -29,7 +29,10 @@ import { decodeExternalSessionCatalogQueryInput, decodeExternalSessionImportInput, } from '@maka/runtime-host/protocol'; -import type { ExternalSessionImportIpcResult } from '../preload/external-session-import-result.js'; +import type { + ExternalSessionImportFailureReason, + ExternalSessionImportIpcResult, +} from '../preload/external-session-import-result.js'; import type { DesktopHostExternalSessionCatalogItem } from '../preload/external-session-catalog.js'; import { handleReconnectableRead, @@ -85,22 +88,42 @@ export function registerRuntimeHostExternalSessionsIpc( } catch (error) { if ( error instanceof RuntimeHostOperationError && - error.operation === 'external-session.import' && - error.code === 'commit_outcome_unknown' + error.operation === 'external-session.import' ) { - // "Unknown" means the task may well be in the catalog, so tell the - // shell to read it again. Without this, the only trace of a maybe- - // committed import is the banner on the page, and the page is gone the - // moment the user leaves Settings -- which is exactly when they come - // back and import the same conversation a second time. No id: the - // whole point is that we do not know which task, if any, landed. - deps.emitSessionsChanged('created'); - return { - ok: false, - reason: 'commit_outcome_unknown', - } satisfies ExternalSessionImportIpcResult; + if (error.code === 'commit_outcome_unknown') { + // "Unknown" means the task may well be in the catalog, so tell the + // shell to read it again. Without this, the only trace of a maybe- + // committed import is the banner on the page, and the page is gone the + // moment the user leaves Settings -- which is exactly when they come + // back and import the same conversation a second time. No id: the + // whole point is that we do not know which task, if any, landed. + deps.emitSessionsChanged('created'); + return { + ok: false, + reason: 'commit_outcome_unknown', + } satisfies ExternalSessionImportIpcResult; + } + const reason = classifyImportFailure(error); + if (reason !== undefined) { + return { ok: false, reason } satisfies ExternalSessionImportIpcResult; + } } throw error; } }); } + +/** + * Turn the intact Host operation error into a typed reason the renderer can + * render distinctly. Done here, in Desktop Main, because Electron IPC drops the + * `code` before the renderer sees the error. The coordinator publishes dedicated + * stable codes for these cases, so this maps by code alone — no message text and + * no reuse of an overloaded code such as `invalid_request`. + */ +function classifyImportFailure( + error: RuntimeHostOperationError, +): Exclude | undefined { + if (error.code === 'model_unavailable') return 'no_model'; + if (error.code === 'source_unreadable') return 'source_unreadable'; + return undefined; +} diff --git a/apps/desktop/src/preload/external-session-import-result.ts b/apps/desktop/src/preload/external-session-import-result.ts index 5be40aae2b..20a7c9a495 100644 --- a/apps/desktop/src/preload/external-session-import-result.ts +++ b/apps/desktop/src/preload/external-session-import-result.ts @@ -19,7 +19,22 @@ import type { SessionSummary } from '@maka/core/session'; -/** Stable Desktop IPC result for the one import failure that must not be retried blindly. */ +/** + * Why an import did not produce a task. Each maps to a specific, actionable + * banner in the import page. Carried as a typed result rather than a thrown + * error because Electron IPC strips the custom `code` off a `RuntimeHostOperationError` + * on its way to the renderer — the reason must be decided in Desktop Main, where + * the code is still intact, and handed across as data. + */ +export type ExternalSessionImportFailureReason = + /** Import ran but its outcome is unknown; the catalog must be re-read (not retried blindly). */ + | 'commit_outcome_unknown' + /** No usable model connection to attach the imported task to — configure a model first. */ + | 'no_model' + /** The source conversation could not be read or converted (e.g. too large or malformed). */ + | 'source_unreadable'; + +/** Stable Desktop IPC result for the import failures the page renders distinctly. */ export type ExternalSessionImportIpcResult = | { readonly ok: true; readonly session: T } - | { readonly ok: false; readonly reason: 'commit_outcome_unknown' }; + | { readonly ok: false; readonly reason: ExternalSessionImportFailureReason }; diff --git a/apps/desktop/src/renderer/locales/external-session-import-copy.ts b/apps/desktop/src/renderer/locales/external-session-import-copy.ts index 48cb7a829c..cbbb03db71 100644 --- a/apps/desktop/src/renderer/locales/external-session-import-copy.ts +++ b/apps/desktop/src/renderer/locales/external-session-import-copy.ts @@ -68,6 +68,10 @@ type ExternalSessionImportCopy = { importInProgressDescription: (name: string) => string; importFailedTitle: string; importFailedFallback: string; + /** No usable model connection to attach the imported task to. */ + importFailedNoModel: string; + /** The source conversation could not be read or converted (e.g. too large). */ + importFailedSourceUnreadable: string; importRecoveredTitle: string; importRecoveredDescription: (name: string) => string; importNotRecordedTitle: string; @@ -133,6 +137,8 @@ const COPY = { importInProgressDescription: (name) => `正在导入「${name}」,完成后会直接打开这个任务。`, importFailedTitle: '导入失败', importFailedFallback: '该对话无法转换或保存。请检查来源后重试。', + importFailedNoModel: '没有可用的模型连接,无法为导入的任务选择模型。请先在 设置 · 模型 中配置并启用一个模型后再导入。', + importFailedSourceUnreadable: '无法读取或转换该对话,它可能过大、已损坏或暂时无法读取。请检查来源后重试。', importRecoveredTitle: '已确认导入', importRecoveredDescription: (name) => `「${name}」导入的任务现已可用。`, importNotRecordedTitle: '没有发现新任务', @@ -189,6 +195,8 @@ const COPY = { importInProgressDescription: (name) => `正在匯入「${name}」,完成後會直接開啟這個任務。`, importFailedTitle: '匯入失敗', importFailedFallback: '該對話無法轉換或儲存。請檢查來源後重試。', + importFailedNoModel: '沒有可用的模型連線,無法為匯入的任務選擇模型。請先在 設定 · 模型 中設定並啟用一個模型後再匯入。', + importFailedSourceUnreadable: '無法讀取或轉換該對話,它可能過大、已損毀或暫時無法讀取。請檢查來源後重試。', importRecoveredTitle: '已確認匯入', importRecoveredDescription: (name) => `「${name}」匯入的任務現已可用。`, importNotRecordedTitle: '沒有發現新任務', @@ -242,6 +250,10 @@ const COPY = { `Importing “${name}”. Maka opens the task as soon as it lands.`, importFailedTitle: 'Import failed', importFailedFallback: 'This conversation could not be converted or saved. Check the source and try again.', + importFailedNoModel: + 'No usable model connection to attach the imported task to. Configure and enable a model in Settings · Models, then import again.', + importFailedSourceUnreadable: + 'This conversation could not be read or converted — it may be too large, malformed, or temporarily unreadable. Check the source and try again.', importRecoveredTitle: 'Import confirmed', importRecoveredDescription: (name) => `The imported task is available now for “${name}”.`, diff --git a/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx b/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx index 39fc454fd3..ff6e26b8a5 100644 --- a/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx +++ b/apps/desktop/src/renderer/settings/import-tasks-settings-page.tsx @@ -198,6 +198,13 @@ type ImportBatchOutcome = { duplicated: number; failed: readonly string[]; unknown: readonly string[]; + /** + * At least one row failed with `no_model`. Surfaced on the summary (not the + * transient importError banner, which the post-run catalog refresh clears) so + * the batch can name the one globally-actionable fix — configure a model — + * once for the whole run. + */ + noModel: boolean; }; const EMPTY_IMPORT_BATCH_OUTCOME: ImportBatchOutcome = { @@ -205,6 +212,7 @@ const EMPTY_IMPORT_BATCH_OUTCOME: ImportBatchOutcome = { duplicated: 0, failed: [], unknown: [], + noModel: false, }; function recordImportBatchResult( @@ -694,7 +702,19 @@ export function ImportTasksSettingsPage(props: { // the user has left steering the shell somewhere they did not ask for. if (!mountedRef.current) return; if (!outcome.ok) { - await recoverUnknownImport(attempt); + // Only an unknown commit outcome is a maybe-landed task to reconcile; + // the other reasons are clean failures with an actionable banner. + // Exhaustive by design — a new reason is a compile error until handled. + if (outcome.reason === 'commit_outcome_unknown') { + await recoverUnknownImport(attempt); + } else if (outcome.reason === 'no_model') { + setImportError(copy.importFailedNoModel); + } else if (outcome.reason === 'source_unreadable') { + setImportError(copy.importFailedSourceUnreadable); + } else { + const _exhaustive: never = outcome.reason; + return _exhaustive; + } return; } props.onImported(outcome.session); @@ -773,21 +793,23 @@ export function ImportTasksSettingsPage(props: { try { const result = await requestImport(attempt.adapterId, attempt.sourceSessionId); if (!mountedRef.current) return; - outcome = recordImportBatchResult( - outcome, - session.id, + if (result.ok) { + outcome = recordImportBatchResult( + outcome, + session.id, + wasImported ? 'duplicated' : 'imported', + ); + } else if (result.reason === 'commit_outcome_unknown') { // Not `failed`: the call did not answer, and only a catalog read // settles whether the conversion landed. Calling it a failure is - // what invites the retry that makes a second copy. - result.ok ? (wasImported ? 'duplicated' : 'imported') : 'unknown', - ); - if (!result.ok) { - // Recorded, not recovered. A single import recovers inline, but - // recovery re-reads the whole catalog window per attempt, and doing - // that between conversions would interleave N full reads with the - // batch and race the writes it is making. The unconfirmed banner - // names every one of these and its 重试 resolves them a press at a - // time, removing each as it settles. + // what invites the retry that makes a second copy. Recorded, not + // recovered — a single import recovers inline, but recovery re-reads + // the whole catalog window per attempt, and doing that between + // conversions would interleave N full reads with the batch and race + // the writes it is making. The unconfirmed banner names every one of + // these and its 重试 resolves them a press at a time, removing each + // as it settles. + outcome = recordImportBatchResult(outcome, session.id, 'unknown'); setUncertainImports((current) => current.some( (entry) => @@ -797,6 +819,22 @@ export function ImportTasksSettingsPage(props: { ? current : [...current, attempt], ); + } else { + // A definite, code-classified failure (no usable model, or an + // unreadable/oversized source) — not a maybe-landed task. Count it as + // failed and never offer recovery: retrying `no_model` just fails + // again, and retrying `source_unreadable` cannot make an unreadable + // conversation readable. Exhaustive by design — a new reason is a + // compile error until handled. + outcome = recordImportBatchResult(outcome, session.id, 'failed'); + if (result.reason === 'no_model') { + // A missing model blocks every row identically; the summary raises + // its actionable banner once for the whole run. + outcome = { ...outcome, noModel: true }; + } else if (result.reason !== 'source_unreadable') { + const _exhaustive: never = result.reason; + return _exhaustive; + } } } catch { if (!mountedRef.current) return; @@ -814,6 +852,9 @@ export function ImportTasksSettingsPage(props: { } } finally { if (mountedRef.current) { + // The summary carries `noModel`, not the transient importError banner, + // because `loadCatalog` below clears importError on its post-run refresh + // and would wipe it before the user sees it. setImportRun({ kind: 'idle', summary: outcome }); // Cleared because it was answered. Leaving the rows marked after a run // invites a second press that would import each of them again. @@ -996,6 +1037,9 @@ export function ImportTasksSettingsPage(props: { importRun.summary.failed.length > 0 ? copy.batchFailed(importRun.summary.failed.length) : null, + // The one globally-actionable failure: name the fix that + // unblocks every row at once. + importRun.summary.noModel ? copy.importFailedNoModel : null, ] .filter(Boolean) .join(' ') || undefined diff --git a/packages/runtime-host/src/__tests__/external-session-coordinator.test.ts b/packages/runtime-host/src/__tests__/external-session-coordinator.test.ts index 6a7826628e..8a90651563 100644 --- a/packages/runtime-host/src/__tests__/external-session-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/external-session-coordinator.test.ts @@ -32,6 +32,10 @@ import { } from '../protocol/index.js'; import type { ConnectionContext } from '../server/operation-dispatcher.js'; import { HostExternalSessionCoordinator } from '../server/external-session-coordinator.js'; +import { + NoUsableImportModelError, + SessionOperationFailure, +} from '../server/session-catalog-coordinator.js'; import { SessionAdmissionGate } from '../server/session-admission-gate.js'; const context: ConnectionContext = { @@ -345,7 +349,10 @@ test('reports conversion errors before persistence and store uncertainty after e ), { ok: false, - error: { code: 'invalid_request', message: 'External Session could not be converted' }, + error: { + code: 'source_unreadable', + message: 'External Session could not be read or converted', + }, }, ); assert.equal(createAttempts, 0); @@ -373,6 +380,55 @@ test('reports conversion errors before persistence and store uncertainty after e assert.equal(persistenceFailure.drainRequests(), 1); }); +test('reports a model-target failure before any commit is attempted', async () => { + let createAttempts = 0; + const fixture = coordinatorFixture([adapterFixture()], { + resolveTarget: async () => { + throw new NoUsableImportModelError( + 'No usable Session model connection is available for import', + ); + }, + createImportedSession: async () => { + createAttempts += 1; + assert.fail('A model-target failure must not enter persistence'); + }, + }); + + assert.deepEqual( + await fixture.coordinator.handlers['external-session.import']( + { adapterId: 'codex', sourceSessionId: 'source-0' }, + context, + ), + { + ok: false, + error: { + code: 'model_unavailable', + message: 'No usable Session model connection is available for import', + }, + }, + ); + assert.equal(createAttempts, 0); + assert.equal(fixture.drainRequests(), 0); +}); + +test('reports an unsupported adapter as invalid_request, not a source-unreadable failure', async () => { + // Guards the shell mapping: only the dedicated `source_unreadable` code becomes + // the "too large or malformed" banner. An unknown adapter is a bad request and + // must stay generic rather than blame the source conversation. + const fixture = coordinatorFixture([adapterFixture()]); + + assert.deepEqual( + await fixture.coordinator.handlers['external-session.import']( + { adapterId: 'unknown-adapter', sourceSessionId: 'source-0' }, + context, + ), + { + ok: false, + error: { code: 'invalid_request', message: 'External Session source is unsupported' }, + }, + ); +}); + test('removes an imported Session when its model history cannot be prepared', async () => { const fixture = coordinatorFixture([adapterFixture()], { prepareImportedSessionHistory: async () => { @@ -468,6 +524,14 @@ function coordinatorFixture( Pick & { prepareImportedSessionHistory(sessionId: string): Promise; discardImportedSession(sessionId: string): Promise; + resolveTarget(): Promise<{ + readonly backend: 'ai-sdk'; + readonly llmConnectionSlug: string; + readonly model: string; + readonly permissionMode: 'ask'; + readonly collaborationMode: 'agent'; + readonly orchestrationMode: 'default'; + }>; } > = {}, ) { @@ -546,14 +610,16 @@ function coordinatorFixture( }, }, }, - resolveTarget: async () => ({ - backend: 'ai-sdk', - llmConnectionSlug: 'default', - model: 'gpt-5', - permissionMode: 'ask', - collaborationMode: 'agent', - orchestrationMode: 'default', - }), + resolveTarget: + storeOverrides.resolveTarget ?? + (async () => ({ + backend: 'ai-sdk', + llmConnectionSlug: 'default', + model: 'gpt-5', + permissionMode: 'ask', + collaborationMode: 'agent', + orchestrationMode: 'default', + })), prepareImportedSessionHistory: storeOverrides.prepareImportedSessionHistory ?? (async (sessionId) => { diff --git a/packages/runtime-host/src/__tests__/protocol.test.ts b/packages/runtime-host/src/__tests__/protocol.test.ts index 9ac1cfde4f..185ffc648a 100644 --- a/packages/runtime-host/src/__tests__/protocol.test.ts +++ b/packages/runtime-host/src/__tests__/protocol.test.ts @@ -437,6 +437,12 @@ describe('Runtime Host bootstrap protocol', () => { assert.ok(RUNTIME_HOST_COMPATIBILITY_EPOCH > 99); }); + test('publishes a new compatibility epoch for external Session import failure reasons', () => { + // model_unavailable / source_unreadable let the shell classify import + // failures by stable code; older peers cannot decode the new codes. + assert.ok(RUNTIME_HOST_COMPATIBILITY_EPOCH > 112); + }); + test('selects the highest mutually supported protocol and rejects a gap', () => { assert.equal(negotiateProtocol({ min: 0, max: 0 }, { min: 0, max: 0 }), 0); assert.equal(negotiateProtocol({ min: 1, max: 3 }, { min: 2, max: 4 }), 3); diff --git a/packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts b/packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts index 614998f366..39b6a8d5b0 100644 --- a/packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts +++ b/packages/runtime-host/src/__tests__/session-catalog-coordinator.test.ts @@ -54,6 +54,8 @@ import { HostProjectMembershipGate } from '../server/project-membership-gate.js' import { HostWorkspaceResolver } from '../server/workspace-resolver.js'; import { HostSessionCatalogCoordinator, + NoUsableImportModelError, + SessionOperationFailure, type HostSessionCatalogCoordinatorOptions, } from '../server/session-catalog-coordinator.js'; import { SessionAdmissionGate } from '../server/session-admission-gate.js'; @@ -1419,6 +1421,147 @@ test('rejects a legacy cursor that carries a Session catalog filter', async () = assert.equal(outcome.error.code, 'invalid_request'); }); +test('external import target falls back to a ready connection when no default is set', async () => { + // The reported bug: a self-configured profile has `defaultTarget: null` while + // holding usable connections, and every import failed before reading the source. + const fixture = createFixture({ + runtimePolicy: importTargetPolicy({ + defaultTarget: null, + connections: [{ connectionId: 'conn-a', slug: 'anthropic', enabledModelIds: ['model-1'] }], + }), + }); + + const target = await fixture.coordinator.resolveExternalSessionImportTarget(); + + assert.equal(target.llmConnectionId, 'conn-a'); + assert.equal(target.llmConnectionSlug, 'anthropic'); + assert.equal(target.model, 'model-1'); + assert.equal(target.collaborationMode, 'agent'); +}); + +test('external import target uses a ready configured default even when it is not first in catalog order', async () => { + // Pins "behavior is unchanged when a default is set and ready": without the + // default-first preference the enumerator would pick conn-a (first in catalog + // order); the configured default is conn-b and must win. + const fixture = createFixture({ + runtimePolicy: importTargetPolicy({ + defaultTarget: { connectionId: 'conn-b', modelId: 'model-2' }, + connections: [ + { connectionId: 'conn-a', slug: 'anthropic', enabledModelIds: ['model-1'] }, + { connectionId: 'conn-b', slug: 'openai', enabledModelIds: ['model-2'] }, + ], + }), + }); + + const target = await fixture.coordinator.resolveExternalSessionImportTarget(); + + assert.equal(target.llmConnectionId, 'conn-b'); + assert.equal(target.model, 'model-2'); +}); + +test('external import target skips an unready default and uses the next ready connection', async () => { + const fixture = createFixture({ + runtimePolicy: importTargetPolicy({ + defaultTarget: { connectionId: 'conn-a', modelId: 'model-1' }, + connections: [ + { connectionId: 'conn-a', slug: 'retired', verdict: { kind: 'provider_retired' } }, + { connectionId: 'conn-b', slug: 'openai', enabledModelIds: ['model-2'] }, + ], + }), + }); + + const target = await fixture.coordinator.resolveExternalSessionImportTarget(); + + assert.equal(target.llmConnectionId, 'conn-b'); + assert.equal(target.model, 'model-2'); +}); + +test('external import target fails cleanly when no connection is usable', async () => { + const fixture = createFixture({ + runtimePolicy: importTargetPolicy({ + defaultTarget: null, + connections: [ + { + connectionId: 'conn-a', + slug: 'openai', + verdict: { kind: 'credential_not_configured', status: { configured: false } as never }, + }, + { connectionId: 'conn-b', slug: 'deepseek', enabled: false }, + ], + }), + }); + + await assert.rejects( + fixture.coordinator.resolveExternalSessionImportTarget(), + (error: unknown) => + error instanceof NoUsableImportModelError && + error.code === 'operation_unavailable' && + /No usable Session model/i.test(error.message), + ); +}); + +test('external import target surfaces a mid-selection identity race instead of masking it', async () => { + // A connection deleted or renamed between the snapshot and resolution makes + // `#resolveModel` throw `operation_conflict`. That is a real race, not an + // unusable candidate: import must surface it, not swallow it and silently pick + // the next (lower-priority) connection. conn-a is the first candidate and is + // mid-race; conn-b is ready — the pre-fix fallback returned conn-b, hiding the + // conflict. + const fixture = createFixture({ + runtimePolicy: importTargetPolicy({ + defaultTarget: null, + connections: [ + { connectionId: 'conn-a', slug: 'anthropic', verdict: { kind: 'not_found' } }, + { connectionId: 'conn-b', slug: 'openai', enabledModelIds: ['model-2'] }, + ], + }), + }); + + await assert.rejects( + fixture.coordinator.resolveExternalSessionImportTarget(), + (error: unknown) => + error instanceof SessionOperationFailure && + !(error instanceof NoUsableImportModelError) && + error.code === 'operation_conflict', + ); +}); + +test('autonomous create target uses the configured default when one is set', async () => { + const fixture = createFixture({ + runtimePolicy: importTargetPolicy({ + defaultTarget: { connectionId: 'conn-a', modelId: 'model-1' }, + connections: [{ connectionId: 'conn-a', slug: 'anthropic', enabledModelIds: ['model-1'] }], + }), + }); + + const target = await fixture.coordinator.resolveDefaultCreateTarget(); + + assert.equal(target.llmConnectionId, 'conn-a'); + assert.equal(target.model, 'model-1'); +}); + +test('autonomous create target fails closed when no default is set, even with a ready connection', async () => { + // The WorkHub coordination / scheduled / root paths must not silently bind a + // connection the user never chose: with no user in the loop, the absence of a + // default fails closed rather than starting on an unintended account. This is + // the counterpart to import's fallback and guards against re-merging the two + // resolutions. + const fixture = createFixture({ + runtimePolicy: importTargetPolicy({ + defaultTarget: null, + connections: [{ connectionId: 'conn-a', slug: 'anthropic', enabledModelIds: ['model-1'] }], + }), + }); + + await assert.rejects( + fixture.coordinator.resolveDefaultCreateTarget(), + (error: unknown) => + error instanceof SessionOperationFailure && + error.code === 'operation_unavailable' && + /No default Session model is configured/i.test(error.message), + ); +}); + function createFixture( options: { readonly labels?: readonly string[]; @@ -1427,6 +1570,7 @@ function createFixture( readonly manager?: Partial; readonly continuity?: Partial; readonly connection?: FixtureConnection; + readonly runtimePolicy?: RuntimePolicy; readonly projectCatalog?: ProjectCatalog; readonly onProjectChanged?: () => void; readonly legacyConnectionIdentity?: boolean; @@ -1476,7 +1620,7 @@ function createFixture( }, ...options.stores, }; - const runtimePolicy = runtimePolicyFixture(options.connection ?? {}); + const runtimePolicy = options.runtimePolicy ?? runtimePolicyFixture(options.connection ?? {}); const manager: ConfigurationAuthority = { runningTurnIds: () => [], transitionSessionConfiguration: async (_sessionId, input) => { @@ -1597,6 +1741,67 @@ function runtimePolicyFixture(overrides: FixtureConnection): RuntimePolicy { }; } +/** + * A runtime policy with several connections and per-connection resolver verdicts, + * for the external-import target tests. `verdict` defaults to `ready`; a connection + * with `enabled: false` is filtered out before resolution, exactly as the catalog + * candidate enumeration does. + */ +function importTargetPolicy(input: { + readonly defaultTarget: { readonly connectionId: string; readonly modelId: string } | null; + readonly connections: ReadonlyArray<{ + readonly connectionId: string; + readonly slug: string; + readonly enabled?: boolean; + readonly enabledModelIds?: readonly string[]; + readonly verdict?: 'ready' | ResolveExecutionConnectionResult; + }>; +}): RuntimePolicy { + const policy = createDefaultRuntimePolicy(); + const entries = input.connections.map((connection) => ({ + connectionId: connection.connectionId, + revision: 1, + slug: connection.slug, + name: connection.slug, + providerType: 'openai' as const, + enabled: connection.enabled ?? true, + enabledModelIds: connection.enabledModelIds ?? ['model-1'], + models: (connection.enabledModelIds ?? ['model-1']).map((id) => ({ id })), + modelSource: 'fetched' as const, + })); + const entryById = new Map(entries.map((entry) => [entry.connectionId, entry] as const)); + const specById = new Map(input.connections.map((spec) => [spec.connectionId, spec] as const)); + return { + connectionCatalog: { + getSnapshot: async () => ({ + revision: 1, + defaultTarget: input.defaultTarget, + connections: entries, + }), + }, + runtimePolicy: { + getSnapshot: async () => ({ revision: 1, policy }), + }, + operations: { + resolveExecutionConnection: async (ref) => { + const connectionId = 'connectionId' in ref ? ref.connectionId : undefined; + const spec = connectionId === undefined ? undefined : specById.get(connectionId); + const entry = connectionId === undefined ? undefined : entryById.get(connectionId); + if (!spec || !entry) return { kind: 'not_found' }; + if (spec.verdict === undefined || spec.verdict === 'ready') { + return { + kind: 'ready', + connection: entry, + secretMaterial: {}, + networkProxy: policy.networkProxy, + }; + } + return spec.verdict; + }, + }, + }; +} + function configurationInput( sessionId: string, expectedRevision: number, diff --git a/packages/runtime-host/src/protocol/external-session.ts b/packages/runtime-host/src/protocol/external-session.ts index 7087db0a36..56182f5d41 100644 --- a/packages/runtime-host/src/protocol/external-session.ts +++ b/packages/runtime-host/src/protocol/external-session.ts @@ -57,6 +57,10 @@ const IMPORT_ERRORS = [ 'not_found', 'operation_conflict', 'commit_outcome_unknown', + // Distinct, stable reasons the import page renders as specific banners, so the + // shell classifies by code rather than by the redacted error message. + 'model_unavailable', + 'source_unreadable', ] as const; export type ExternalSessionSourceQueryInput = Record; diff --git a/packages/runtime-host/src/protocol/index.ts b/packages/runtime-host/src/protocol/index.ts index b58467c6ee..d5fb8a4457 100644 --- a/packages/runtime-host/src/protocol/index.ts +++ b/packages/runtime-host/src/protocol/index.ts @@ -101,7 +101,10 @@ export const RUNTIME_HOST_REGISTRATION_SCHEMA_VERSION = 1 as const; export const RUNTIME_HOST_PROTOCOL_VERSION = 0 as const; // Increment when the same protocol version no longer guarantees safe Client-Host // interoperability. Mismatches are rejected before domain commands are admitted. -export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 112 as const; +export const RUNTIME_HOST_COMPATIBILITY_EPOCH = 113 as const; +// 113: External-session import publishes distinct `model_unavailable` and +// `source_unreadable` error codes so the shell classifies failures by code +// instead of the redacted message. Older peers cannot decode the new codes. // 112: Owners can query the Host execution environment through an extensible, // bounded resource-envelope contract. Older Hosts do not implement the query. // 111: Client Capability tool schemas may use draft-07 tuple additionalItems. diff --git a/packages/runtime-host/src/protocol/operation-spec.ts b/packages/runtime-host/src/protocol/operation-spec.ts index 7b85f061a4..cbdd9a6768 100644 --- a/packages/runtime-host/src/protocol/operation-spec.ts +++ b/packages/runtime-host/src/protocol/operation-spec.ts @@ -31,6 +31,10 @@ export type HostOperationErrorCode = | 'operation_conflict' | 'capability_unavailable' | 'invalid_request' + // External-session import: no usable model connection to attach the task to. + | 'model_unavailable' + // External-session import: the source could not be read or converted. + | 'source_unreadable' | 'projection_incomplete' | 'stale_cursor' | 'persistence_failed' diff --git a/packages/runtime-host/src/server/execution-composition.ts b/packages/runtime-host/src/server/execution-composition.ts index bb9145e053..19c4186205 100644 --- a/packages/runtime-host/src/server/execution-composition.ts +++ b/packages/runtime-host/src/server/execution-composition.ts @@ -1550,7 +1550,7 @@ export async function createExecutionRuntimeHostComposition( }, resolveCreateTarget: async () => { const { projectId: _projectId, ...target } = - await sessionCatalog.resolveExternalSessionImportTarget(); + await sessionCatalog.resolveDefaultCreateTarget(); return { ...target, permissionMode: 'explore' }; }, requestDrain: context.requestDrain, diff --git a/packages/runtime-host/src/server/external-session-coordinator.ts b/packages/runtime-host/src/server/external-session-coordinator.ts index 18cad24d7f..53852ef9f2 100644 --- a/packages/runtime-host/src/server/external-session-coordinator.ts +++ b/packages/runtime-host/src/server/external-session-coordinator.ts @@ -45,6 +45,7 @@ import type { ExternalSessionOperationHandlerMap } from './operation-dispatcher. import { projectSessionCatalogRecord, SessionOperationFailure, + NoUsableImportModelError, } from './session-catalog-coordinator.js'; import type { SessionAdmissionGate } from './session-admission-gate.js'; import { type HostWorkspaceResolver, WorkspaceResolutionError } from './workspace-resolver.js'; @@ -246,6 +247,9 @@ export class HostExternalSessionCoordinator { try { target = await this.#resolveTarget(); } catch (error) { + if (error instanceof NoUsableImportModelError) { + return importFailure('model_unavailable', error.message); + } if (error instanceof SessionOperationFailure) { return importFailure(error.code, error.message); } @@ -269,10 +273,10 @@ export class HostExternalSessionCoordinator { } catch (error) { if (!commitAttempted) { return importFailure( - isSourceSessionNotFound(error) ? 'not_found' : 'invalid_request', + isSourceSessionNotFound(error) ? 'not_found' : 'source_unreadable', isSourceSessionNotFound(error) ? 'External Session does not exist' - : 'External Session could not be converted', + : 'External Session could not be read or converted', ); } this.#requestDrain(); diff --git a/packages/runtime-host/src/server/session-catalog-coordinator.ts b/packages/runtime-host/src/server/session-catalog-coordinator.ts index 3180a34226..67b4762fb0 100644 --- a/packages/runtime-host/src/server/session-catalog-coordinator.ts +++ b/packages/runtime-host/src/server/session-catalog-coordinator.ts @@ -19,7 +19,7 @@ import { RuntimeHostProtocolError } from '../protocol/errors.js'; import { createHash } from 'node:crypto'; -import { authorizeConnectionModel } from '@maka/core/llm-connections'; +import { authorizeConnectionModel, connectionEnabledModelIds } from '@maka/core/llm-connections'; import { isModelExplicitlyUnsupportedForChat } from '@maka/core/model-catalog'; import { thinkingVariantsForConnection } from '@maka/core/model-thinking'; import { @@ -28,6 +28,7 @@ import { type ExecutionBoundarySummary, } from '@maka/core/sandbox-boundary'; import type { CreateSessionInput } from '@maka/core/runtime-inputs'; +import type { ConnectionCatalogEntry, ConnectionCatalogSnapshot } from '@maka/core/runtime-policy'; import { DEFAULT_SESSION_NAME, normalizeUserSessionName } from '@maka/core/session-name'; import { isSessionStartModeLabel as isExecutionSemanticLabel, @@ -149,6 +150,20 @@ export class SessionOperationFailure extends Error { } } +/** + * The import path found no ready connection+model to attach the task to. A + * distinct type (not just a message) so `#importSession` can map it to the + * stable `model_unavailable` wire code without inspecting the message — the + * generic `operation_unavailable` code it carries is also used for an + * unavailable source, which is a different failure. + */ +export class NoUsableImportModelError extends SessionOperationFailure { + constructor(message: string) { + super('operation_unavailable', message); + this.name = 'NoUsableImportModelError'; + } +} + export interface HostSessionCatalogCoordinatorOptions { readonly stores: SessionCatalogStores; readonly runtimePolicy: SessionRuntimePolicyStores; @@ -169,6 +184,68 @@ interface ResolvedSessionModel { readonly model: string; } +/** A connection+model the import path may attempt, in preference order. */ +interface ImportModelCandidate { + readonly connectionId: string; + readonly connectionSlug: string; + readonly modelId: string; +} + +/** + * Connection+model candidates for an imported task, most-preferred first: the + * configured default (kept at its exact precedence), then one ready model per + * enabled connection in catalog order. Model-level readiness that is a pure + * catalog fact — enabled, not quarantined, chat-capable — is applied here so an + * unusable connection costs one `#resolveModel` attempt, not one per enabled + * model (a connection may enable hundreds). Connection-level readiness + * (credential, retired provider, identity) stays in `#resolveModel`, which + * remains the sole arbiter of those. + */ +function importModelCandidates(snapshot: ConnectionCatalogSnapshot): ImportModelCandidate[] { + const byId = new Map( + snapshot.connections.map((connection) => [connection.connectionId, connection]), + ); + const candidates: ImportModelCandidate[] = []; + const seen = new Set(); + const push = (connection: ConnectionCatalogEntry, modelId: string): void => { + const key = `${connection.connectionId} ${modelId}`; + if (seen.has(key)) return; + seen.add(key); + candidates.push({ + connectionId: connection.connectionId, + connectionSlug: connection.slug, + modelId, + }); + }; + // The connection's first enabled model that is a valid chat target, by pure + // catalog facts alone (no credential or provider-liveness read). Emitting only + // this one — rather than every enabled model — bounds the enumeration to one + // candidate per connection, so a connection-level failure (missing credential, + // retired provider) costs a single `#resolveModel` round trip. + const firstReadyModel = (connection: ConnectionCatalogEntry): string | undefined => { + for (const modelId of connectionEnabledModelIds(connection)) { + const model = authorizeConnectionModel(connection, modelId); + if (model && !isModelExplicitlyUnsupportedForChat(model)) return modelId; + } + return undefined; + }; + // Default first, so a configured-and-ready default keeps today's behavior. + // `retainedDefaultTarget` + `isValidTarget` guarantee a persisted default is + // enabled and present in `enabledModelIds`, so its exact model is taken at its + // precedence rather than re-picked from the connection. + const preferred = snapshot.defaultTarget; + if (preferred) { + const connection = byId.get(preferred.connectionId); + if (connection?.enabled) push(connection, preferred.modelId); + } + for (const connection of snapshot.connections) { + if (!connection.enabled) continue; + const modelId = firstReadyModel(connection); + if (modelId !== undefined) push(connection, modelId); + } + return candidates; +} + /** Host-owned Session catalog, creation, and configuration authority. */ export class HostSessionCatalogCoordinator { readonly handlers: SessionCatalogOperationHandlerMap = { @@ -206,11 +283,32 @@ export class HostSessionCatalogCoordinator { this.#sessionAccessAuthority = options.sessionAccessAuthority; } + /** + * Target for a task imported from another agent's conversation. Import is an + * explicit, one-off user action on a specific conversation, so it prefers the + * configured default but falls back to any ready connection+model — see + * `#resolveImportModel`. + */ async resolveExternalSessionImportTarget(): Promise> { - const [model, policy] = await Promise.all([ - this.#resolveModel({ kind: 'default' }, undefined), - this.#readRuntimePolicy(), - ]); + return this.#composeCreateTarget(this.#resolveImportModel()); + } + + /** + * Target for an autonomous create path (WorkHub coordination, scheduled/root). + * Unlike import there is no user in the loop to pick a model, so this fails + * closed when no default is configured rather than binding a connection the + * user never chose. Import's fallback deliberately does not reach here; keeping + * the two resolvers apart is what confines the guess to an explicit user + * action. + */ + async resolveDefaultCreateTarget(): Promise> { + return this.#composeCreateTarget(this.#resolveModel({ kind: 'default' }, undefined)); + } + + async #composeCreateTarget( + modelResolution: Promise, + ): Promise> { + const [model, policy] = await Promise.all([modelResolution, this.#readRuntimePolicy()]); return { llmConnectionId: model.connectionId, llmConnectionSlug: model.connectionSlug, @@ -804,6 +902,51 @@ export class HostSessionCatalogCoordinator { } } + /** + * Model for an imported task. Prefers the configured default but falls back to + * any ready connection+model, because a default is only auto-set during + * onboarding bootstrap (`setDefaultIfMissing`) and a self-configured profile + * legitimately has `defaultTarget: null` while holding perfectly usable + * connections. Without the fallback, every import fails before the source is + * even read. Unlike interactive session creation, import has no model picker, + * so this is the only place that can choose one. + */ + async #resolveImportModel(): Promise { + let snapshot: ConnectionCatalogSnapshot; + try { + snapshot = await this.#runtimePolicy.connectionCatalog.getSnapshot(); + } catch { + throw new SessionOperationFailure('persistence_failed', 'Connection catalog is unavailable'); + } + for (const candidate of importModelCandidates(snapshot)) { + try { + return await this.#resolveModel( + { + kind: 'explicit', + connectionId: candidate.connectionId, + connectionSlug: candidate.connectionSlug, + model: candidate.modelId, + }, + undefined, + ); + } catch (error) { + if (!(error instanceof SessionOperationFailure)) throw error; + // Only a genuinely unusable candidate is skippable: `invalid_request` + // (disabled / retired / not-enabled / non-chat) and `operation_unavailable` + // (no credential). Any other code — notably `operation_conflict`, thrown + // when the connection was deleted or renamed after the snapshot — is a + // real fault the caller must see, not a reason to silently fall through + // to `model_unavailable` or a lower-priority connection. + if (error.code !== 'invalid_request' && error.code !== 'operation_unavailable') { + throw error; + } + } + } + throw new NoUsableImportModelError( + 'No usable Session model connection is available for import', + ); + } + async #resolveModel( target: SessionModelTarget, thinkingLevel: SessionCreateInput['thinkingLevel'],