From 455e522f1ef931b3dafaa27c9a28b89a69eebc07 Mon Sep 17 00:00:00 2001 From: VectorPeak <73048950+VectorPeak@users.noreply.github.com> Date: Sun, 19 Jul 2026 09:51:08 +0800 Subject: [PATCH] fix(desktop): align source_test metadata contract Co-authored-by: chatgpt-codex-connector[bot] <199175422+chatgpt-codex-connector[bot]@users.noreply.github.com> --- .../src/handlers/source-test.test.ts | 56 +++++++++++++++++++ .../src/handlers/source-test.ts | 15 +++-- .../packages/session-tools-core/src/types.ts | 4 +- 3 files changed, 68 insertions(+), 7 deletions(-) diff --git a/packages/desktop/packages/session-tools-core/src/handlers/source-test.test.ts b/packages/desktop/packages/session-tools-core/src/handlers/source-test.test.ts index b59eb500817..f290e9b8c0d 100644 --- a/packages/desktop/packages/session-tools-core/src/handlers/source-test.test.ts +++ b/packages/desktop/packages/session-tools-core/src/handlers/source-test.test.ts @@ -24,6 +24,29 @@ interface CtxOverrides { validateStdioMcpConnection?: SessionToolContext['validateStdioMcpConnection']; } +const SHARED_CONNECTION_STATUSES = new Set>([ + 'connected', + 'needs_auth', + 'failed', + 'untested', +]); + +function assertSharedSourceMetadataContract(source: SourceConfig): void { + if ( + source.lastTestedAt !== undefined && + (!Number.isInteger(source.lastTestedAt) || source.lastTestedAt < 0) + ) { + throw new Error('lastTestedAt must be a non-negative integer timestamp'); + } + + if ( + source.connectionStatus !== undefined && + !SHARED_CONNECTION_STATUSES.has(source.connectionStatus) + ) { + throw new Error(`unsupported connectionStatus: ${source.connectionStatus}`); + } +} + function createCtx(workspacePath: string, overrides: CtxOverrides = {}): SessionToolContext { const saved: { last?: SourceConfig } = {}; const ctx = { @@ -58,6 +81,7 @@ function createCtx(workspacePath: string, overrides: CtxOverrides = {}): Session return JSON.parse(readFileSync(configPath, 'utf-8')) as SourceConfig; }, saveSourceConfig: (source: SourceConfig) => { + assertSharedSourceMetadataContract(source); saved.last = source; const configPath = join(workspacePath, 'sources', source.slug, 'config.json'); writeFileSync(configPath, JSON.stringify(source, null, 2)); @@ -149,6 +173,8 @@ describe('source_test auto-enable', () => { readFileSync(join(tempDir, 'sources', 'craft-kb', 'config.json'), 'utf-8') ) as SourceConfig; expect(persisted.enabled).toBe(true); + expect(Number.isInteger(persisted.lastTestedAt)).toBe(true); + expect(persisted.connectionStatus).toBe('connected'); }); it('already-enabled source still calls activation callback (session may be stale)', async () => { @@ -192,6 +218,8 @@ describe('source_test auto-enable', () => { ) as SourceConfig; // saveSourceConfig still runs (metadata update), but enabled flag must remain false. expect(persisted.enabled).toBe(false); + expect(Number.isInteger(persisted.lastTestedAt)).toBe(true); + expect(persisted.connectionStatus).toBe('connected'); }); it('validation errors skip auto-enable entirely (even when autoEnable is default)', async () => { @@ -217,6 +245,34 @@ describe('source_test auto-enable', () => { readFileSync(join(tempDir, 'sources', 'broken', 'config.json'), 'utf-8') ) as SourceConfig; expect(persisted.enabled).toBe(false); + expect(Number.isInteger(persisted.lastTestedAt)).toBe(true); + expect(persisted.connectionStatus).toBe('failed'); + expect(persisted.connectionError).toBe('boom'); + }); + + it('persists needs_auth when connection succeeds but auth is missing', async () => { + writeSource(tempDir, 'oauth-source', { + isAuthenticated: false, + mcp: { + transport: 'stdio', + command: 'echo', + args: ['ok'], + authType: 'oauth', + }, + }); + + const ctx = createCtx(tempDir, { + validateStdioMcpConnection: stubMcpOk(), + }); + + const result = await handleSourceTest(ctx, { sourceSlug: 'oauth-source' }); + + expect(result.isError).toBe(false); + const persisted = JSON.parse( + readFileSync(join(tempDir, 'sources', 'oauth-source', 'config.json'), 'utf-8') + ) as SourceConfig; + expect(Number.isInteger(persisted.lastTestedAt)).toBe(true); + expect(persisted.connectionStatus).toBe('needs_auth'); }); it('without activateSourceInSession, flag flip still happens with restart hint', async () => { diff --git a/packages/desktop/packages/session-tools-core/src/handlers/source-test.ts b/packages/desktop/packages/session-tools-core/src/handlers/source-test.ts index 31cfe03d5df..75e39797fe3 100644 --- a/packages/desktop/packages/session-tools-core/src/handlers/source-test.ts +++ b/packages/desktop/packages/session-tools-core/src/handlers/source-test.ts @@ -65,7 +65,7 @@ export async function handleSourceTest( const lines: string[] = []; let hasErrors = false; let hasWarnings = false; - let connectionStatus: ConnectionStatus = 'unknown'; + let connectionStatus: ConnectionStatus = 'untested'; let connectionError: string | undefined; // 1. Check source exists @@ -122,19 +122,24 @@ export async function handleSourceTest( lines.push(...connectionResult.lines); if (connectionResult.hasError) { hasErrors = true; - connectionStatus = 'error'; + connectionStatus = 'failed'; connectionError = connectionResult.error; } else if (connectionResult.success) { connectionStatus = 'connected'; } else { - connectionStatus = 'disconnected'; + connectionStatus = 'untested'; } // 7. Auth status lines.push('\n## Authentication'); const authResult = await checkAuthStatus(ctx, source, sourceSlug); lines.push(...authResult.lines); - if (authResult.hasWarning) hasWarnings = true; + if (authResult.hasWarning) { + hasWarnings = true; + if (!connectionResult.hasError) { + connectionStatus = 'needs_auth'; + } + } // 8. Auto-enable + metadata update // Defaults to true; pass autoEnable: false to keep pure validation behavior. @@ -145,7 +150,7 @@ export async function handleSourceTest( if (ctx.saveSourceConfig) { const updatedSource: SourceConfig = { ...source, - lastTestedAt: new Date().toISOString(), + lastTestedAt: Date.now(), connectionStatus, connectionError, // Fold enabled flip into the same save — one write, not two. diff --git a/packages/desktop/packages/session-tools-core/src/types.ts b/packages/desktop/packages/session-tools-core/src/types.ts index 5bab403b51a..d4711af2e28 100644 --- a/packages/desktop/packages/session-tools-core/src/types.ts +++ b/packages/desktop/packages/session-tools-core/src/types.ts @@ -299,7 +299,7 @@ export interface LocalSourceConfig { /** * Connection status for sources */ -export type ConnectionStatus = 'connected' | 'disconnected' | 'error' | 'unknown'; +export type ConnectionStatus = 'connected' | 'needs_auth' | 'failed' | 'untested'; /** * Full source configuration (simplified version for core package) @@ -315,7 +315,7 @@ export interface SourceConfig { api?: ApiSourceConfig; local?: LocalSourceConfig; isAuthenticated?: boolean; - lastTestedAt?: string; // ISO date string + lastTestedAt?: number; // Unix timestamp in milliseconds createdAt?: number; updatedAt?: number; // Display fields