Skip to content

fix(deploy): bound integration status requests #314

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
khaliqgant merged 3 commits into from
Aug 19, 2026
+186 −19
Merged

fix(deploy): bound integration status requests #314

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
7ea8d2e
fix(deploy): bound integration status requests
Aug 18, 2026
55b2e56
fix(deploy): keep timeout through response parsing
Aug 18, 2026
cdb2fa4
test(deploy): preserve signal-ignoring timeout coverage
Aug 18, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
137 changes: 137 additions & 0 deletions packages/deploy/src/integrations-list.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,143 @@ test('listIntegrations throws loud endpoint errors while authenticated', async (
);
});

test('listIntegrations bounds a status endpoint that never settles', async () => {
const startedAt = Date.now();
const operation = listIntegrations({
workspaceId: 'ws-1',
token: 'tok',
requestTimeoutMs: 20,
client: {
async fetch(pathname) {
if (pathname === '/api/v1/integrations/catalog') {
return json({ providers: [{ id: 'daytona' }] });
}
if (pathname === '/api/v1/me/integrations') {
return json({ integrations: [] });
}
if (pathname === '/api/v1/workspaces/ws-1/integrations') {
return json({ integrations: [] });
}
if (pathname.endsWith('/status?scope=deployer_user')) {
return json({ provider: 'daytona', status: 'connected' });
}
return new Promise<Response>(() => {});
}
}
});
let guardTimer: ReturnType<typeof setTimeout> | undefined;
const guard = new Promise<never>((_, reject) => {
guardTimer = setTimeout(
() => reject(new Error('test guard: integration request never settled')),
500
);
});

try {
await assert.rejects(
Promise.race([operation, guard]),
(err) => {
assert.ok(err instanceof IntegrationsListError);
assert.equal(err.status, 408);
assert.equal(
err.endpoint,
'/api/v1/workspaces/ws-1/integrations/daytona/status?scope=workspace'
);
assert.match(err.message, /timed out after 20ms/);
return true;
}
);
} finally {
if (guardTimer) clearTimeout(guardTimer);
}
assert.ok(Date.now() - startedAt < 500);
Comment thread
coderabbitai[bot] marked this conversation as resolved.
});

test('listIntegrations preserves its typed timeout when an abort-aware client rejects', async () => {
let guardTimer: ReturnType<typeof setTimeout> | undefined;
const operation = listIntegrations({
workspaceId: 'ws-1',
token: 'tok',
requestTimeoutMs: 20,
client: {
async fetch(_pathname, init = {}) {
return new Promise<Response>((_, reject) => {
init.signal?.addEventListener(
'abort',
() => reject(new Error('client abort rejection')),
{ once: true }
);
});
}
}
});
const guard = new Promise<never>((_, reject) => {
guardTimer = setTimeout(
() => reject(new Error('test guard: abort-aware request never settled')),
500
);
});

try {
await assert.rejects(
Promise.race([operation, guard]),
(err) => {
assert.ok(err instanceof IntegrationsListError);
assert.equal(err.status, 408);
assert.equal(err.endpoint, '/api/v1/integrations/catalog');
assert.match(err.message, /timed out after 20ms/);
return true;
}
);
} finally {
if (guardTimer) clearTimeout(guardTimer);
}
});

test('listIntegrations bounds response body consumption and preserves the typed timeout', async () => {
let guardTimer: ReturnType<typeof setTimeout> | undefined;
const operation = listIntegrations({
workspaceId: 'ws-1',
token: 'tok',
requestTimeoutMs: 20,
client: {
async fetch(pathname) {
if (pathname === '/api/v1/integrations/catalog') {
return json({ providers: [{ id: 'daytona' }] });
}
if (pathname === '/api/v1/me/integrations') {
return new Response(new ReadableStream({ start() {} }), {
status: 200,
headers: { 'content-type': 'application/json' }
});
}
return json({ integrations: [] });
}
}
});
const guard = new Promise<never>((_, reject) => {
guardTimer = setTimeout(
() => reject(new Error('test guard: response body never settled')),
500
);
});

try {
await assert.rejects(
Promise.race([operation, guard]),
(err) => {
assert.ok(err instanceof IntegrationsListError);
assert.equal(err.status, 408);
assert.equal(err.endpoint, '/api/v1/me/integrations');
assert.match(err.message, /timed out after 20ms/);
return true;
}
);
} finally {
if (guardTimer) clearTimeout(guardTimer);
}
});

