diff --git a/.project-docs/30-worklog/tasks/20260823-pi-core-host-api-a17f6c2e.md b/.project-docs/30-worklog/tasks/20260823-pi-core-host-api-a17f6c2e.md index bcc23c3..c295b05 100644 --- a/.project-docs/30-worklog/tasks/20260823-pi-core-host-api-a17f6c2e.md +++ b/.project-docs/30-worklog/tasks/20260823-pi-core-host-api-a17f6c2e.md @@ -96,6 +96,17 @@ cleanup, and stable fixed Host error projection including storage failures. - PI-105 remains closed; its vendor-neutral project seam and live target `get_commands` seam were accepted structurally. +- Planner re-review of cumulative HEAD `52b2467` again concluded `NEEDS FIX` + and kept the Ready Frontier at `{PI-100}`. The three remaining P1 gaps were + resolved/active Conversation model switching bypassing `runtime.setModel()`, + credential-refresh rejection escaping the auth-required contract, and + unmapped thinking/session-binding/runtime-model persistence failures. +- The incremental correction keeps unresolved first-model selection on the + validate/persist/initial-prepare path, restores target-only runtime model + switching for resolved Conversations without disposal, types every + production credential-refresh failure as authentication failure, and maps + Pi session registry binding/model writes plus service thinking writes to the + stable storage failure contract. ## Outcome @@ -118,30 +129,37 @@ - Provider changes advance the Pi provider revision, project config/knowledge changes advance the resource revision and invalidate cached Agent resources, and deactivating a project disposes its Conversation workers/interactions. +- Resolved model changes reuse the target runtime's `setModel` operation and do + not dispose an active worker; unresolved Conversations still persist the + validated first model before lazy preparation. Provider refresh rejection is + projected as `CODING_PROVIDER_AUTH_REQUIRED`, while authoritative + Conversation/session write failures are projected as + `CODING_STORAGE_WRITE_FAILED`. - No Renderer route was migrated here; PI-110 remains the next consumer of the new Main contract. Old `/api/opencode` removal remains PI-140 scope. ## Verification - `pnpm install --frozen-lockfile` — Passed using the repository-pinned pnpm. -- `pnpm run typecheck` — Passed after final implementation changes. +- `pnpm run typecheck` — Passed after the latest review corrections. - `pnpm run lint:check` — Passed with zero errors; six existing Renderer warnings remain outside PI-100-owned files. -- Focused PI/coding/provider suites — Passed: 7 files / 53 tests. -- In-memory Conversation contract plus PI-100 Host/SSE suite — Passed: 2 files / - 23 tests. Coverage includes local-only create, one Main composition, concurrent - dedupe, uncertain no-resend, global and target snapshot-first streams, real - HTTP SSE ordering, interaction route ID correlation, command degradation, and - runtime error redaction. -- `pnpm test` — Passed: 208 files / 2247 tests passed, 2 skipped. +- Latest review-focused suites — Passed: 4 files / 26 tests. Coverage adds + resolved active-run target-only model switching without disposal, + credential-refresh rejection and missing refreshed Works credentials, + first session-binding write failure, runtime model metadata write failure, + and service thinking metadata write failure. +- `pnpm test` — Passed after the latest corrections: 209 files / 2263 tests + passed, 2 skipped (2265 total). - `pnpm run build:vite` — Passed for Renderer, Electron Main, Preload, and utility worker. Existing dynamic-import/chunk-size warnings remain. -- `pnpm run test:e2e` — Production build passed and 27/28 Windows Electron tests - passed. The pre-existing OpenCode slash-command assertion failed because its - legacy `/command` request omitted `model`; isolated rerun reproduced it. This - task changes no Renderer file, OpenCode command route, or slash-command spec, - and the new composition performs no runtime/provider/project read before a - coding Snapshot/execution request, so no causal PI-100 path was found. +- `pnpm run test:e2e` — Earlier cumulative PI-100 verification built the + production app and passed 27/28 Windows Electron tests. The pre-existing + OpenCode slash-command assertion failed because its legacy `/command` request + omitted `model`; isolated rerun reproduced it. The latest correction changes + only target model/auth/storage failure paths plus unit tests, so this legacy + E2E was not rerun. No Renderer, OpenCode command route, or slash-command spec + is changed by PI-100. - Real external Provider turns remain **Explicitly Waived / Accepted Risk**; `realTurnVerified=false`. This is not Pass evidence. - macOS x64/arm64 validation remains skipped by user direction and mandatory at diff --git a/electron/api/coding-provider-auth.ts b/electron/api/coding-provider-auth.ts index 455e578..0ab08b6 100644 --- a/electron/api/coding-provider-auth.ts +++ b/electron/api/coding-provider-auth.ts @@ -8,33 +8,53 @@ import { const WORKS_SQUARE_AI_GATEWAY_CREDENTIAL_MODE = 'works_square_ai_gateway'; const AUTHENTICATION_ERROR_PATTERN = /\b(?:401|403|unauthori[sz]ed|forbidden|authentication failed|auth failed|invalid (?:api key|credential|access token|bearer token)|(?:access |bearer )?token expired)\b/i; +export class CodingProviderCredentialRefreshError extends Error { + constructor(message: string, options?: ErrorOptions) { + super(message, options); + this.name = 'CodingProviderCredentialRefreshError'; + } +} + export function isCodingProviderAuthenticationError(error: unknown): boolean { - return error instanceof Error && AUTHENTICATION_ERROR_PATTERN.test(error.message); + return error instanceof CodingProviderCredentialRefreshError + || (error instanceof Error && AUTHENTICATION_ERROR_PATTERN.test(error.message)); } export async function refreshCodingProviderCredential(accountId: string): Promise { - const providerService = getProviderService(); - const account = await providerService.getAccount(accountId); - if (!account?.enabled) throw new Error('Provider account is unavailable'); + try { + const providerService = getProviderService(); + const account = await providerService.getAccount(accountId); + if (!account?.enabled) { + throw new CodingProviderCredentialRefreshError('Provider account is unavailable'); + } - if (account.metadata?.worksSquareCredentialMode === WORKS_SQUARE_AI_GATEWAY_CREDENTIAL_MODE) { - markWorksSquareAIGatewayCredentialExpired(); - const credential = await getFreshWorksSquareAIGatewayCredential(); - if (!credential) throw new Error('Provider credential refresh failed'); - await providerService.updateAccount(account.id, { - baseUrl: credential.oneApiBaseUrl, - metadata: { - ...account.metadata, - worksSquareCredentialExpiresAt: credential.expiresAt === null - ? undefined - : new Date(credential.expiresAt).toISOString(), - }, - }, credential.accessToken); - return; - } + if (account.metadata?.worksSquareCredentialMode === WORKS_SQUARE_AI_GATEWAY_CREDENTIAL_MODE) { + markWorksSquareAIGatewayCredentialExpired(); + const credential = await getFreshWorksSquareAIGatewayCredential(); + if (!credential) { + throw new CodingProviderCredentialRefreshError('Provider credential refresh failed'); + } + await providerService.updateAccount(account.id, { + baseUrl: credential.oneApiBaseUrl, + metadata: { + ...account.metadata, + worksSquareCredentialExpiresAt: credential.expiresAt === null + ? undefined + : new Date(credential.expiresAt).toISOString(), + }, + }, credential.accessToken); + return; + } - const current = await resolvePiProviderCredentialFromSecretStore(account); - if (!current && account.authMode !== 'local') { - throw new Error('Provider credential is unavailable'); + const current = await resolvePiProviderCredentialFromSecretStore(account); + if (!current && account.authMode !== 'local') { + throw new CodingProviderCredentialRefreshError('Provider credential is unavailable'); + } + } catch (error) { + if (error instanceof CodingProviderCredentialRefreshError) throw error; + throw new CodingProviderCredentialRefreshError( + 'Provider credential refresh failed', + { cause: error }, + ); } } diff --git a/electron/coding-runtime/conversation-service.ts b/electron/coding-runtime/conversation-service.ts index c8a3d45..5959f35 100644 --- a/electron/coding-runtime/conversation-service.ts +++ b/electron/coding-runtime/conversation-service.ts @@ -339,11 +339,24 @@ export class CodingConversationService { } async setModel(conversationId: string, model: ProductModelRef): Promise { - const { project } = await this.projects.findActiveConversation(conversationId); + const { project, conversation } = await this.projects.findActiveConversation(conversationId); let selected: ProductModelRef; try { selected = await this.runtime.validateModel(model); } catch (error) { runtimeError(error); } + if (conversation.modelResolution === 'resolved' && conversation.model) { + await this.ensurePrepared(conversationId); + let state: ConversationModelState; + try { + state = await this.runtime.setModel({ + conversationId, + accountId: selected.accountId, + modelId: selected.modelId, + }); + } catch (error) { runtimeError(error); } + await persist(() => this.projects.conversationStore(project.path).setModelState(conversationId, state)); + return state; + } const state: ConversationModelState = { model: selected, modelResolution: 'resolved', @@ -364,7 +377,7 @@ export class CodingConversationService { const { project } = await this.projects.findActiveConversation(conversationId); try { const state = await this.runtime.setThinking({ conversationId, thinkingLevel }); - await this.projects.conversationStore(project.path).setModelState(conversationId, state); + await persist(() => this.projects.conversationStore(project.path).setModelState(conversationId, state)); return state; } catch (error) { runtimeError(error); } } diff --git a/electron/coding-runtime/in-memory-conversation-runtime.ts b/electron/coding-runtime/in-memory-conversation-runtime.ts index 592282b..ef7e985 100644 --- a/electron/coding-runtime/in-memory-conversation-runtime.ts +++ b/electron/coding-runtime/in-memory-conversation-runtime.ts @@ -2,8 +2,6 @@ import type { CodingConversationRuntime, CodingRuntimeCommand, CodingRuntimeDiagnostics, - CodingRuntimeErrorCode, - CodingRuntimePublicError, ConversationModelState, ConversationInteraction, ConversationInteractionResponse, @@ -22,22 +20,14 @@ import type { SetConversationModelInput, SetThinkingLevelInput, } from './contracts'; +import { CodingRuntimeContractError } from './runtime-errors'; +export { CodingRuntimeContractError } from './runtime-errors'; import { createConversationReducerState, reduceConversationPatch, type ConversationReducerState, } from './conversation-reducer'; -export class CodingRuntimeContractError extends Error { - readonly publicError: CodingRuntimePublicError; - - constructor(code: CodingRuntimeErrorCode, message: string, recoverable: boolean) { - super(message); - this.name = 'CodingRuntimeContractError'; - this.publicError = { code, message, recoverable }; - } -} - export interface InMemoryConversationRuntimeOptions { snapshots?: ConversationSnapshot[]; commands?: CodingRuntimeCommand[]; diff --git a/electron/coding-runtime/pi/runtime.ts b/electron/coding-runtime/pi/runtime.ts index f672995..2c42dce 100644 --- a/electron/coding-runtime/pi/runtime.ts +++ b/electron/coding-runtime/pi/runtime.ts @@ -38,7 +38,7 @@ import { reduceConversationPatch, type ConversationReducerState, } from '../conversation-reducer'; -import { CodingRuntimeContractError } from '../in-memory-conversation-runtime'; +import { CodingRuntimeContractError } from '../runtime-errors'; import { PiEventProjector, type PiEventProjectorOptions, diff --git a/electron/coding-runtime/pi/session-registry.ts b/electron/coding-runtime/pi/session-registry.ts index 19d165d..9839f9a 100644 --- a/electron/coding-runtime/pi/session-registry.ts +++ b/electron/coding-runtime/pi/session-registry.ts @@ -12,6 +12,7 @@ import type { ConversationModelState, PrepareConversationInput, } from '../contracts'; +import { CodingRuntimeContractError } from '../runtime-errors'; export interface PiRegisteredConversation { projectPath: string; @@ -22,6 +23,7 @@ export interface PiRegisteredConversation { export interface PiSessionRegistryOptions { projectStore: CodingProjectStore; + createConversationStore?: typeof createCodingConversationStore; } interface RegistryRecord extends PiRegisteredConversation { @@ -55,11 +57,13 @@ function sameModelState(left: ConversationModelState, right: ConversationModelSt export class PiSessionRegistry { private readonly projectStore: CodingProjectStore; + private readonly createConversationStore: typeof createCodingConversationStore; private readonly records = new Map(); private readonly prepareFlights = new Map>(); constructor(options: PiSessionRegistryOptions) { this.projectStore = options.projectStore; + this.createConversationStore = options.createConversationStore ?? createCodingConversationStore; } async prepare(input: PrepareConversationInput): Promise { @@ -71,7 +75,9 @@ export class PiSessionRegistry { createBinding: () => Promise, ): Promise { const record = await this.prepareRecord(input); - const conversation = await record.store.ensureSessionBinding(input.conversationId, createBinding); + const conversation = await this.persistWrite( + () => record.store.ensureSessionBinding(input.conversationId, createBinding), + ); record.conversation = conversation; record.session = { piSessionId: conversation.piSessionId as string, @@ -86,7 +92,9 @@ export class PiSessionRegistry { ): Promise { const record = this.records.get(conversationId); if (!record) throw new Error('Conversation is not registered'); - record.conversation = await record.store.setModelState(conversationId, model); + record.conversation = await this.persistWrite( + () => record.store.setModelState(conversationId, model), + ); return modelStateOf(record.conversation); } @@ -118,7 +126,7 @@ export class PiSessionRegistry { candidate.id === input.agentId && candidate.enabled && !candidate.archivedAt )); if (!agent) throw new Error('Coding Agent does not exist'); - const store = createCodingConversationStore(project.path); + const store = this.createConversationStore(project.path); const conversation = await store.get(input.conversationId); if (!conversation || conversation.agentId !== input.agentId) { throw new Error('Coding Conversation does not exist for the selected Agent'); @@ -138,4 +146,17 @@ export class PiSessionRegistry { this.records.set(input.conversationId, record); return record; } + + private async persistWrite(operation: () => Promise): Promise { + try { + return await operation(); + } catch (error) { + if (error instanceof CodingRuntimeContractError) throw error; + throw new CodingRuntimeContractError( + 'CODING_STORAGE_WRITE_FAILED', + 'Coding Conversation state could not be persisted', + true, + ); + } + } } diff --git a/electron/coding-runtime/runtime-errors.ts b/electron/coding-runtime/runtime-errors.ts new file mode 100644 index 0000000..17d8935 --- /dev/null +++ b/electron/coding-runtime/runtime-errors.ts @@ -0,0 +1,14 @@ +import type { + CodingRuntimeErrorCode, + CodingRuntimePublicError, +} from './contracts'; + +export class CodingRuntimeContractError extends Error { + readonly publicError: CodingRuntimePublicError; + + constructor(code: CodingRuntimeErrorCode, message: string, recoverable: boolean) { + super(message); + this.name = 'CodingRuntimeContractError'; + this.publicError = { code, message, recoverable }; + } +} diff --git a/tests/unit/coding-core-routes.test.ts b/tests/unit/coding-core-routes.test.ts index f1e9185..b6632d1 100644 --- a/tests/unit/coding-core-routes.test.ts +++ b/tests/unit/coding-core-routes.test.ts @@ -187,6 +187,57 @@ describe('PI-100 coding core Host contract', () => { }); }); + it('switches a resolved active Conversation model through the target runtime without disposing it', async () => { + const result = await setup(); + const conversation = await createConversation(result.conversations); + await result.conversations.acceptPrompt({ + conversationId: conversation.id, + clientRequestId: 'request-active-model-switch', + mode: 'prompt', + text: 'Keep this run active', + attachments: [], + }); + await expect(result.runtime.getSnapshot(conversation.id)).resolves.toMatchObject({ + run: { status: 'running' }, + }); + const setModel = vi.spyOn(result.runtime, 'setModel'); + const dispose = vi.spyOn(result.runtime, 'dispose'); + const nextModel = { ...MODEL, modelId: 'model-next' }; + + await expect(result.conversations.setModel(conversation.id, nextModel)).resolves.toEqual({ + model: nextModel, + modelResolution: 'resolved', + }); + expect(setModel).toHaveBeenCalledWith({ + conversationId: conversation.id, + accountId: nextModel.accountId, + modelId: nextModel.modelId, + }); + expect(dispose).not.toHaveBeenCalled(); + await expect(result.runtime.getSnapshot(conversation.id)).resolves.toMatchObject({ + run: { status: 'running' }, + }); + await expect(result.projects.conversationStore(result.root).get(conversation.id)).resolves.toMatchObject({ + model: nextModel, + modelResolution: 'resolved', + }); + }); + + it('maps Conversation thinking metadata write failures to the stable storage error', async () => { + const result = await setup(); + const conversation = await createConversation(result.conversations); + await result.conversations.getSnapshot(conversation.id); + const store = result.projects.conversationStore(result.root); + vi.spyOn(result.projects, 'conversationStore').mockReturnValue(store); + vi.spyOn(store, 'setModelState') + .mockRejectedValueOnce(new Error('disk full')); + + await expect(result.conversations.setThinking(conversation.id, 'high')).rejects.toMatchObject({ + status: 500, + code: 'CODING_STORAGE_WRITE_FAILED', + }); + }); + it('disposes and moves a bound session to Main-owned trash before deleting metadata', async () => { const result = await setup(); const conversation = await createConversation(result.conversations); diff --git a/tests/unit/coding-provider-auth.test.ts b/tests/unit/coding-provider-auth.test.ts new file mode 100644 index 0000000..b538491 --- /dev/null +++ b/tests/unit/coding-provider-auth.test.ts @@ -0,0 +1,63 @@ +// @vitest-environment node + +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +const mocks = vi.hoisted(() => ({ + getAccount: vi.fn(), + updateAccount: vi.fn(), + resolveCredential: vi.fn(), + getFreshCredential: vi.fn(), + markExpired: vi.fn(), +})); + +vi.mock('../../electron/services/providers/provider-service', () => ({ + getProviderService: () => ({ + getAccount: mocks.getAccount, + updateAccount: mocks.updateAccount, + }), +})); + +vi.mock('../../electron/coding-runtime/pi/provider-config', () => ({ + resolvePiProviderCredentialFromSecretStore: mocks.resolveCredential, +})); + +vi.mock('../../electron/services/works-square-ai-gateway', () => ({ + getFreshWorksSquareAIGatewayCredential: mocks.getFreshCredential, + markWorksSquareAIGatewayCredentialExpired: mocks.markExpired, +})); + +import { + CodingProviderCredentialRefreshError, + isCodingProviderAuthenticationError, + refreshCodingProviderCredential, +} from '../../electron/api/coding-provider-auth'; + +describe('coding Provider credential refresh boundary', () => { + beforeEach(() => { + for (const mock of Object.values(mocks)) mock.mockReset(); + }); + + it('types a rejected production refresh callback as an authentication failure', async () => { + mocks.getAccount.mockRejectedValueOnce(new Error('secure store unavailable')); + + const failure = await refreshCodingProviderCredential('account-a').catch((error) => error); + expect(failure).toBeInstanceOf(CodingProviderCredentialRefreshError); + expect(isCodingProviderAuthenticationError(failure)).toBe(true); + }); + + it('types a missing refreshed Works credential as an authentication failure', async () => { + mocks.getAccount.mockResolvedValueOnce({ + id: 'account-works', + enabled: true, + authMode: 'api_key', + metadata: { worksSquareCredentialMode: 'works_square_ai_gateway' }, + }); + mocks.getFreshCredential.mockResolvedValueOnce(null); + + const failure = await refreshCodingProviderCredential('account-works').catch((error) => error); + expect(mocks.markExpired).toHaveBeenCalledTimes(1); + expect(failure).toBeInstanceOf(CodingProviderCredentialRefreshError); + expect(isCodingProviderAuthenticationError(failure)).toBe(true); + expect(mocks.updateAccount).not.toHaveBeenCalled(); + }); +}); diff --git a/tests/unit/pi-runtime-auth-recovery.test.ts b/tests/unit/pi-runtime-auth-recovery.test.ts index 3c6b6d9..8800a3d 100644 --- a/tests/unit/pi-runtime-auth-recovery.test.ts +++ b/tests/unit/pi-runtime-auth-recovery.test.ts @@ -4,6 +4,10 @@ import { mkdtemp, rm } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import path from 'node:path'; import { afterEach, describe, expect, it, vi } from 'vitest'; +import { + CodingProviderCredentialRefreshError, + isCodingProviderAuthenticationError, +} from '../../electron/api/coding-provider-auth'; import { createCodingConversationStore } from '../../electron/coding-projects/conversation-store'; import { createCodingProjectAgent } from '../../electron/coding-projects/project-config'; import { @@ -64,67 +68,70 @@ afterEach(async () => { await Promise.all(roots.splice(0).map((root) => rm(root, { recursive: true, force: true }))); }); +async function setupAuthRuntime(refreshCredential: (accountId: string) => Promise) { + const projectPath = await mkdtemp(path.join(tmpdir(), 'makelore-pi-auth-')); + roots.push(projectPath); + const projectStore = createCodingProjectStore(createMemoryCodingProjectStorage(), { + createId: () => 'project-auth', + now: () => NOW, + }); + await createLocalCodingProject({ projectPath, now: NOW }, projectStore); + await createCodingProjectAgent(projectPath, { + id: 'agent-auth', + avatarId: 'avatar-01', + roleName: 'Implementer', + name: 'Auth Agent', + model: { accountId: 'account-auth', modelId: 'model-auth', thinkingLevel: 'medium' }, + modelResolution: 'resolved', + responsibility: { mission: 'Implement', owns: [], boundaries: [], collaborators: [], principles: [] }, + }, { now: NOW }); + const store = createCodingConversationStore(projectPath, { + createId: () => 'f47ac10b-58cc-4372-a567-0e02b2c3d479', + now: () => NOW, + }); + const conversation = await store.create({ + agentId: 'agent-auth', + title: 'Auth Conversation', + model: { accountId: 'account-auth', modelId: 'model-auth', thinkingLevel: 'medium' }, + modelResolution: 'resolved', + }); + const workers: AuthFailureWorker[] = []; + const pool = new PiWorkerPool({ + maxIdle: 2, + openWorker: async ({ conversation: input, existingSession }) => { + const worker = new AuthFailureWorker(`worker-${workers.length + 1}`); + workers.push(worker); + return { + worker, + session: existingSession ?? { + piSessionId: `session-${input.conversationId}`, + sessionKey: `key-${input.conversationId}`, + }, + }; + }, + }); + const runtime = new PiConversationRuntime({ + pool, + registry: new PiSessionRegistry({ projectStore }), + resolveModel: async () => { throw new Error('not used'); }, + refreshCredential, + isAuthenticationError: isCodingProviderAuthenticationError, + createId: () => 'run-auth', + }); + await runtime.prepare({ + conversationId: conversation.id, + projectId: 'project-auth', + agentId: 'agent-auth', + title: conversation.title, + model: { model: conversation.model, modelResolution: conversation.modelResolution }, + }); + return { conversation, pool, runtime, workers }; +} + describe('Pi runtime Provider authentication recovery', () => { it('refreshes and reopens once, then exposes the second authentication failure without looping', async () => { - const projectPath = await mkdtemp(path.join(tmpdir(), 'makelore-pi-auth-')); - roots.push(projectPath); - const projectStore = createCodingProjectStore(createMemoryCodingProjectStorage(), { - createId: () => 'project-auth', - now: () => NOW, - }); - await createLocalCodingProject({ projectPath, now: NOW }, projectStore); - await createCodingProjectAgent(projectPath, { - id: 'agent-auth', - avatarId: 'avatar-01', - roleName: 'Implementer', - name: 'Auth Agent', - model: { accountId: 'account-auth', modelId: 'model-auth', thinkingLevel: 'medium' }, - modelResolution: 'resolved', - responsibility: { mission: 'Implement', owns: [], boundaries: [], collaborators: [], principles: [] }, - }, { now: NOW }); - const store = createCodingConversationStore(projectPath, { - createId: () => 'f47ac10b-58cc-4372-a567-0e02b2c3d479', - now: () => NOW, - }); - const conversation = await store.create({ - agentId: 'agent-auth', - title: 'Auth Conversation', - model: { accountId: 'account-auth', modelId: 'model-auth', thinkingLevel: 'medium' }, - modelResolution: 'resolved', - }); - const workers: AuthFailureWorker[] = []; - const pool = new PiWorkerPool({ - maxIdle: 2, - openWorker: async ({ conversation: input, existingSession }) => { - const worker = new AuthFailureWorker(`worker-${workers.length + 1}`); - workers.push(worker); - return { - worker, - session: existingSession ?? { - piSessionId: `session-${input.conversationId}`, - sessionKey: `key-${input.conversationId}`, - }, - }; - }, - }); const refreshCredential = vi.fn(async () => undefined); - const runtime = new PiConversationRuntime({ - pool, - registry: new PiSessionRegistry({ projectStore }), - resolveModel: async () => { throw new Error('not used'); }, - refreshCredential, - isAuthenticationError: (error) => ( - error instanceof PiProcessError && error.message.includes('401') - ), - createId: () => 'run-auth', - }); - await runtime.prepare({ - conversationId: conversation.id, - projectId: 'project-auth', - agentId: 'agent-auth', - title: conversation.title, - model: { model: conversation.model, modelResolution: conversation.modelResolution }, - }); + const { conversation, pool, runtime, workers } = await setupAuthRuntime(refreshCredential); await expect(runtime.prompt({ clientRequestId: 'request-auth', @@ -147,4 +154,25 @@ describe('Pi runtime Provider authentication recovery', () => { .toEqual([1, 1]); expect(pool.getState(conversation.id)).toMatchObject({ state: 'idle', generation: 2 }); }); + + it('projects a rejected credential refresh as authentication required', async () => { + const refreshCredential = vi.fn(async () => { + throw new CodingProviderCredentialRefreshError('Provider credential refresh failed'); + }); + const { conversation, runtime } = await setupAuthRuntime(refreshCredential); + + await expect(runtime.prompt({ + clientRequestId: 'request-auth-refresh-rejected', + conversationId: conversation.id, + mode: 'prompt', + text: 'Do not leak refresh failures', + attachments: [], + })).rejects.toMatchObject({ + publicError: { + code: 'CODING_PROVIDER_AUTH_REQUIRED', + recoverable: true, + }, + }); + expect(refreshCredential).toHaveBeenCalledTimes(1); + }); }); diff --git a/tests/unit/pi-session-registry.test.ts b/tests/unit/pi-session-registry.test.ts index 6d9419d..97240c5 100644 --- a/tests/unit/pi-session-registry.test.ts +++ b/tests/unit/pi-session-registry.test.ts @@ -92,4 +92,101 @@ describe('Pi session registry', () => { session: { piSessionId: 'pi-session-a', sessionKey: 'session-key-a' }, }); }); + + it('maps first session binding persistence failure to the stable storage error', async () => { + const projectPath = await mkdtemp(path.join(tmpdir(), 'makelore-pi-registry-binding-failure-')); + roots.push(projectPath); + const projectStore = createCodingProjectStore(createMemoryCodingProjectStorage(), { + createId: () => 'project-binding-failure', + now: () => NOW, + }); + await createLocalCodingProject({ projectPath, now: NOW }, projectStore); + await createCodingProjectAgent(projectPath, { + id: 'agent-a', + avatarId: 'avatar-01', + roleName: 'Implementer', + name: 'Agent A', + model: { accountId: 'account-a', modelId: 'model-a', thinkingLevel: 'medium' }, + modelResolution: 'resolved', + responsibility: { mission: 'Implement', owns: [], boundaries: [], collaborators: [], principles: [] }, + }, { now: NOW }); + const conversations = createCodingConversationStore(projectPath, { + createId: () => 'f47ac10b-58cc-4372-a567-0e02b2c3d480', + now: () => NOW, + }); + const created = await conversations.create({ + agentId: 'agent-a', + title: 'Conversation A', + model: { accountId: 'account-a', modelId: 'model-a', thinkingLevel: 'medium' }, + modelResolution: 'resolved', + }); + const registry = new PiSessionRegistry({ + projectStore, + createConversationStore: () => conversations, + }); + const input = { + conversationId: created.id, + projectId: 'project-binding-failure', + agentId: 'agent-a', + title: created.title, + model: { model: created.model, modelResolution: created.modelResolution }, + } as const; + await registry.prepare(input); + vi.spyOn(conversations, 'ensureSessionBinding').mockRejectedValueOnce(new Error('disk full')); + + await expect(registry.ensureBinding(input, async () => ({ + piSessionId: 'pi-session-a', + sessionKey: 'session-key-a', + }))).rejects.toMatchObject({ + publicError: { code: 'CODING_STORAGE_WRITE_FAILED', recoverable: true }, + }); + }); + + it('maps model metadata persistence failure to the stable storage error', async () => { + const projectPath = await mkdtemp(path.join(tmpdir(), 'makelore-pi-registry-model-failure-')); + roots.push(projectPath); + const projectStore = createCodingProjectStore(createMemoryCodingProjectStorage(), { + createId: () => 'project-model-failure', + now: () => NOW, + }); + await createLocalCodingProject({ projectPath, now: NOW }, projectStore); + await createCodingProjectAgent(projectPath, { + id: 'agent-a', + avatarId: 'avatar-01', + roleName: 'Implementer', + name: 'Agent A', + model: { accountId: 'account-a', modelId: 'model-a', thinkingLevel: 'medium' }, + modelResolution: 'resolved', + responsibility: { mission: 'Implement', owns: [], boundaries: [], collaborators: [], principles: [] }, + }, { now: NOW }); + const conversations = createCodingConversationStore(projectPath, { + createId: () => 'f47ac10b-58cc-4372-a567-0e02b2c3d481', + now: () => NOW, + }); + const created = await conversations.create({ + agentId: 'agent-a', + title: 'Conversation A', + model: { accountId: 'account-a', modelId: 'model-a', thinkingLevel: 'medium' }, + modelResolution: 'resolved', + }); + const registry = new PiSessionRegistry({ + projectStore, + createConversationStore: () => conversations, + }); + await registry.prepare({ + conversationId: created.id, + projectId: 'project-model-failure', + agentId: 'agent-a', + title: created.title, + model: { model: created.model, modelResolution: created.modelResolution }, + }); + vi.spyOn(conversations, 'setModelState').mockRejectedValueOnce(new Error('disk full')); + + await expect(registry.setModel(created.id, { + model: { accountId: 'account-b', modelId: 'model-b', thinkingLevel: 'high' }, + modelResolution: 'resolved', + })).rejects.toMatchObject({ + publicError: { code: 'CODING_STORAGE_WRITE_FAILED', recoverable: true }, + }); + }); });