diff --git a/docs/features/magic-notes/technical-design.md b/docs/features/magic-notes/technical-design.md index d54ef666..02f8172f 100644 --- a/docs/features/magic-notes/technical-design.md +++ b/docs/features/magic-notes/technical-design.md @@ -474,6 +474,13 @@ The built-in MCP gateway returns invalid scoped-tool arguments as an the model to correct its arguments without an MCP internal-error response. Validation failures do not execute the tool. +Note tools can be authorized with the knowledge-base switch off. The shared +desktop gateway binds authorization to each request, without a fixed elapsed-time +expiry, so long-running requests retain tool access. Request completion, +cancellation, failure cleanup, or gateway disposal revokes the token. Invalid or +revoked tokens report unavailable tool authorization rather than a knowledge-base +expiry error. + These note tools run in the desktop gateway. Remote gbagent ACP sessions currently inject only the image MCP server, so this change requires no daemon implementation update. diff --git a/src/main/agent/knowledge-mcp-gateway.obsidian.test.ts b/src/main/agent/knowledge-mcp-gateway.obsidian.test.ts index fe024ef1..caa85931 100644 --- a/src/main/agent/knowledge-mcp-gateway.obsidian.test.ts +++ b/src/main/agent/knowledge-mcp-gateway.obsidian.test.ts @@ -151,7 +151,7 @@ describe('Obsidian gateway', () => { gateway.revokeObsidianCapabilities() gateway.revokeObsidianCapabilities() for (const revoked of [token, writeToken, mixedToken]) { - expect(() => gateway.getAvailableToolNames(revoked)).toThrow('unavailable or expired') + expect(() => gateway.getAvailableToolNames(revoked)).toThrow('Tool authorization is unavailable') } expect(gateway.getAvailableToolNames(knowledgeToken)).toEqual(['knowledge_list', 'knowledge_search']) const replacement = gateway.grant('replacement', [], signal, 'none', undefined, undefined, undefined, undefined, diff --git a/src/main/agent/knowledge-mcp-gateway.test.ts b/src/main/agent/knowledge-mcp-gateway.test.ts index 791d1d90..18f50ba9 100644 --- a/src/main/agent/knowledge-mcp-gateway.test.ts +++ b/src/main/agent/knowledge-mcp-gateway.test.ts @@ -199,6 +199,7 @@ const temporaryDirectories: string[] = [] const httpServers: Server[] = [] afterEach(async () => { + vi.restoreAllMocks() await Promise.all(gateways.splice(0).map((gateway) => gateway.dispose())) for (const database of databases.splice(0)) { database.close() @@ -663,13 +664,10 @@ describe('KnowledgeMcpGateway', () => { ).rejects.toThrow() }) - it('creates no capability for empty scope and rejects revoked, aborted, and expired capabilities', async () => { + it('keeps long-running request capabilities valid until revoked or aborted', async () => { const { service } = createService() - let now = 1_000 - const gateway = new KnowledgeMcpGateway(service, { - capabilityTtlMs: 10, - now: () => now - }) + const clock = vi.spyOn(Date, 'now').mockReturnValue(1_000) + const gateway = new KnowledgeMcpGateway(service) gateways.push(gateway) expect( gateway.grant('empty', [], new AbortController().signal) @@ -680,10 +678,12 @@ describe('KnowledgeMcpGateway', () => { [firstLibraryId], new AbortController().signal )! + clock.mockReturnValue(1_000 + 24 * 60 * 60_000) + await expect(gateway.search(revoked, { query: 'x' })).resolves.toHaveLength(1) gateway.revoke(revoked) await expect( gateway.search(revoked, { query: 'x' }) - ).rejects.toThrow('unavailable or expired') + ).rejects.toThrow('Tool authorization is unavailable') const abortController = new AbortController() const aborted = gateway.grant( @@ -694,21 +694,23 @@ describe('KnowledgeMcpGateway', () => { abortController.abort() await expect( gateway.search(aborted, { query: 'x' }) - ).rejects.toThrow('unavailable or expired') + ).rejects.toThrow('Tool authorization is unavailable') - const expired = gateway.grant( - 'expired', + const nextRequest = gateway.grant( + 'next-request', [firstLibraryId], new AbortController().signal )! - now += 11 + expect(nextRequest).not.toBe(revoked) await expect( - gateway.search(expired, { query: 'x' }) - ).rejects.toThrow('unavailable or expired') + gateway.search(nextRequest, { query: 'x' }) + ).resolves.toHaveLength(1) + clock.mockRestore() }) - it('grants bounded global Magic Notes search without a knowledge scope', () => { + it('keeps Magic Notes available during long requests without a knowledge scope', () => { const { service } = createService() + const clock = vi.spyOn(Date, 'now').mockReturnValue(1_000) const searchMagicNotes = vi.fn(() => [ { noteId: '00000000-0000-4000-8000-000000000701', @@ -754,6 +756,7 @@ describe('KnowledgeMcpGateway', () => { 'read' )! + clock.mockReturnValue(1_000 + 24 * 60 * 60_000) expect(gateway.getAvailableToolNames(token)).toEqual([ 'note_list', 'note_get', @@ -777,6 +780,9 @@ describe('KnowledgeMcpGateway', () => { noteIds: ['not-allowed'] }) ).toThrow() + gateway.revoke(token) + expect(() => gateway.getAvailableToolNames(token)).toThrow('Tool authorization is unavailable') + clock.mockRestore() }) it('keeps Ask read-only and supports revision-safe Magic Notes CRUD in Execute', async () => { @@ -980,6 +986,7 @@ describe('KnowledgeMcpGateway', () => { }) it('proxies custom MCP through a request-scoped loopback token without exposing the upstream credential', async () => { + const clock = vi.spyOn(Date, 'now').mockReturnValue(1_000) const upstreamAuthorizations: Array = [] const upstream = createServer(async (request, response) => { upstreamAuthorizations.push(request.headers.authorization) @@ -1081,6 +1088,7 @@ describe('KnowledgeMcpGateway', () => { expect(JSON.stringify(listed)).not.toContain( `127.0.0.1:${address.port}` ) + clock.mockReturnValue(1_000 + 24 * 60 * 60_000) const result = await client.callTool({ name: listed.tools[0]!.name, arguments: { value: 'hello' } @@ -1094,6 +1102,7 @@ describe('KnowledgeMcpGateway', () => { } finally { gateway.revoke(token) await client.close() + clock.mockRestore() } }) diff --git a/src/main/agent/knowledge-mcp-gateway.ts b/src/main/agent/knowledge-mcp-gateway.ts index be5deeac..12465760 100644 --- a/src/main/agent/knowledge-mcp-gateway.ts +++ b/src/main/agent/knowledge-mcp-gateway.ts @@ -103,8 +103,6 @@ const MAX_DOWNSTREAM_MCP_SESSIONS_PER_CAPABILITY = 8 const CUSTOM_MCP_TIMEOUT_MS = 30_000 const CUSTOM_MCP_MAX_TOTAL_TIMEOUT_MS = 5 * 60_000 const CUSTOM_MCP_TASK_CANCEL_TIMEOUT_MS = 5_000 -const DEFAULT_CAPABILITY_TTL_MS = 10 * 60_000 -const MAX_CAPABILITY_TTL_MS = 15 * 60_000 const customMcpJsonSchemaValidator = new AjvJsonSchemaValidator() export { @@ -215,7 +213,6 @@ type Capability = { browserTabId?: BrowserTabId browserUsageLease?: BrowserTabUsageLease obsidian?: { settings: ObsidianSettings; access: 'read' | 'write' } - expiresAt: number signal: AbortSignal brokerController: AbortController customMcpServers: readonly ResolvedMcpServer[] @@ -256,9 +253,7 @@ type DownstreamMcpSession = { export type KnowledgeMcpGatewayOptions = { storyGraphService?: StoryGraphService - capabilityTtlMs?: number maximumBodyBytes?: number - now?: () => number magicNotesDatabase?: MagicNotesDatabase configService?: GoodBuddyConfigService obsidianService?: ObsidianService @@ -387,8 +382,6 @@ export class KnowledgeMcpGateway { private readonly downstreamMcpCleanups = new Set>() private readonly customMcpCleanups = new Set>() private readonly activeObsidianCalls = new Set>() - private readonly now: () => number - private readonly capabilityTtlMs: number private readonly maximumBodyBytes: number private readonly magicNotesDatabase?: MagicNotesDatabase private readonly configService?: GoodBuddyConfigService @@ -410,18 +403,8 @@ export class KnowledgeMcpGateway { private readonly knowledgeService: KnowledgeService, options: KnowledgeMcpGatewayOptions = {} ) { - const ttl = options.capabilityTtlMs ?? DEFAULT_CAPABILITY_TTL_MS - if ( - !Number.isSafeInteger(ttl) || - ttl < 1 || - ttl > MAX_CAPABILITY_TTL_MS - ) { - throw new RangeError('Knowledge capability TTL is invalid') - } - this.capabilityTtlMs = ttl this.maximumBodyBytes = options.maximumBodyBytes ?? MAX_REQUEST_BODY_BYTES - this.now = options.now ?? Date.now this.storyGraphService = options.storyGraphService this.magicNotesDatabase = options.magicNotesDatabase this.configService = options.configService @@ -615,7 +598,6 @@ export class KnowledgeMcpGateway { private storeCapability( value: Omit< Capability, - | 'expiresAt' | 'references' | 'removeAbortListener' | 'brokerController' @@ -628,10 +610,10 @@ export class KnowledgeMcpGateway { const abort = (): void => { this.revoke(token) } + // Capabilities live with the request; elapsed time must not interrupt long runs. value.signal.addEventListener('abort', abort, { once: true }) this.capabilities.set(token, { ...value, - expiresAt: this.now() + this.capabilityTtlMs, brokerController, references: new Map(), removeAbortListener: () => { @@ -713,13 +695,9 @@ export class KnowledgeMcpGateway { private getCapability(token: string): Capability { const capability = this.capabilities.get(token) - if ( - !capability || - capability.signal.aborted || - capability.expiresAt <= this.now() - ) { + if (!capability || capability.signal.aborted) { this.revoke(token) - throw new Error('Knowledge capability is unavailable or expired') + throw new Error('Tool authorization is unavailable: the request has ended or the token is invalid') } return capability } diff --git a/src/main/index.ts b/src/main/index.ts index a9edcade..7acdc226 100644 --- a/src/main/index.ts +++ b/src/main/index.ts @@ -1446,8 +1446,6 @@ if (hasSingleInstanceLock) { (await capabilityService.getEnabledBuiltinMcpServerIds(runtimeTarget)).includes('story-graph'), read: (name, input, projectId, signal) => startupAssistantDatabase.readStoryGraph(name, input, projectId, signal) }, - // This dedicated gateway lives until native-client disposal, not a chat request timeout. - now: () => 0, magicNotesDatabase: startupAssistantDatabase, configService: goodbuddyConfigService, obsidianService, launchEnvironmentProvider: startupLocalToolEnvironmentService.launchEnvironmentProvider }) diff --git a/src/main/ipc.test.ts b/src/main/ipc.test.ts index 7e8e4591..49c2a064 100644 --- a/src/main/ipc.test.ts +++ b/src/main/ipc.test.ts @@ -9041,7 +9041,7 @@ describe('registerIpcHandlers agent terminal state', () => { rootDirectory: join(root, 'clients'), managedNodeDirectory: join(root, 'node'), bundledRuntimePaths: resolveBundledRuntimePaths({ appPath: process.cwd(), resourcesPath: '', packaged: false }), npmCliPath: resolveNpmCliPaths({ appPath: process.cwd(), resourcesPath: '', packaged: false }).npmCliPath, - createGateway: () => new KnowledgeMcpGateway({} as never, { now: () => 0 }), + createGateway: () => new KnowledgeMcpGateway({} as never), openExternal: async (url) => { opened.push(url) } }) harness = createHarness({}, undefined, 'always', undefined, false, undefined, undefined, undefined,