test('listIntegrations accepts adapter slug as provider filter and suggests it on unknown providers', async () => {
const base = {
activeWorkspace: null,
Expand Down
68 changes: 49 additions & 19 deletions packages/deploy/src/integrations-list.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,8 @@ export interface ListIntegrationsOptions {
resolveWorkspaceToken?: typeof resolveWorkspaceToken;
provider?: string;
includeTriggers?: boolean;
/** Maximum time for any single Cloud catalog/status request. */
requestTimeoutMs?: number;
}

export class IntegrationsListError extends Error {
Expand Down Expand Up @@ -386,26 +388,54 @@ async function requestJson(
pathname: string,
init: RequestInit = {}
): Promise<unknown> {
const response = options.client
? await options.client.fetch(pathname, init)
: await (options.fetch ?? fetch)(`${cloudUrl}${pathname}`, {
...init,
headers: {
accept: 'application/json',
'content-type': 'application/json',
...(auth.token ? { authorization: `Bearer ${auth.token}` } : {}),
...(init.headers ?? {})
}
});
if (!response.ok) {
const body = await response.text().catch(() => '');
const excerpt = body.length > 400 ? `${body.slice(0, 400)}...` : body;
throw new IntegrationsListError(
`integration catalog/status request failed: ${response.status} ${pathname}${excerpt ? ` ${excerpt}` : ''}`,
{ status: response.status, endpoint: pathname, body: excerpt }
);
const timeoutMs = options.requestTimeoutMs ?? 10_000;
const controller = new AbortController();
const upstreamSignal = init.signal;
const abortFromUpstream = () => controller.abort(upstreamSignal?.reason);
if (upstreamSignal?.aborted) abortFromUpstream();
else upstreamSignal?.addEventListener('abort', abortFromUpstream, { once: true });

let timeout: ReturnType<typeof setTimeout> | undefined;
const timeoutError = new IntegrationsListError(
`integration catalog/status request timed out after ${timeoutMs}ms: ${pathname}`,
{ status: 408, endpoint: pathname, body: '' }
);
const timeoutPromise = new Promise<never>((_, reject) => {
timeout = setTimeout(() => {
reject(timeoutError);
controller.abort(timeoutError);
}, timeoutMs);
});

try {
const request = (async (): Promise<unknown> => {
const response = options.client
? await options.client.fetch(pathname, { ...init, signal: controller.signal })
: await (options.fetch ?? fetch)(`${cloudUrl}${pathname}`, {
...init,
signal: controller.signal,
headers: {
accept: 'application/json',
'content-type': 'application/json',
...(auth.token ? { authorization: `Bearer ${auth.token}` } : {}),
...(init.headers ?? {})
}
});
if (!response.ok) {
const body = await response.text().catch(() => '');
const excerpt = body.length > 400 ? `${body.slice(0, 400)}...` : body;
throw new IntegrationsListError(
`integration catalog/status request failed: ${response.status} ${pathname}${excerpt ? ` ${excerpt}` : ''}`,
{ status: response.status, endpoint: pathname, body: excerpt }
);
}
return await response.json();
})();
return await Promise.race([request, timeoutPromise]);
} finally {
if (timeout) clearTimeout(timeout);

@chatgpt-codex-connector chatgpt-codex-connector Bot Aug 18, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Keep the timeout active while consuming the response body

When an endpoint sends response headers but then stalls before completing the body, fetch resolves, this finally immediately clears the timeout, and the subsequent response.text() or response.json() can remain pending indefinitely. This leaves agentworkforce integrations vulnerable to the same hang the change is intended to prevent; keep the timer and abort signal active until body consumption finishes, or race the entire fetch-and-parse operation against the timeout.

Useful? React with 👍 / 👎.

@khaliqgant khaliqgant Aug 19, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Audited at HEAD cdb2fa47 — valid finding, and it is already fixed (in 55b2e56e).

You are right about the code you reviewed. This comment is anchored to original_commit_id 7ea8d2e7, where the finally { clearTimeout(timeout) } closed immediately after Promise.race([request, timeoutPromise]) resolved the headers, and response.json() / the error-path response.text() then ran outside the race. A response whose body stream never closes left listIntegrations pending forever — i.e. the fix for a hang still contained a hang path. GitHub re-anchored the comment onto cdb2fa47, which is why it still reads as open.

What 55b2e56e changed (packages/deploy/src/integrations-list.ts:409-441 at HEAD): the whole response-processing path is now one inner async IIFE, and that is what gets raced:

const request = (async (): Promise<unknown> => {
  const response = options.client ? await options.client.fetch(...) : await (options.fetch ?? fetch)(...);
  if (!response.ok) {
    const body = await response.text().catch(() => '');   // <- now inside the race
    throw new IntegrationsListError(...);
  }
  return await response.json();                            // <- now inside the race
})();
return await Promise.race([request, timeoutPromise]);
} finally {
  if (timeout) clearTimeout(timeout);
  upstreamSignal?.removeEventListener('abort', abortFromUpstream);
}

Both cleanups you asked about — the timer and the upstream abort-listener removal — are in a finally that now wraps the entire fetch-and-parse operation, so they run only once the race settles, not when headers arrive.

Regression test + non-vacuity proof. listIntegrations bounds response body consumption and preserves the typed timeout (integrations-list.test.ts:342) serves /api/v1/me/integrations as new Response(new ReadableStream({ start() {} })) — headers sent, body never closes — and asserts a typed 408 for that endpoint with /timed out after 20ms/.

I proved it is not vacuous rather than assuming it. I reverted only integrations-list.ts to its 7ea8d2e7 shape, kept the HEAD test file, rebuilt, and re-ran:

not ok 6 - listIntegrations bounds response body consumption and preserves the typed timeout
  duration_ms: 506.063333
  error: The expression evaluated to a falsy value:
    assert.ok(err instanceof IntegrationsListError)

The 506 ms duration is the tell: the 500 ms test guard fired because listIntegrations never settled — exactly the hang you described — and the guard's plain Error is not an IntegrationsListError. With the 55b2e56e source restored, all 7 tests in the file pass and the case settles in ~22 ms. Full packages/deploy suite: 256/256.

No code change needed at HEAD cdb2fa47. Thanks — this was the important one.

upstreamSignal?.removeEventListener('abort', abortFromUpstream);
}
return await response.json();
}

function adapterSlugForCloudProvider(provider: string): string {
Expand Down
Loading