diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 458bb67e0a..261aece1d1 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -67,6 +67,14 @@ jobs: bump-dev-version: needs: publish if: ${{ inputs.dry-run != true }} + # A reusable-workflow CALL cannot grant the callee more than the calling job holds, + # and GitHub refuses the whole run at startup when the called workflow's own job + # declares permissions the caller did not pass down ("startup_failure", runs + # 33615174183 / 33615177849 — the first dispatches since #3129 wired this call). + # The callee's job declares exactly these two; nothing else in this file gains them. + permissions: + contents: write + pull-requests: write uses: ./.github/workflows/dev-version-bump.yml with: released-version: v${{ inputs.version }} diff --git a/src/lib/process-control.ts b/src/lib/process-control.ts index 13ab3a0c95..3b4efd6ce3 100644 --- a/src/lib/process-control.ts +++ b/src/lib/process-control.ts @@ -66,10 +66,10 @@ export function gracefulStopHost(hostname: string | undefined): string { } /** - * Outcome of a graceful stop attempt. `"refused"` is distinct from failure: the proxy answered - * that it must NOT be stopped from here, so callers must not escalate to a forced kill. + * Outcome of a graceful stop attempt. The string results are distinct from transport failure: + * the proxy answered and is stopping, so callers must not escalate to a forced kill. */ -export type GracefulStopResult = boolean | "refused"; +export type GracefulStopResult = boolean | "refused" | "teardown-unconfirmed"; /** A proxy declined shutdown because a service under another home owns it (HTTP 409). */ export class ProxyOwnershipRefusedError extends Error {} @@ -92,6 +92,7 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): const token = configuredAdminToken(env.OPENCODEX_HOME?.trim() || undefined, env as NodeJS.ProcessEnv); if (token) headers["x-opencodex-api-key"] = token; const fetchFn = io.fetchFn ?? fetch; + let sharedTeardownConfirmed = false; try { // `ocx stop` asks the proxy NOT to restore shared client config: it does that itself, // after verifying a stopped Task Scheduler did not respawn the proxy (#3008). Letting @@ -113,6 +114,14 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): // still-running service. Report the refusal instead of forcing. if (res.status === 409) return "refused"; if (!res.ok) return false; + const body: unknown = await res.json().catch(() => null); + const expectedTeardown = io.deferSharedTeardownNonce ? "deferred" : "performed"; + sharedTeardownConfirmed = !!body + && typeof body === "object" + && "success" in body + && body.success === true + && "sharedTeardown" in body + && body.sharedTeardown === expectedTeardown; } catch { return false; } @@ -120,7 +129,8 @@ export async function stopProxyGracefully(pid: number, io: GracefulStopIo = {}): // Honor the server's own drain window: /api/stop answers 200 first, then drains for // config.shutdownTimeoutMs. Waiting less than that hard-kills mid-drain. const exitTimeoutMs = io.exitTimeoutMs ?? drainDeadlineMs(); - return waitExit(pid, exitTimeoutMs); + if (!waitExit(pid, exitTimeoutMs)) return false; + return sharedTeardownConfirmed ? true : "teardown-unconfirmed"; } function drainDeadlineMs(): number { @@ -144,6 +154,13 @@ export async function stopProxy(pid: number, io: GracefulStopIo = {}): Promise { }, fetchFn: async (_input, init) => { token = new Headers(init?.headers).get("x-opencodex-api-key"); - return new Response(null, { status: 200 }); + return Response.json({ success: true, sharedTeardown: "performed" }); }, }); expect(result).toBe(true); diff --git a/tests/grok-lifecycle.test.ts b/tests/grok-lifecycle.test.ts index a9687b9270..d2f21710d3 100644 --- a/tests/grok-lifecycle.test.ts +++ b/tests/grok-lifecycle.test.ts @@ -505,9 +505,12 @@ describe("POST /api/stop teardown", () => { const stopProxyFn = sliceFn(PROCESS_CONTROL_SOURCE, "export async function stopProxy(", "export function killProxy("); const refusedAt = stopProxyFn.indexOf('graceful === "refused"'); + const unconfirmedAt = stopProxyFn.indexOf('graceful === "teardown-unconfirmed"'); const killAt = stopProxyFn.indexOf("killProxy(pid)"); expect(refusedAt).toBeGreaterThan(-1); expect(refusedAt).toBeLessThan(killAt); + expect(unconfirmedAt).toBeGreaterThan(-1); + expect(unconfirmedAt).toBeLessThan(killAt); expect(stopProxyFn).toContain("throw new ProxyOwnershipRefusedError("); }); }); diff --git a/tests/process-control-graceful.test.ts b/tests/process-control-graceful.test.ts index 54c9ea6b9b..a9ba6b53fa 100644 --- a/tests/process-control-graceful.test.ts +++ b/tests/process-control-graceful.test.ts @@ -2,7 +2,7 @@ import { describe, expect, test } from "bun:test"; import { gracefulStopHost, stopProxyGracefully } from "../src/lib/process-control"; function okResponse(): Response { - return new Response(JSON.stringify({ success: true }), { status: 200 }); + return new Response(JSON.stringify({ success: true, sharedTeardown: "performed" }), { status: 200 }); } describe("gracefulStopHost", () => { @@ -106,4 +106,33 @@ describe("stopProxyGracefully", () => { }); expect(noExit).toBe(false); }); + + test("does not treat process exit as proof that shared teardown succeeded", async () => { + for (const body of [ + { success: false, sharedTeardown: "performed" }, + { success: true }, + ]) { + const result = await stopProxyGracefully(7, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response(JSON.stringify(body), { status: 200 })) as typeof fetch, + waitExit: () => true, + env: {}, + }); + expect(result).toBe("teardown-unconfirmed"); + } + }); + + test("accepts an explicitly confirmed deferred teardown", async () => { + const result = await stopProxyGracefully(7, { + readRuntime: () => ({ port: 10100 }), + fetchFn: (async () => new Response(JSON.stringify({ + success: true, + sharedTeardown: "deferred", + }), { status: 200 })) as typeof fetch, + waitExit: () => true, + env: {}, + deferSharedTeardownNonce: "owned-receipt", + }); + expect(result).toBe(true); + }); }); diff --git a/tests/stop-deferred-teardown.test.ts b/tests/stop-deferred-teardown.test.ts index 12a6f6003d..ce07c35a54 100644 --- a/tests/stop-deferred-teardown.test.ts +++ b/tests/stop-deferred-teardown.test.ts @@ -53,7 +53,7 @@ describe("stopProxyGracefully deferral flag", () => { readRuntime: () => ({ port: 10100 }), fetchFn: (async (url: string | URL | Request) => { urls.push(String(url)); - return new Response(JSON.stringify({ success: true }), { status: 200 }); + return new Response(JSON.stringify({ success: true, sharedTeardown: "performed" }), { status: 200 }); }) as typeof fetch, waitExit: () => true, env: {}, @@ -67,7 +67,7 @@ describe("stopProxyGracefully deferral flag", () => { readRuntime: () => ({ port: 10100 }), fetchFn: (async (url: string | URL | Request) => { urls.push(String(url)); - return new Response(JSON.stringify({ success: true }), { status: 200 }); + return new Response(JSON.stringify({ success: true, sharedTeardown: "deferred" }), { status: 200 }); }) as typeof fetch, waitExit: () => true, env: {}, @@ -86,7 +86,7 @@ describe("stopProxyGracefully deferral flag", () => { runtimeEndpoint: { hostname: "127.0.0.1", port: 10100 }, fetchFn: (async (url: string | URL | Request) => { urls.push(String(url)); - return new Response(JSON.stringify({ success: true }), { status: 200 }); + return new Response(JSON.stringify({ success: true, sharedTeardown: "performed" }), { status: 200 }); }) as typeof fetch, waitExit: () => true, env: {},