fix: keep tool authorization valid throughout long requests
Remove the fixed gateway capability lifetime so active requests retain knowledge, notes, browser, configuration, and custom MCP access. Preserve token validation and request cancellation or cleanup revocation, and replace the misleading knowledge expiry error with a tool authorization error. Remove native-client clock overrides and cover long-running scoped and HTTP MCP calls. Validation: 34 focused tests passed; changed-file lint passed. Repository typecheck and lint remain blocked by unrelated manual-compaction and sidebar probe files. Release note: Long-running Agent requests no longer lose tool access after ten minutes, including when the knowledge-base switch is off. Request cancellation and completion still revoke access.
This commit is contained in:
@@ -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,
|
||||
|
||||
@@ -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<string | undefined> = []
|
||||
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()
|
||||
}
|
||||
})
|
||||
|
||||
|
||||
@@ -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<Promise<void>>()
|
||||
private readonly customMcpCleanups = new Set<Promise<void>>()
|
||||
private readonly activeObsidianCalls = new Set<Promise<CallToolResult>>()
|
||||
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
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user