From b03582ae8740e6b56d12945e95f57cd658048fd8 Mon Sep 17 00:00:00 2001 From: Danny Avila Date: Sun, 27 Sep 2026 11:41:22 -0400 Subject: [PATCH 1/2] feat: Allow Installation-Scoped GitHub Tokens for Trusted Workers (#263) * feat(code): allow installation-scoped GitHub tokens on trusted workers * fix(code): revalidate GitHub installation token grants --------- Co-authored-by: Lia --- docs/remote-bridge/worker-runbook.md | 9 + packages/code/README.md | 11 ++ packages/code/src/cli.test.ts | 29 +++ packages/code/src/cli.ts | 23 ++- packages/code/src/github.test.ts | 266 +++++++++++++++++++++++++++ packages/code/src/github.ts | 159 ++++++++++++---- 6 files changed, 459 insertions(+), 38 deletions(-) diff --git a/docs/remote-bridge/worker-runbook.md b/docs/remote-bridge/worker-runbook.md index 3043c541..493eb000 100644 --- a/docs/remote-bridge/worker-runbook.md +++ b/docs/remote-bridge/worker-runbook.md @@ -338,6 +338,15 @@ filesystem root, but anyone able to alter a checkout's remote can select any repository where the App is installed; keep the App's installation scope narrow. Pass the checkout as the command working directory; changing directories only inside the shell cannot change the token chosen before command launch. +For a trusted VM that needs to switch among repositories in the same installed +account or organization inside one command, set +`LIBRECHAT_CODE_GITHUB_TOKEN_SCOPE=installation`. The resolved installation +token covers only repositories and permissions GitHub granted to that App +installation. It refreshes after two minutes so newly approved permissions +become available without a worker restart. The default is `repository`. +Commands spanning different accounts or organizations must start in a checkout +from the target account or organization; a shell `cd` cannot switch the +installation chosen at command launch. Set `LIBRECHAT_CODE_GITHUB_INSTALLATION_ID` only as a legacy fixed-installation fallback; it cannot be combined with checkout routing. diff --git a/packages/code/README.md b/packages/code/README.md index 5a06b92a..a87c87df 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -308,6 +308,17 @@ installed**. Use this mode only where the machine operator trusts the VM and the App's installation scope; the default `admitted` mode keeps the startup binding. Checkout routing requires an App without a fixed installation ID. +On a trusted VM, `--github-token-scope installation` (or +`LIBRECHAT_CODE_GITHUB_TOKEN_SCOPE=installation`) mints one token for all +repositories GitHub grants to the resolved App installation. This lets a +command started in one checkout push to another repository in the same account +or organization, including through `cd` or `git -C`, and use organization +Projects. GitHub still enforces the installation's selected repositories and +permissions. Tokens are shared by installation, refreshed after two minutes, +and kept out of the sandbox's readable environment. The default remains +`repository`. A command crossing to another account or organization still +needs to start in a checkout belonging to that account or organization. + For compatibility with deployments that intentionally bind a worker to one installation, set the optional legacy `LIBRECHAT_CODE_GITHUB_INSTALLATION_ID` fallback. diff --git a/packages/code/src/cli.test.ts b/packages/code/src/cli.test.ts index f95d7eee..f6737d5f 100644 --- a/packages/code/src/cli.test.ts +++ b/packages/code/src/cli.test.ts @@ -358,6 +358,35 @@ test('CLI rejects checkout routing outside a trusted VM or without repository-sc assert.match(invalid.stderr, /must be admitted or checkout/); }); +test('CLI permits installation-scoped GitHub tokens only for a trusted VM with routed App auth', () => { + const cli = fileURLToPath(new URL('./cli.js', import.meta.url)); + const base = { + ...process.env, + LIBRECHAT_CODE_URL: 'http://127.0.0.1:1/v1', + LIBRECHAT_CODE_WORKER_TOKEN: 'worker-secret', + LIBRECHAT_CODE_WORKER_ID: 'engineering-vm', + LIBRECHAT_CODE_WORKER_DIR: process.cwd(), + LIBRECHAT_CODE_ALLOW_WORKSPACE_COMMANDS: 'true', + LIBRECHAT_CODE_GITHUB_TOKEN: undefined, + LIBRECHAT_CODE_GITHUB_APP_ID: '123', + LIBRECHAT_CODE_GITHUB_PRIVATE_KEY_FILE: '/does/not/matter', + LIBRECHAT_CODE_GITHUB_INSTALLATION_ID: undefined, + LIBRECHAT_CODE_GITHUB_TOKEN_SCOPE: 'installation', + }; + const restricted = spawnSync(process.execPath, [cli], { encoding: 'utf8', env: base }); + assert.match(restricted.stderr, /Installation-scoped GitHub tokens require the trusted-vm/); + const trusted = spawnSync(process.execPath, [cli], { + encoding: 'utf8', + env: { ...base, LIBRECHAT_CODE_COMMAND_POLICY_PRESET: 'trusted-vm' }, + }); + assert.doesNotMatch(trusted.stderr, /Installation-scoped GitHub tokens require/); + const fixed = spawnSync(process.execPath, [cli], { + encoding: 'utf8', + env: { ...base, LIBRECHAT_CODE_GITHUB_INSTALLATION_ID: '456' }, + }); + assert.match(fixed.stderr, /without a fixed installation ID/); +}); + test('CLI requires a runtime image for Docker supervision', () => { const result = spawnSync( process.execPath, diff --git a/packages/code/src/cli.ts b/packages/code/src/cli.ts index 841164fb..6e3a0062 100644 --- a/packages/code/src/cli.ts +++ b/packages/code/src/cli.ts @@ -157,6 +157,7 @@ function githubCredentials(args: string[]): { mode?: 'app' | 'token'; repositoryRouting?: boolean; checkoutRouting?: boolean; + installationTokenScope?: boolean; policyIdentity: string; } { const token = nonEmpty(process.env.LIBRECHAT_CODE_GITHUB_TOKEN); @@ -191,6 +192,18 @@ function githubCredentials(args: string[]): { 'Checkout GitHub repository routing requires a GitHub App without a fixed installation ID', ); } + const tokenScope = + option(args, '--github-token-scope')?.trim().toLowerCase() ?? + process.env.LIBRECHAT_CODE_GITHUB_TOKEN_SCOPE?.trim().toLowerCase() ?? + 'repository'; + if (tokenScope !== 'repository' && tokenScope !== 'installation') { + throw new Error('GitHub token scope must be repository or installation'); + } + if (tokenScope === 'installation' && (!hasApp || installationId)) { + throw new Error( + 'Installation-scoped GitHub tokens require a GitHub App without a fixed installation ID', + ); + } const configuredHostValue = nonEmpty( process.env.LIBRECHAT_CODE_GITHUB_HOST, ); @@ -225,16 +238,19 @@ function githubCredentials(args: string[]): { mode: 'app', repositoryRouting: !installationId, checkoutRouting: routing === 'checkout', + installationTokenScope: tokenScope === 'installation', policyIdentity: gitHubAuthenticationPolicyIdentity({ mode: 'app', host, appId, installationId, - }) + (routing === 'checkout' ? ':routing:checkout' : ''), + }) + (routing === 'checkout' ? ':routing:checkout' : '') + + (tokenScope === 'installation' ? ':scope:installation' : ''), privateKeyPath, provider: new GitHubAppCredentialProvider({ appId: appId!, installationId, + tokenScope, privateKeyPath: privateKeyPath!, host, apiUrl, @@ -568,6 +584,11 @@ async function run( 'Checkout GitHub repository routing requires the trusted-vm command policy', ); } + if (github.installationTokenScope && commandPolicy.preset !== 'trusted-vm') { + throw new Error( + 'Installation-scoped GitHub tokens require the trusted-vm command policy', + ); + } const githubDomains = github.provider ? github.host === 'github.com' ? [...GITHUB_ALLOWED_DOMAINS] diff --git a/packages/code/src/github.test.ts b/packages/code/src/github.test.ts index b13d70f7..5d401515 100644 --- a/packages/code/src/github.test.ts +++ b/packages/code/src/github.test.ts @@ -239,6 +239,272 @@ test('routes and scopes GitHub App tokens per repository installation', async (t ); }); +test('installation scope shares one token across repositories in an organization and refreshes grants', async (t) => { + const directory = await mkdtemp(join(tmpdir(), 'librechat-code-github-installation-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const privateKeyPath = join(directory, 'app.pem'); + const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + await writeFile( + privateKeyPath, + privateKey.export({ type: 'pkcs8', format: 'pem' }), + { mode: 0o600 }, + ); + let now = new Date('2030-01-01T00:00:00Z'); + const minted: Array<{ installation: string; body?: string }> = []; + const provider = new GitHubAppCredentialProvider({ + appId: '123', + tokenScope: 'installation', + privateKeyPath, + now: () => now, + fetch: (async (input, init) => { + const url = String(input); + if (url.endsWith('/app')) return Response.json({ slug: 'lia' }); + if (url.endsWith('/users/lia%5Bbot%5D')) { + return Response.json({ id: 1234, login: 'lia[bot]', type: 'Bot' }); + } + if (/\/repos\/LibreChat-AI\/(agents|LibreChat)\/installation$/.test(url)) { + return Response.json({ id: 111 }); + } + if (url.endsWith('/repos/ClickHouse/Agents/installation')) { + return Response.json({ id: 222 }); + } + const installation = /\/app\/installations\/(\d+)\/access_tokens$/.exec(url)?.[1]; + if (installation) { + minted.push({ + installation, + body: typeof init?.body === 'string' ? init.body : undefined, + }); + return Response.json({ + token: `ghs_${installation}_${minted.length}_abcdefghijklmnopqrstuvwxyz`, + expires_at: '2030-01-01T01:00:00Z', + }, { status: 201 }); + } + return Response.json({}, { status: 404 }); + }) as typeof fetch, + }); + const [agents, librechat] = await Promise.all([ + provider.getCredential(undefined, 'LibreChat-AI/agents'), + provider.getCredential(undefined, 'LibreChat-AI/LibreChat'), + ]); + assert.equal(agents.value, librechat.value); + assert.deepEqual(minted, [{ installation: '111', body: undefined }]); + const clickhouse = await provider.getCredential(undefined, 'ClickHouse/Agents'); + assert.notEqual(clickhouse.value, librechat.value); + assert.deepEqual(minted.map(entry => entry.installation), ['111', '222']); + now = new Date('2030-01-01T00:02:01Z'); + const refreshed = await provider.getCredential(undefined, 'LibreChat-AI/LibreChat'); + assert.notEqual(refreshed.value, librechat.value); + assert.deepEqual(minted.map(entry => entry.installation), ['111', '222', '111']); +}); + +test('installation scope shares a bounded lookup when a waiter cancels', async (t) => { + const directory = await mkdtemp(join(tmpdir(), 'librechat-code-github-shared-lookup-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const privateKeyPath = join(directory, 'app.pem'); + const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + await writeFile(privateKeyPath, privateKey.export({ type: 'pkcs8', format: 'pem' }), { + mode: 0o600, + }); + let releaseLookup!: (response: Response) => void; + const lookupResponse = new Promise(resolve => { releaseLookup = resolve; }); + let lookupStarted!: () => void; + const started = new Promise(resolve => { lookupStarted = resolve; }); + const cancelled = new AbortController(); + let lookups = 0; + let mints = 0; + const provider = new GitHubAppCredentialProvider({ + appId: '123', + tokenScope: 'installation', + privateKeyPath, + now: () => new Date('2030-01-01T00:00:00Z'), + fetch: (async (input, init) => { + const url = String(input); + if (url.endsWith('/repos/acme/project/installation')) { + lookups += 1; + assert.ok(init?.signal); + assert.notEqual(init.signal, cancelled.signal); + lookupStarted(); + return lookupResponse; + } + if (url.endsWith('/app/installations/111/access_tokens')) { + mints += 1; + return Response.json({ + token: 'ghs_shared_abcdefghijklmnopqrstuvwxyz', + expires_at: '2030-01-01T01:00:00Z', + }, { status: 201 }); + } + if (url.endsWith('/app')) return Response.json({ slug: 'lia' }); + if (url.endsWith('/users/lia%5Bbot%5D')) { + return Response.json({ id: 1234, login: 'lia[bot]', type: 'Bot' }); + } + return Response.json({}, { status: 404 }); + }) as typeof fetch, + }); + const first = provider.getCredential(cancelled.signal, 'acme/project'); + const others = Array.from({ length: 12 }, () => + provider.getCredential(undefined, 'acme/project')); + await started; + cancelled.abort(new Error('first command cancelled')); + await assert.rejects(first, /first command cancelled/); + releaseLookup(Response.json({ id: 111 })); + const credentials = await Promise.all(others); + assert.equal(lookups, 1); + assert.equal(mints, 1); + assert.equal(new Set(credentials.map(credential => credential.value)).size, 1); +}); + +test('installation scope times out a stalled lookup without a caller signal', async (t) => { + const directory = await mkdtemp(join(tmpdir(), 'librechat-code-github-lookup-timeout-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const privateKeyPath = join(directory, 'app.pem'); + const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + await writeFile(privateKeyPath, privateKey.export({ type: 'pkcs8', format: 'pem' }), { + mode: 0o600, + }); + const timeout = new AbortController(); + t.mock.method(AbortSignal, 'timeout', (ms: number) => { + assert.equal(ms, 30_000); + return timeout.signal; + }); + let lookupStarted!: () => void; + const started = new Promise(resolve => { lookupStarted = resolve; }); + const provider = new GitHubAppCredentialProvider({ + appId: '123', + tokenScope: 'installation', + privateKeyPath, + fetch: (async (input, init) => { + if (String(input).endsWith('/repos/acme/project/installation')) { + assert.equal(init?.signal, timeout.signal); + lookupStarted(); + return new Promise((_resolve, reject) => { + timeout.signal.addEventListener('abort', () => reject(timeout.signal.reason), { once: true }); + }); + } + return Response.json({}, { status: 404 }); + }) as typeof fetch, + }); + const pending = provider.getCredential(undefined, 'acme/project'); + await started; + timeout.abort(new DOMException('Installation lookup timed out', 'TimeoutError')); + await assert.rejects(pending, { name: 'TimeoutError' }); +}); + +test('installation replacement checks every shared waiter’s repository grant', async (t) => { + const directory = await mkdtemp(join(tmpdir(), 'librechat-code-github-replaced-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const privateKeyPath = join(directory, 'app.pem'); + const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + await writeFile(privateKeyPath, privateKey.export({ type: 'pkcs8', format: 'pem' }), { + mode: 0o600, + }); + let now = new Date('2030-01-01T00:00:00Z'); + let replaced = false; + let bLookups = 0; + let oldMints = 0; + let releaseOldMint!: () => void; + const oldMintResponse = new Promise(resolve => { releaseOldMint = resolve; }); + let oldMintStarted!: () => void; + const oldMintPending = new Promise(resolve => { oldMintStarted = resolve; }); + const provider = new GitHubAppCredentialProvider({ + appId: '123', + tokenScope: 'installation', + privateKeyPath, + now: () => now, + fetch: (async input => { + const url = String(input); + if (url.endsWith('/repos/acme/a/installation')) { + return Response.json({ id: replaced ? 222 : 111 }); + } + if (url.endsWith('/repos/acme/b/installation')) { + bLookups += 1; + return replaced ? Response.json({}, { status: 404 }) : Response.json({ id: 111 }); + } + if (url.endsWith('/app/installations/111/access_tokens')) { + oldMints += 1; + if (replaced) { + oldMintStarted(); + await oldMintResponse; + return Response.json({}, { status: 404 }); + } + return Response.json({ + token: 'ghs_old_abcdefghijklmnopqrstuvwxyz', + expires_at: '2030-01-01T00:06:00Z', + }, { status: 201 }); + } + if (url.endsWith('/app/installations/222/access_tokens')) { + return Response.json({ + token: 'ghs_new_abcdefghijklmnopqrstuvwxyz', + expires_at: '2030-01-01T01:00:00Z', + }, { status: 201 }); + } + if (url.endsWith('/app')) return Response.json({ slug: 'lia' }); + if (url.endsWith('/users/lia%5Bbot%5D')) { + return Response.json({ id: 1234, login: 'lia[bot]', type: 'Bot' }); + } + return Response.json({}, { status: 404 }); + }) as typeof fetch, + }); + const old = await provider.getCredential(undefined, 'acme/a'); + assert.equal((await provider.getCredential(undefined, 'acme/b')).value, old.value); + replaced = true; + now = new Date('2030-01-01T00:01:01Z'); // Token needs refresh; both mappings are still live. + const forA = provider.getCredential(undefined, 'acme/a'); + await oldMintPending; + const forB = provider.getCredential(undefined, 'acme/b'); + await new Promise(resolve => setImmediate(resolve)); // Join A's stale installation mint. + releaseOldMint(); + const [a, b] = await Promise.allSettled([forA, forB]); + assert.equal(a.status, 'fulfilled'); + if (a.status === 'fulfilled') assert.equal(a.value.value, 'ghs_new_abcdefghijklmnopqrstuvwxyz'); + assert.equal(b.status, 'rejected'); + if (b.status === 'rejected') assert.match(String(b.reason), /not installed for acme\/b/); + assert.equal(oldMints, 2); // One initial mint and just one shared failed refresh. + assert.equal(bLookups, 2); // B must revalidate instead of receiving A's new token. +}); + +test('installation scope follows a repository transfer after the lookup expires', async (t) => { + const directory = await mkdtemp(join(tmpdir(), 'librechat-code-github-transfer-')); + t.after(() => rm(directory, { recursive: true, force: true })); + const privateKeyPath = join(directory, 'app.pem'); + const { privateKey } = generateKeyPairSync('rsa', { modulusLength: 2048 }); + await writeFile(privateKeyPath, privateKey.export({ type: 'pkcs8', format: 'pem' }), { + mode: 0o600, + }); + let now = new Date('2030-01-01T00:00:00Z'); + let lookupCount = 0; + const provider = new GitHubAppCredentialProvider({ + appId: '123', + tokenScope: 'installation', + privateKeyPath, + now: () => now, + fetch: (async (input) => { + const url = String(input); + if (url.endsWith('/app')) return Response.json({ slug: 'lia' }); + if (url.endsWith('/users/lia%5Bbot%5D')) { + return Response.json({ id: 1234, login: 'lia[bot]', type: 'Bot' }); + } + if (url.endsWith('/repos/acme/project/installation')) { + lookupCount += 1; + return Response.json({ id: lookupCount === 1 ? 111 : 222 }); + } + const installation = /\/app\/installations\/(\d+)\/access_tokens$/.exec(url)?.[1]; + if (installation) { + return Response.json({ + token: `ghs_${installation}_abcdefghijklmnopqrstuvwxyz`, + expires_at: '2030-01-01T01:00:00Z', + }); + } + return Response.json({}, { status: 404 }); + }) as typeof fetch, + }); + assert.equal((await provider.getCredential(undefined, 'acme/project')).value, + 'ghs_111_abcdefghijklmnopqrstuvwxyz'); + now = new Date('2030-01-01T00:02:01Z'); + assert.equal((await provider.getCredential(undefined, 'acme/project')).value, + 'ghs_222_abcdefghijklmnopqrstuvwxyz'); + assert.equal(lookupCount, 2); +}); + test('keeps a shared token refresh alive when one waiter is cancelled', async (t) => { const directory = await mkdtemp(join(tmpdir(), 'librechat-code-github-cancel-')); t.after(() => rm(directory, { recursive: true, force: true })); diff --git a/packages/code/src/github.ts b/packages/code/src/github.ts index 19b8e639..8519938f 100644 --- a/packages/code/src/github.ts +++ b/packages/code/src/github.ts @@ -41,6 +41,8 @@ export interface GitHubAppCredentialProviderOptions { appId: string; /** Legacy fixed installation. Omit to resolve the installation per repository. */ installationId?: string; + /** Opt in to all repositories granted to the resolved installation. */ + tokenScope?: 'repository' | 'installation'; privateKeyPath: string; apiUrl?: string; /** Git HTTPS hostname; non-public hosts default to the GHES /api/v3 base. */ @@ -52,6 +54,13 @@ export interface GitHubAppCredentialProviderOptions { const execFileAsync = promisify(execFile); const GITHUB_SHARED_REQUEST_TIMEOUT_MS = 30_000; +const GITHUB_INSTALLATION_CACHE_MS = 2 * 60_000; + +class InstallationChangedError extends Error { + constructor(readonly installationId: string) { + super('GitHub App installation changed while issuing a token'); + } +} async function waitForShared( promise: Promise, @@ -248,8 +257,14 @@ function createAppJwt(appId: string, privateKey: string, now: Date): string { export class GitHubAppCredentialProvider implements GitHubCredentialProvider { private readonly cached = new Map(); + private readonly cachedAt = new Map(); private readonly inFlight = new Map>(); private readonly installationIds = new Map(); + private readonly installationIdCachedAt = new Map(); + private readonly installationIdInFlight = new Map< + string, + Promise<{ id: string; jwt: string }> + >(); private appLogin?: string; private appLoginInFlight?: Promise; private actor?: GitHubCredential['actor']; @@ -423,32 +438,80 @@ export class GitHubAppCredentialProvider implements GitHubCredentialProvider { private async resolveInstallationId( repository: string, - jwt: string, signal?: AbortSignal, - ): Promise { - if (this.options.installationId) return this.options.installationId; + jwt?: string, + ): Promise<{ id: string; jwt?: string }> { + if (this.options.installationId) { + return { id: this.options.installationId, jwt }; + } const cached = this.installationIds.get(repository); - if (cached) return cached; - const { owner, name } = repositoryName(repository); - const response = await this.request( - `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/installation`, - jwt, - signal, - ); - if (!response.ok) { - throw new Error( - response.status === 404 - ? `GitHub App is not installed for ${repository}` - : `GitHub App installation lookup failed with status ${response.status}`, + const now = (this.options.now ?? (() => new Date()))().getTime(); + if ( + cached && + now - (this.installationIdCachedAt.get(repository) ?? 0) < GITHUB_INSTALLATION_CACHE_MS + ) return { id: cached, jwt }; + const existing = this.installationIdInFlight.get(repository); + if (existing) return waitForShared(existing, signal); + const pending = (async () => { + const sharedSignal = AbortSignal.timeout(GITHUB_SHARED_REQUEST_TIMEOUT_MS); + const lookupJwt = jwt ?? await this.appJwt( + (this.options.now ?? (() => new Date()))(), ); + const { owner, name } = repositoryName(repository); + const response = await this.request( + `/repos/${encodeURIComponent(owner)}/${encodeURIComponent(name)}/installation`, + lookupJwt, + sharedSignal, + ); + if (!response.ok) { + throw new Error( + response.status === 404 + ? `GitHub App is not installed for ${repository}` + : `GitHub App installation lookup failed with status ${response.status}`, + ); + } + const body = (await response.json()) as { id?: unknown }; + if (!Number.isSafeInteger(body.id) || Number(body.id) <= 0) { + throw new Error('GitHub App installation response is invalid'); + } + const id = String(body.id); + this.installationIds.set(repository, id); + this.installationIdCachedAt.set( + repository, + (this.options.now ?? (() => new Date()))().getTime(), + ); + return { id, jwt: lookupJwt }; + })(); + this.installationIdInFlight.set(repository, pending); + const clearPending = () => { + if (this.installationIdInFlight.get(repository) === pending) { + this.installationIdInFlight.delete(repository); + } + }; + void pending.then(clearPending, clearPending); + return waitForShared(pending, signal); + } + + private async waitForCredential( + pending: Promise, + signal: AbortSignal | undefined, + repository: string | undefined, + retried: boolean, + ): Promise { + try { + return await waitForShared(pending, signal); + } catch (error) { + if (!(error instanceof InstallationChangedError) || retried || !repository) { + throw error; + } + // A shared mint can serve several repositories. Never pass its replacement + // installation's token to a repository whose grant has not been checked. + if (this.installationIds.get(repository) === error.installationId) { + this.installationIds.delete(repository); + this.installationIdCachedAt.delete(repository); + } + return this.getCredentialAttempt(signal, repository, true); } - const body = (await response.json()) as { id?: unknown }; - if (!Number.isSafeInteger(body.id) || Number(body.id) <= 0) { - throw new Error('GitHub App installation response is invalid'); - } - const installationId = String(body.id); - this.installationIds.set(repository, installationId); - return installationId; } async getCredential( @@ -462,32 +525,46 @@ export class GitHubAppCredentialProvider implements GitHubCredentialProvider { ); } if (repository) repositoryName(repository); + return this.getCredentialAttempt(signal, repository, false); + } + + private async getCredentialAttempt( + signal: AbortSignal | undefined, + repository: string | undefined, + retried: boolean, + ): Promise { + signal?.throwIfAborted(); + const installationScope = this.options.tokenScope === 'installation'; + const installation = installationScope + ? await this.resolveInstallationId(repository ?? '', signal) + : undefined; const now = (this.options.now ?? (() => new Date()))(); - const key = this.options.installationId ?? repository!; + const key = installation?.id ?? this.options.installationId ?? repository!; const cached = this.cached.get(key); if ( cached?.expiresAt != null && - cached.expiresAt.getTime() - now.getTime() > 5 * 60_000 + cached.expiresAt.getTime() - now.getTime() > 5 * 60_000 && + (!installationScope || now.getTime() - (this.cachedAt.get(key) ?? 0) < GITHUB_INSTALLATION_CACHE_MS) ) { return cached; } const existing = this.inFlight.get(key); - if (existing) return waitForShared(existing, signal); + if (existing) return this.waitForCredential(existing, signal, repository, retried); const pending = (async () => { const sharedSignal = AbortSignal.timeout( GITHUB_SHARED_REQUEST_TIMEOUT_MS, ); - const jwt = await this.appJwt(now); - const scopedRepository = repository + const jwt = installation?.jwt ?? await this.appJwt(now); + const scopedRepository = !installationScope && repository ? repositoryName(repository).name : undefined; - const installationId = await this.resolveInstallationId( + const resolvedInstallationId = installation?.id ?? (await this.resolveInstallationId( repository ?? '', - jwt, sharedSignal, - ); + jwt, + )).id; let response = await this.request( - `/app/installations/${installationId}/access_tokens`, + `/app/installations/${resolvedInstallationId}/access_tokens`, jwt, sharedSignal, { @@ -495,18 +572,20 @@ export class GitHubAppCredentialProvider implements GitHubCredentialProvider { headers: { 'Content-Type': 'application/json', }, - ...(this.options.installationId + ...(this.options.installationId || installationScope ? {} : { body: JSON.stringify({ repositories: [scopedRepository] }) }), }, ); if (!this.options.installationId && response.status === 404) { + if (installationScope) throw new InstallationChangedError(resolvedInstallationId); this.installationIds.delete(repository!); - const refreshedInstallationId = await this.resolveInstallationId( + this.installationIdCachedAt.delete(repository!); + const refreshedInstallationId = (await this.resolveInstallationId( repository!, - jwt, sharedSignal, - ); + jwt, + )).id; response = await this.request( `/app/installations/${refreshedInstallationId}/access_tokens`, jwt, @@ -516,7 +595,9 @@ export class GitHubAppCredentialProvider implements GitHubCredentialProvider { headers: { 'Content-Type': 'application/json', }, - body: JSON.stringify({ repositories: [scopedRepository] }), + ...(installationScope + ? {} + : { body: JSON.stringify({ repositories: [scopedRepository] }) }), }, ); } @@ -554,6 +635,10 @@ export class GitHubAppCredentialProvider implements GitHubCredentialProvider { actor, }; this.cached.set(key, credential); + this.cachedAt.set( + key, + (this.options.now ?? (() => new Date()))().getTime(), + ); return credential; })(); this.inFlight.set(key, pending); @@ -561,7 +646,7 @@ export class GitHubAppCredentialProvider implements GitHubCredentialProvider { if (this.inFlight.get(key) === pending) this.inFlight.delete(key); }; void pending.then(clearPending, clearPending); - return waitForShared(pending, signal); + return this.waitForCredential(pending, signal, repository, retried); } } From 67329888d66060b7e21416f539f6f0ddf56fff28 Mon Sep 17 00:00:00 2001 From: "lia-by-librechat[bot]" <328778573+lia-by-librechat[bot]@users.noreply.github.com> Date: Sun, 27 Sep 2026 15:19:05 -0400 Subject: [PATCH 2/2] =?UTF-8?q?=E2=8F=B3=20fix:=20Let=20BYOM=20Workspace?= =?UTF-8?q?=20Calls=20Queue=20Past=20Thirty=20Seconds=20(#264)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ⏳ fix: Let BYOM Workspace Calls Queue Past Thirty Seconds * fix: Recompute BYOM Reservation TTLs After Admission --------- Co-authored-by: Lia --- docs/byom-worker-admission.md | 48 +++++++++------ docs/remote-bridge/README.md | 6 ++ packages/code/README.md | 10 +++- service/src/bridge/concurrent-store.test.ts | 45 ++++++++++++++ service/src/bridge/store.ts | 14 +++-- service/src/bridge/worker-admission.test.ts | 66 +++++++++++++++++++++ service/src/workspace-tools/router.test.ts | 41 +++++++++---- service/src/workspace-tools/router.ts | 28 +++++---- 8 files changed, 212 insertions(+), 46 deletions(-) diff --git a/docs/byom-worker-admission.md b/docs/byom-worker-admission.md index 2c2b354b..2fcd9f9f 100644 --- a/docs/byom-worker-admission.md +++ b/docs/byom-worker-admission.md @@ -5,14 +5,17 @@ The limit is 32 admitted requests per worker, including the active request. When the limit is reached, the workspace endpoint returns HTTP 429 with `WORKER_QUEUE_FULL`. A different worker has an independent admission queue. -The workspace HTTP endpoint allows at most 30 seconds for admission. After -admission and worker validation, a separate execution deadline starts. Commands -receive their requested timeout (30 seconds by default, up to five minutes), -capped by the operator's `JOB_TIMEOUT`, plus five seconds to settle the result. -Read/search/list operations receive up to 30 seconds, also capped by `JOB_TIMEOUT`. -Disconnecting or cancelling removes the waiting -request without cancelling the active assignment. Expired entries are pruned; -Redis key expiry also bounds state left by a crashed API process. +The Code API workspace HTTP endpoint allows up to the smaller of `JOB_TIMEOUT` +and five minutes for admission while the caller stays connected. After admission +and worker validation, a separate execution deadline starts. Commands receive +their requested timeout (30 seconds by default, up to five minutes), capped by +the operator's `JOB_TIMEOUT`, plus five seconds to settle the result. Other +operations receive up to 30 seconds, also capped by `JOB_TIMEOUT`. +Disconnecting or cancelling removes a waiting request without cancelling the +active assignment. Expired entries are pruned; Redis key expiry also bounds +state left by a crashed API process. Reservations derive their TTL at acquisition +from the remaining absolute deadline or a fresh execution budget; enqueued +assignment records use the final execution deadline, not the elapsed queue budget. After admission, the API revalidates the worker incarnation, identity, tenant binding and workspace operation. A waiting request cannot migrate to a replacement @@ -26,13 +29,24 @@ Existing workers still execute one assignment at a time. Parallel execution acro workspaces requires separate lease claims and isolated native sandbox contexts; this admission change does not advertise that capability. -LibreChat must allow queue time plus execution/settlement time and five seconds -for HTTP delivery: 65 seconds for reads, 70 seconds for default commands, and -340 seconds for five-minute commands. Either side can be upgraded first. Older -clients still cancel at their earlier deadline; newer clients preserve errors from -older servers without retrying mutations. Both updates are needed for the full -waiting budget. Any reverse proxy request timeout must accommodate these totals. -The worker package does not need an update for the deadline change. +Clients and reverse proxies must allow queue time plus execution/settlement time +and five seconds for HTTP delivery. With the default five-minute `JOB_TIMEOUT`, +that is at least 335 seconds for non-command tools, 340 seconds for default +commands, and 610 seconds for five-minute commands. With a smaller `JOB_TIMEOUT`, +use `min(JOB_TIMEOUT, 300s)` for the queue, plus `min(JOB_TIMEOUT, 30s)` for other +operations or `min(JOB_TIMEOUT, requested command timeout) + 5s` for commands, +plus five seconds for delivery. -Focused regression coverage lives in `service/src/bridge/admission.test.ts` and -`service/src/bridge/worker-admission.test.ts`. +At the time of this change, LibreChat's `getWorkspaceToolTimeoutMs` still budgets +only 30 seconds for a single admission attempt (65/70/340 seconds in total). +Its `maxQueueWaitMs` is a retry horizon after a typed capacity rejection, **not** +a per-attempt HTTP timeout. Updating Code API alone therefore does not guarantee +the full wait. An earlier client, tool, or proxy timeout disconnects the request; +if work was already admitted, a mutation may have run and must not be blindly +retried. Match LibreChat's per-attempt timeout and each intermediary to the new +budget before relying on it. Existing workers do not need an update. + +Focused regression coverage lives in `service/src/bridge/admission.test.ts`, +`service/src/bridge/worker-admission.test.ts`, +`service/src/bridge/concurrent-store.test.ts`, and +`service/src/workspace-tools/router.test.ts`. diff --git a/docs/remote-bridge/README.md b/docs/remote-bridge/README.md index efe84b84..c5bc573c 100644 --- a/docs/remote-bridge/README.md +++ b/docs/remote-bridge/README.md @@ -239,6 +239,12 @@ execution. worker. The lower API or worker slot ceiling wins, and assignments sharing the same workspace isolation key remain serialized while independent conversation worktrees may run concurrently. +- Workspace tool admission waits for capacity up to the smaller of `JOB_TIMEOUT` + and five minutes while the HTTP caller remains connected. Disconnects cancel + waiting, and admitted work receives a separate execution budget. A shorter + client or proxy timeout can end the wait sooner; Code API does not receive an + absolute caller deadline. See [BYOM worker admission](../byom-worker-admission.md) + for the caller and proxy timeout requirements. - Dynamic workers are fenced to their server-issued tenant before assignment. - Each assignment has an absolute deadline, generation, and random lease token. - Settlements with the wrong worker, generation, token, or expired deadline are diff --git a/packages/code/README.md b/packages/code/README.md index a87c87df..87a6fe61 100644 --- a/packages/code/README.md +++ b/packages/code/README.md @@ -791,11 +791,17 @@ Legacy requests without a conversation identity continue to use the selected source root. Older Code API deployments do not negotiate the capability, so the worker omits it until every request path understands the isolation boundary. -Admission waits at most 30 seconds. A `WORKSPACE_QUEUE_TIMEOUT` response (HTTP -503, `Retry-After: 1`) means the operation was not assigned or started; wait for +On an updated Code API, admission waits up to the smaller of `JOB_TIMEOUT` and +five minutes while the HTTP caller remains connected; older Code API versions +waited at most 30 seconds. A `WORKSPACE_QUEUE_TIMEOUT` response (HTTP 503, +`Retry-After: 1`) means the operation was not assigned or started; wait for capacity before submitting it again. This is distinct from `ASSIGNMENT_EXPIRED` or a transport timeout after dispatch, where execution may have occurred and mutations must not be blindly retried. No automatic retry is added by this policy. +Align the client's per-attempt timeout and any proxy with the queue **plus** +execution budget before relying on the longer wait. See the +[BYOM worker admission guide](../../docs/byom-worker-admission.md) for the +current client limitation and the timeout calculations. Keep the existing URL, pairing/identity, and network policy configuration. The primary root keeps its configured workspace ID (default `primary`). Repeat diff --git a/service/src/bridge/concurrent-store.test.ts b/service/src/bridge/concurrent-store.test.ts index 33f3d082..6bdfde88 100644 --- a/service/src/bridge/concurrent-store.test.ts +++ b/service/src/bridge/concurrent-store.test.ts @@ -357,6 +357,51 @@ test('same-root work waits while another root progresses', async () => { await Promise.all([nextA, b]); }); +test('a long same-root queue allowance does not extend slot or assignment TTLs', async () => { + await register(); + const active = dispatch('a'); + const activeAssignment = await store.lease( + workerId, incarnationId, 1000, undefined, undefined, 0, + ); + const controller = new AbortController(); + const queued = store.dispatchWorkspaceTool({ + workerId, + signal: controller.signal, + deadlineAtMs: Date.now() + 300_000, + executionTimeoutMs: 305_000, + request: { protocolVersion: 1, operation: 'read_file', workspaceId: 'a', path: 'second.txt' }, + }); + void queued.catch(() => undefined); + await new Promise((resolve) => setTimeout(resolve, 150)); + await settle(activeAssignment!); + await active; + + const assignment = await store.lease( + workerId, incarnationId, 1000, undefined, undefined, 0, + ); + if (assignment == null) { + controller.abort(); + await queued.catch(() => undefined); + throw new Error('Queued request was not leased'); + } + expect(assignment.request).toMatchObject({ path: 'second.txt' }); + try { + const expiresAtMs = Date.parse(assignment.expiresAt); + const slotExpiresAtMs = Number(await redis.hget( + `codeapi:bridge:v1:worker:${workerId}:workspace-slots`, 'e:0', + )); + expect(slotExpiresAtMs).toBeGreaterThan(expiresAtMs + 28_000); + expect(slotExpiresAtMs).toBeLessThan(expiresAtMs + 32_000); + const assignmentTtlMs = await redis.pttl(`codeapi:bridge:v1:assignment:${assignment.assignmentId}`); + const remainingMs = expiresAtMs - Date.now(); + expect(assignmentTtlMs).toBeGreaterThan(remainingMs + 28_000); + expect(assignmentTtlMs).toBeLessThan(remainingMs + 32_000); + } finally { + await settle(assignment); + await queued; + } +}); + test('conversation worktrees on one source use independent capacity lanes', async () => { await register(); const firstId = 'a'.repeat(64); diff --git a/service/src/bridge/store.ts b/service/src/bridge/store.ts index 2ffeec66..3d9d7e1c 100644 --- a/service/src/bridge/store.ts +++ b/service/src/bridge/store.ts @@ -863,10 +863,6 @@ export class RedisBridgeStore { const assignmentId = randomBytes(18).toString('base64url'); const leaseToken = randomBytes(32).toString('base64url'); - // The lock is acquired before admission finishes; it must outlive the later execution deadline. - const ttlSeconds = assignmentTtlSeconds( - args.deadlineAtMs + (args.executionTimeoutMs ?? 0), - ); const lockIncarnationId = registration.incarnationId; let assignment: StoredAssignment | undefined; let enqueueAttempted = false; @@ -931,6 +927,14 @@ export class RedisBridgeStore { ); continue; } + // A queued request may have waited nearly its full admission budget. + // Reserve for the *remaining* absolute deadline or a fresh execution + // budget, not the original queue window plus execution again. + const ttlSeconds = assignmentTtlSeconds( + args.executionTimeoutMs === undefined + ? args.deadlineAtMs + : Date.now() + args.executionTimeoutMs, + ); if (workspaceSlots != null) { workspaceLeaseSlot = await this.dispatchCommand( () => @@ -1070,7 +1074,7 @@ export class RedisBridgeStore { enqueueAttempted = true; return this.enqueueForActiveIncarnation( assignment!, - ttlSeconds, + assignmentTtlSeconds(Date.parse(assignment!.expiresAt)), readyToken, ); }, diff --git a/service/src/bridge/worker-admission.test.ts b/service/src/bridge/worker-admission.test.ts index 655748b1..98363c34 100644 --- a/service/src/bridge/worker-admission.test.ts +++ b/service/src/bridge/worker-admission.test.ts @@ -161,6 +161,72 @@ test('execution receives a fresh budget after waiting and the lock covers long c await second; }); +test('a long queue allowance does not extend serial lock or assignment TTLs after admission', async () => { + await register(); + const active = dispatch('first'); + const activeAssignment = await store.lease(workerId, incarnationId, 1000); + const controller = new AbortController(); + const queued = dispatch('second', controller, 300_000, 305_000); + void queued.catch(() => undefined); + await new Promise((resolve) => setTimeout(resolve, 150)); + await settle(activeAssignment); + await active; + + const assignment = await store.lease(workerId, incarnationId, 1000); + if (assignment == null) { + controller.abort(); + await queued.catch(() => undefined); + throw new Error('Queued request was not leased'); + } + expect(assignment.request).toMatchObject({ path: 'second' }); + try { + const expiresAtMs = Date.parse(assignment.expiresAt); + for (const key of [ + `codeapi:bridge:v1:worker:${workerId}:lock`, + `codeapi:bridge:v1:worker:${workerId}:lock:incarnation`, + `codeapi:bridge:v1:assignment:${assignment.assignmentId}`, + ]) { + const ttlMs = await redis.pttl(key); + const remainingMs = expiresAtMs - Date.now(); + expect(ttlMs).toBeGreaterThan(remainingMs + 28_000); + expect(ttlMs).toBeLessThan(remainingMs + 32_000); + } + } finally { + await settle(assignment); + await queued; + } +}); + +test('absolute-deadline callers keep only their remaining deadline in the serial lock TTL', async () => { + await register(); + const active = dispatch('first'); + const activeAssignment = await store.lease(workerId, incarnationId, 1000); + const controller = new AbortController(); + const queued = dispatch('absolute', controller, 2_500); + void queued.catch(() => undefined); + await new Promise((resolve) => setTimeout(resolve, 1450)); + await settle(activeAssignment); + await active; + + const assignment = await store.lease(workerId, incarnationId, 1000); + if (assignment == null) { + controller.abort(); + await queued.catch(() => undefined); + throw new Error('Queued request was not leased'); + } + expect(assignment.request).toMatchObject({ path: 'absolute' }); + try { + const remainingMs = Date.parse(assignment.expiresAt) - Date.now(); + expect(remainingMs).toBeGreaterThan(0); + const ttlMs = await redis.pttl(`codeapi:bridge:v1:worker:${workerId}:lock`); + expect(ttlMs).toBeGreaterThan(remainingMs + 28_000); + expect(ttlMs).toBeLessThan(remainingMs + 31_000); + } finally { + await settle(assignment); + await queued; + } +}); + test('execution expires independently of an unused queue allowance', async () => { await register(); const completion = dispatch('short', new AbortController(), 5000, 150); diff --git a/service/src/workspace-tools/router.test.ts b/service/src/workspace-tools/router.test.ts index a52b3f0b..18d24a76 100644 --- a/service/src/workspace-tools/router.test.ts +++ b/service/src/workspace-tools/router.test.ts @@ -67,13 +67,16 @@ test('binds instance admission to the authenticated tenant and user while preser expect(dispatched[1]?.workspaceInstanceId).toBeUndefined(); }); -test.each<[WorkspaceToolRequest, number, number?]>([ - [{ protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path: 'README.md' }, 30_000, undefined], - [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 35_000, undefined], - [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready', timeoutMs: 300_000 }, 305_000, undefined], - [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 6000, 1000], - [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 35_000, 600_000], -])('separates the admission deadline from execution budget for %j', async (request, expectedExecution, ceiling) => { +test.each<[WorkspaceToolRequest, number, number?, number?]>([ + [{ protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path: 'README.md' }, 30_000, undefined, undefined], + [{ protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path: 'README.md' }, 30_000, 125_000, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 35_000, undefined, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready', timeoutMs: 90_000 }, 95_000, 125_000, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready', timeoutMs: 300_000 }, 305_000, undefined, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 6000, 1000, undefined], + [{ protocolVersion: 1, operation: 'execute_command', workspaceId: 'primary', command: 'echo ready' }, 35_000, 600_000, undefined], + [{ protocolVersion: 1, operation: 'read_file', workspaceId: 'primary', path: 'README.md' }, 30_000, 125_000, 5000], +])('separates the admission deadline from execution budget for %j', async (request, expectedExecution, ceiling, queueTimeoutMs) => { const app = express(); app.use(json()); app.use((req, _res, next) => { @@ -86,6 +89,7 @@ test.each<[WorkspaceToolRequest, number, number?]>([ app.use(createWorkspaceToolsRouter({ backend: 'remote-bridge', configuredWorkerId: 'user-worker', dynamicWorkers: false, timeoutMs: ceiling, + queueTimeoutMs, store: { async dispatchWorkspaceTool(args) { executionBudget = args.executionTimeoutMs; if (args.request.operation === 'execute_command') commandTimeout = args.request.timeoutMs; @@ -103,8 +107,19 @@ test.each<[WorkspaceToolRequest, number, number?]>([ await response.json(); expect(executionBudget).toBe(expectedExecution); if (request.operation === 'execute_command') expect(commandTimeout).toBe(expectedExecution - 5000); - expect(queueRemaining).toBeGreaterThan(29_000); - expect(queueRemaining).toBeLessThanOrEqual(30_000); + const expectedQueueBudget = queueTimeoutMs ?? Math.min(ceiling ?? 30_000, 300_000); + expect(queueRemaining).toBeGreaterThan(expectedQueueBudget - 1000); + expect(queueRemaining).toBeLessThanOrEqual(expectedQueueBudget); +}); + +test.each([0, 300_001, Number.POSITIVE_INFINITY])('rejects an unbounded queue override (%s)', (queueTimeoutMs) => { + expect(() => createWorkspaceToolsRouter({ + backend: 'remote-bridge', + configuredWorkerId: 'user-worker', + dynamicWorkers: false, + queueTimeoutMs, + store: { async dispatchWorkspaceTool() { throw new Error('must not dispatch'); } }, + })).toThrow('Workspace queue timeout must be between 1 and 300000 milliseconds'); }); test('rejects new workspace dispatches while the service is shutting down', async () => { @@ -404,7 +419,7 @@ test.each([ status: expectedStatus, errorCode, outcome: 'completed', - deadlineBudgetMs: 60_000, + deadlineBudgetMs: 330_000, dispatchDurationMs: expect.any(Number), }), ); @@ -486,6 +501,7 @@ test('logs a disconnected dispatch once without inventing HTTP 200', async () => const closed = Promise.withResolvers(); const settlementGate = Promise.withResolvers(); let dispatchAborted = false; + let queueRemaining: number | undefined; let closeConnection = (): void => { throw new Error('connection not ready'); }; app.use(json()); app.use((req, res, next) => { @@ -499,8 +515,10 @@ test('logs a disconnected dispatch once without inventing HTTP 200', async () => backend: 'remote-bridge', configuredWorkerId: 'user-worker', dynamicWorkers: true, + timeoutMs: 125_000, store: { - async dispatchWorkspaceTool({ signal }) { + async dispatchWorkspaceTool({ deadlineAtMs, signal }) { + queueRemaining = deadlineAtMs - Date.now(); started.resolve(); return await new Promise((_resolve, reject) => { signal.addEventListener( @@ -531,6 +549,7 @@ test('logs a disconnected dispatch once without inventing HTTP 200', async () => closeConnection(); await expect(response).rejects.toThrow(); await closed.promise; + expect(queueRemaining).toBeGreaterThan(120_000); expect(dispatchAborted).toBe(true); expect(logSpy).not.toHaveBeenCalled(); settlementGate.resolve(); diff --git a/service/src/workspace-tools/router.ts b/service/src/workspace-tools/router.ts index 3e1f9fee..51e47f26 100644 --- a/service/src/workspace-tools/router.ts +++ b/service/src/workspace-tools/router.ts @@ -20,6 +20,8 @@ import { } from '../bridge/selection'; import { principalWorkspaceInstanceId } from '../bridge/workspace-instance'; +const MAX_WORKSPACE_QUEUE_WAIT_MS = 5 * 60_000; + interface WorkspaceToolsRouterOptions { store: Pick; backend: 'http' | 'lambda-microvm' | 'remote-bridge'; @@ -50,15 +52,19 @@ export function bridgeStoreStatus(error: BridgeStoreError): number { } export function createWorkspaceToolsRouter(options: WorkspaceToolsRouterOptions): Router { - const queueBudgetMs = options.queueTimeoutMs ?? 30_000; - if (!Number.isSafeInteger(queueBudgetMs) || queueBudgetMs < 1 || queueBudgetMs > 30_000) { - throw new RangeError('Workspace queue timeout must be between 1 and 30000 milliseconds'); - } if (options.timeoutMs !== undefined && ( !Number.isSafeInteger(options.timeoutMs) || options.timeoutMs < 1 )) { throw new RangeError('Workspace execution timeout must be a positive safe integer'); } + // The HTTP disconnect cancels waiting; this bounds admission while the caller remains connected. + const queueBudgetMs = options.queueTimeoutMs ?? Math.min( + options.timeoutMs ?? 30_000, + MAX_WORKSPACE_QUEUE_WAIT_MS, + ); + if (!Number.isSafeInteger(queueBudgetMs) || queueBudgetMs < 1 || queueBudgetMs > MAX_WORKSPACE_QUEUE_WAIT_MS) { + throw new RangeError('Workspace queue timeout must be between 1 and 300000 milliseconds'); + } const router = Router(); router.post( @@ -86,13 +92,13 @@ export function createWorkspaceToolsRouter(options: WorkspaceToolsRouterOptions) const principalRequest: WorkspaceToolRequest = req.body.workspaceInstanceId == null ? req.body : { - ...req.body, - workspaceInstanceId: principalWorkspaceInstanceId({ - instanceId: req.body.workspaceInstanceId, - tenantId: principal.tenantId, - principalId: principal.userId, - }), - }; + ...req.body, + workspaceInstanceId: principalWorkspaceInstanceId({ + instanceId: req.body.workspaceInstanceId, + tenantId: principal.tenantId, + principalId: principal.userId, + }), + }; const request: WorkspaceToolRequest = principalRequest.operation === 'execute_command' ? { ...principalRequest, timeoutMs: Math.min( principalRequest.timeoutMs ?? BRIDGE_WORKSPACE_COMMAND_DEFAULT_TIMEOUT_MS,