Skip to content

test(control-plane): stabilize cancel integration test #643

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
ColeMurray merged 1 commit into from
May 17, 2026
Merged

test(control-plane): stabilize cancel integration test #643

Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
55 changes: 54 additions & 1 deletion packages/control-plane/test/integration/do-internal-routes.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,60 @@
import { describe, it, expect } from "vitest";
import { afterEach, beforeEach, describe, it, expect, vi } from "vitest";
import { env } from "cloudflare:test";
import { initSession, queryDO, seedEvents } from "./helpers";
import type { SpawnContext, ChildSessionDetail } from "@open-inspect/shared";

const originalFetch = globalThis.fetch;

function installModalFetchMock(): void {
vi.stubGlobal(
"fetch",
vi.fn(async (input: RequestInfo | URL, init?: RequestInit) => {
const url =
typeof input === "string" ? input : input instanceof URL ? input.toString() : input.url;

if (!url.includes(".modal.run")) {
return originalFetch(input, init);
}

const body = JSON.parse(String(init?.body ?? "{}")) as { sandbox_id?: string };
const sandboxId = body.sandbox_id ?? "sandbox-test";

return Response.json({
success: true,
data: {
sandbox_id: sandboxId,
modal_object_id: `modal-${sandboxId}`,
status: "running",
created_at: Date.now(),
},
});
})
);
}

async function waitForSandboxSpawn(stub: DurableObjectStub): Promise<void> {
const deadline = Date.now() + 1000;

while (Date.now() < deadline) {
const rows = await queryDO<{ status: string }>(stub, "SELECT status FROM sandbox LIMIT 1");
if (rows[0]?.status === "connecting") {
return;
}
await new Promise((resolve) => setTimeout(resolve, 10));
}

throw new Error("Timed out waiting for sandbox spawn");
}
Comment on lines +35 to +47
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 15, 2026

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Extract timeout literals to named constants with unit suffixes.

Lines 36 and 43 use literal timeout values (1000 and 10). As per coding guidelines, TypeScript timeout values should be defined as named constants with Ms suffix and defined once rather than restated as literals.

⏱️ Proposed fix to extract timeout constants

Add constants at the top of the file (after imports):

+const SANDBOX_SPAWN_TIMEOUT_MS = 1000;
+const SANDBOX_SPAWN_POLL_INTERVAL_MS = 10;
+
 const originalFetch = globalThis.fetch;

Then update the function:

 async function waitForSandboxSpawn(stub: DurableObjectStub): Promise<void> {
-  const deadline = Date.now() + 1000;
+  const deadline = Date.now() + SANDBOX_SPAWN_TIMEOUT_MS;
 
   while (Date.now() < deadline) {
     const rows = await queryDO<{ status: string }>(stub, "SELECT status FROM sandbox LIMIT 1");
     if (rows[0]?.status === "connecting") {
       return;
     }
-    await new Promise((resolve) => setTimeout(resolve, 10));
+    await new Promise((resolve) => setTimeout(resolve, SANDBOX_SPAWN_POLL_INTERVAL_MS));
   }
 
   throw new Error("Timed out waiting for sandbox spawn");
 }

As per coding guidelines: Use seconds for Python timeouts and milliseconds for TypeScript timeouts, encoding the unit in variable names (TypeScript: timeoutMs, TIMEOUT_MS). Define each default timeout value exactly once as a named constant and import it everywhere rather than restating literal values.

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/control-plane/test/integration/do-internal-routes.test.ts` around
lines 35 - 47, Extract the magic timeout literals in waitForSandboxSpawn into
named constants (e.g., SANDBOX_SPAWN_TIMEOUT_MS and SANDBOX_POLL_INTERVAL_MS)
defined once at the top of the file (after imports) with the Ms suffix to
indicate milliseconds, then replace the hardcoded 1000 and 10 in
waitForSandboxSpawn with those constants; if these timeouts are needed
elsewhere, export the constants for reuse instead of repeating literals.


describe("DO internal sub-session routes", () => {
beforeEach(() => {
installModalFetchMock();
});

afterEach(() => {
vi.unstubAllGlobals();
});

describe("GET /internal/spawn-context", () => {
it("returns SpawnContext with session and owner info", async () => {
const { stub } = await initSession({
Expand Down Expand Up @@ -190,6 +241,8 @@ describe("DO internal sub-session routes", () => {
userId: "user-1",
});

await waitForSandboxSpawn(stub);

// Set session to active to simulate a running session
await queryDO(stub, "UPDATE session SET status = 'active'");

Expand Down
Loading