diff --git a/.github/scripts/pr-automation-workflow.test.cjs b/.github/scripts/pr-automation-workflow.test.cjs index 7fa3e79fcd..9d94aea923 100644 --- a/.github/scripts/pr-automation-workflow.test.cjs +++ b/.github/scripts/pr-automation-workflow.test.cjs @@ -81,6 +81,8 @@ describe("PR automation workflow contract", () => { assert.match(source, /!pr\.merged && pr\.head\?\.sha === run\.head_sha/); assert.match(source, /context\.eventName !== "workflow_run"/); assert.match(source, /\[409, 422\]/); + assert.match(source, /isMissingPullRequestError\(error\)/); + assert.match(source, /linked pull request.*no longer exists/); assert.doesNotMatch(source, /reRunWorkflow\s*\(/); }); diff --git a/.github/scripts/pr-automation.cjs b/.github/scripts/pr-automation.cjs index a45bf8ecb5..f9ba6bfc28 100644 --- a/.github/scripts/pr-automation.cjs +++ b/.github/scripts/pr-automation.cjs @@ -178,6 +178,10 @@ function workflowRunRetryDisposition({ run, pr, repository } = {}) { return { action: "rerun", reason: `first-${run.conclusion}`, runId: Number(run.id), pullNumber: Number(pr.number) }; } +function isMissingPullRequestError(error) { + return Number(error?.status) === 404; +} + function filePathEntries(files = []) { return (files || []).flatMap(file => { if (typeof file === "string") return [{ filename: file }]; @@ -441,6 +445,7 @@ module.exports = { buildAutomationComment, classifyPullRequest, exactHeadGate, + isMissingPullRequestError, summarizeAgedHolds, workflowRunRetryDisposition, }; diff --git a/.github/scripts/pr-automation.test.cjs b/.github/scripts/pr-automation.test.cjs index 7d102d1a6f..fcb4175581 100644 --- a/.github/scripts/pr-automation.test.cjs +++ b/.github/scripts/pr-automation.test.cjs @@ -9,6 +9,7 @@ const { buildAutomationComment, classifyPullRequest, exactHeadGate, + isMissingPullRequestError, REQUIRED_CHECKS, summarizeAgedHolds, workflowRunRetryDisposition, @@ -134,6 +135,15 @@ describe("workflowRunRetryDisposition", () => { }); }); +describe("isMissingPullRequestError", () => { + it("recognizes only GitHub's missing-resource status", () => { + assert.equal(isMissingPullRequestError({ status: 404 }), true); + assert.equal(isMissingPullRequestError({ status: "404" }), true); + assert.equal(isMissingPullRequestError({ status: 403 }), false); + assert.equal(isMissingPullRequestError(new Error("not found")), false); + }); +}); + function passingGateInput(overrides = {}) { return { liveHeadSha: SHA, diff --git a/.github/scripts/release-postpublish.cjs b/.github/scripts/release-postpublish.cjs new file mode 100644 index 0000000000..cce72f1b08 --- /dev/null +++ b/.github/scripts/release-postpublish.cjs @@ -0,0 +1,53 @@ +"use strict"; + +const SHA_RE = /^[0-9a-f]{40}$/i; + +function decideReleasePostpublish({ + expectedSha, + npmExists, + npmGitHead, + tagSha, + releaseExists, + dryRun, +}) { + if (!SHA_RE.test(String(expectedSha || ""))) { + throw new Error("expected release SHA must be a full commit SHA"); + } + if (npmExists && !SHA_RE.test(String(npmGitHead || ""))) { + throw new Error("published npm version has no trustworthy gitHead"); + } + if (npmExists && String(npmGitHead).toLowerCase() !== String(expectedSha).toLowerCase()) { + throw new Error("published npm version belongs to a different commit"); + } + if (tagSha && !SHA_RE.test(String(tagSha))) { + throw new Error("release tag did not resolve to a commit"); + } + if (tagSha && String(tagSha).toLowerCase() !== String(expectedSha).toLowerCase()) { + throw new Error("release tag belongs to a different commit"); + } + if (releaseExists && !tagSha) { + throw new Error("GitHub Release exists without its verified tag"); + } + + if (dryRun) { + return { + action: "dry-run", + publish: false, + createTag: false, + createRelease: false, + }; + } + + if (!npmExists && (tagSha || releaseExists)) { + throw new Error("Git metadata already exists before npm publication"); + } + + return { + action: npmExists ? "resume" : "publish", + publish: !npmExists, + createTag: !tagSha, + createRelease: !releaseExists, + }; +} + +module.exports = { decideReleasePostpublish }; diff --git a/.github/scripts/release-postpublish.test.cjs b/.github/scripts/release-postpublish.test.cjs new file mode 100644 index 0000000000..c39490db96 --- /dev/null +++ b/.github/scripts/release-postpublish.test.cjs @@ -0,0 +1,63 @@ +"use strict"; + +const assert = require("node:assert/strict"); +const { describe, it } = require("node:test"); +const { decideReleasePostpublish } = require("./release-postpublish.cjs"); + +const SHA = "0123456789abcdef0123456789abcdef01234567"; +const OTHER_SHA = "89abcdef0123456789abcdef0123456789abcdef"; + +function decide(overrides = {}) { + return decideReleasePostpublish({ + expectedSha: SHA, + npmExists: false, + npmGitHead: "", + tagSha: "", + releaseExists: false, + dryRun: false, + ...overrides, + }); +} + +describe("release post-publish recovery", () => { + it("starts a new release only when all public metadata is absent", () => { + assert.deepEqual(decide(), { + action: "publish", + publish: true, + createTag: true, + createRelease: true, + }); + }); + + it("resumes an npm-success/tag-failure run at the exact package gitHead", () => { + assert.deepEqual(decide({ npmExists: true, npmGitHead: SHA }), { + action: "resume", + publish: false, + createTag: true, + createRelease: true, + }); + }); + + it("makes a completed release idempotent", () => { + assert.deepEqual(decide({ + npmExists: true, + npmGitHead: SHA, + tagSha: SHA, + releaseExists: true, + }), { + action: "resume", + publish: false, + createTag: false, + createRelease: false, + }); + }); + + it("fails closed on mismatched or incomplete provenance", () => { + assert.throws(() => decide({ expectedSha: "short" }), /full commit SHA/); + assert.throws(() => decide({ npmExists: true, npmGitHead: OTHER_SHA }), /different commit/); + assert.throws(() => decide({ npmExists: true, npmGitHead: "" }), /trustworthy gitHead/); + assert.throws(() => decide({ tagSha: OTHER_SHA }), /different commit/); + assert.throws(() => decide({ releaseExists: true }), /without its verified tag/); + assert.throws(() => decide({ tagSha: SHA }), /before npm publication/); + }); +}); diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 33b77d7c9a..a415006f62 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -634,7 +634,7 @@ jobs: # green again. Keep the leg sharded and bounded: removing it from ordinary # pushes is what allowed the Windows-only backlog to accumulate unnoticed. platform-windows: - name: windows ${{ matrix.shard }}/4 + name: windows ${{ matrix.shard }}/6 needs: [changes] if: >- (github.event_name != 'pull_request' && github.event_name != 'merge_group') || @@ -656,11 +656,14 @@ jobs: # margin. 25 leaves the outer bound in place — a wedged shard still dies — # while making a completed shard the normal outcome. The crash-retry below can # double a shard's work, and this ceiling has to cover that second attempt too. + # Four shards later grew back into that ceiling as the suite expanded. Six + # shards reduce each hosted-Windows process's filesystem, Worker, and child + # process pressure without raising the ceiling or weakening assertions. timeout-minutes: 25 strategy: fail-fast: false matrix: - shard: [1, 2, 3, 4] + shard: [1, 2, 3, 4, 5, 6] steps: - name: Checkout uses: actions/checkout@9c091bb21b7c1c1d1991bb908d89e4e9dddfe3e0 # v7 @@ -722,11 +725,11 @@ jobs: fi general_list="$(mktemp -t ocx-windows-general.XXXXXX)" serial_list="$(mktemp -t ocx-windows-serial.XXXXXX)" - if ! bun scripts/ci/test-lanes.ts --lane general "${timing_args[@]}" --shard ${{ matrix.shard }}/4 > "$general_list"; then + if ! bun scripts/ci/test-lanes.ts --lane general "${timing_args[@]}" --shard ${{ matrix.shard }}/6 > "$general_list"; then echo "::error::Windows general lane selection failed." exit 1 fi - if ! bun scripts/ci/test-lanes.ts --lane serial "${timing_args[@]}" --shard ${{ matrix.shard }}/4 > "$serial_list"; then + if ! bun scripts/ci/test-lanes.ts --lane serial "${timing_args[@]}" --shard ${{ matrix.shard }}/6 > "$serial_list"; then echo "::error::Windows serial lane selection failed." exit 1 fi @@ -766,12 +769,12 @@ jobs: exit 0 fi if ! grep -Eqi 'oh no: Bun has crashed|Internal assertion failure|Segmentation fault at address|Illegal instruction|Bus error|Aborted \(core dumped\)' "$suite_log"; then - echo "::error::Windows shard ${{ matrix.shard }}/4 failed on attempt ${attempt} (exit ${suite_status}); assertion failures are not retried." + echo "::error::Windows shard ${{ matrix.shard }}/6 failed on attempt ${attempt} (exit ${suite_status}); assertion failures are not retried." exit "$suite_status" fi - echo "::warning::Bun runtime crash in Windows shard ${{ matrix.shard }}/4 (exit ${suite_status}, attempt ${attempt})." + echo "::warning::Bun runtime crash in Windows shard ${{ matrix.shard }}/6 (exit ${suite_status}, attempt ${attempt})." done - echo "::error::Bun runtime crash repeated on Windows shard ${{ matrix.shard }}/4; failing after one retry." + echo "::error::Bun runtime crash repeated on Windows shard ${{ matrix.shard }}/6; failing after one retry." exit 1 - name: CLI help smoke diff --git a/.github/workflows/pr-automation.yml b/.github/workflows/pr-automation.yml index a3b2f664eb..1a92d63f45 100644 --- a/.github/workflows/pr-automation.yml +++ b/.github/workflows/pr-automation.yml @@ -75,6 +75,7 @@ jobs: buildAutomationComment, classifyPullRequest, exactHeadGate, + isMissingPullRequestError, summarizeAgedHolds, workflowRunRetryDisposition, } = require(path.join(process.cwd(), ".github", "scripts", "pr-automation.cjs")); @@ -144,7 +145,14 @@ jobs: })).filter(pr => !pr.merged && pr.head?.sha === run.head_sha); } if (candidates.length !== 1) return; - const pr = await getPr(Number(candidates[0].number)); + let pr; + try { + pr = await getPr(Number(candidates[0].number)); + } catch (error) { + if (!isMissingPullRequestError(error)) throw error; + core.info(`CI rerun skipped: linked pull request #${candidates[0].number} no longer exists.`); + return; + } const disposition = workflowRunRetryDisposition({ run, pr, repository: `${owner}/${repo}`, }); diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index a6961cfd35..48dadd860a 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -357,6 +357,7 @@ jobs: # PREREQUISITE: configure the Trusted Publisher for this repo + workflow on npmjs.com — possible # only AFTER the package's first version exists (do the first publish locally, see the runbook). - name: Preflight release metadata + id: release-metadata env: GH_TOKEN: ${{ github.token }} RELEASE_VERSION: ${{ env.DISPATCH_VERSION }} @@ -371,37 +372,41 @@ jobs: git fetch --force --tags origin existing_tag_sha="$(git rev-parse -q --verify "refs/tags/${release_tag}^{commit}" || true)" - if [ -n "$existing_tag_sha" ] && [ "$existing_tag_sha" != "$GITHUB_SHA" ]; then - echo "::error::${release_tag} already points at ${existing_tag_sha}, not ${GITHUB_SHA}" - exit 1 - fi - - if [ -n "$existing_tag_sha" ]; then - if [ "$dry_run" = "true" ]; then - echo "::notice::${release_tag} already exists at this commit; dry-run only" - else - echo "::error::${release_tag} already exists. Refusing to publish a version with pre-existing Git metadata." - exit 1 - fi + release_exists=false + gh release view "$release_tag" >/dev/null 2>&1 && release_exists=true + + npm_exists=false + npm_git_head="" + if npm_metadata="$(npm view "${pkg_name}@${RELEASE_VERSION}" version gitHead --json 2>/dev/null)"; then + npm_exists=true + npm_git_head="$(jq -r '.gitHead // empty' <<<"$npm_metadata")" fi - if gh release view "$release_tag" >/dev/null 2>&1; then - if [ "$dry_run" = "true" ]; then - echo "::notice::GitHub Release ${release_tag} already exists; dry-run only" - else - echo "::error::GitHub Release ${release_tag} already exists. Choose the next unused patch version." - exit 1 - fi - fi - - if npm view "${pkg_name}@${RELEASE_VERSION}" version >/dev/null 2>&1; then - if [ "$dry_run" = "true" ]; then - echo "::notice::${pkg_name}@${RELEASE_VERSION} already exists on npm; dry-run only" - else - echo "::error::${pkg_name}@${RELEASE_VERSION} already exists on npm. Choose the next unused patch version." - exit 1 - fi - fi + decision="$( + EXPECTED_SHA="$GITHUB_SHA" \ + NPM_EXISTS="$npm_exists" \ + NPM_GIT_HEAD="$npm_git_head" \ + TAG_SHA="$existing_tag_sha" \ + RELEASE_EXISTS="$release_exists" \ + RELEASE_DRY_RUN="$dry_run" \ + node - <<'NODE' + const { decideReleasePostpublish } = require("./.github/scripts/release-postpublish.cjs"); + const result = decideReleasePostpublish({ + expectedSha: process.env.EXPECTED_SHA, + npmExists: process.env.NPM_EXISTS === "true", + npmGitHead: process.env.NPM_GIT_HEAD, + tagSha: process.env.TAG_SHA, + releaseExists: process.env.RELEASE_EXISTS === "true", + dryRun: process.env.RELEASE_DRY_RUN === "true", + }); + process.stdout.write(JSON.stringify(result)); + NODE + )" + action="$(jq -r .action <<<"$decision")" + echo "publish-needed=$(jq -r .publish <<<"$decision")" >> "$GITHUB_OUTPUT" + echo "tag-needed=$(jq -r .createTag <<<"$decision")" >> "$GITHUB_OUTPUT" + echo "release-needed=$(jq -r .createRelease <<<"$decision")" >> "$GITHUB_OUTPUT" + echo "::notice::Release metadata disposition: ${action}" - name: Build and validate release changelog env: @@ -427,6 +432,7 @@ jobs: DRY_RUN: ${{ env.DISPATCH_DRY_RUN }} NPM_DIST_TAG: ${{ env.DISPATCH_TAG }} CANDIDATE_PACKAGE_PATH: ${{ steps.candidate-package.outputs.path }} + PUBLISH_NEEDED: ${{ steps.release-metadata.outputs.publish-needed }} run: | set -euo pipefail package_file="" @@ -438,6 +444,8 @@ jobs: echo "::notice::TRANSITIONAL DRY RUN — building locally; automatic main releases always consume an immutable candidate" npm run prepublishOnly npm pack --dry-run + elif [ "$PUBLISH_NEEDED" != "true" ]; then + echo "::notice::Exact npm version is already published from ${GITHUB_SHA}; resuming post-publish metadata only" elif [ -z "$DISPATCH_CANDIDATE_RUN_ID" ]; then echo "::warning::transitional manual release is building locally; migrate this caller to candidate IDs" npm publish --tag "$NPM_DIST_TAG" --access public @@ -474,6 +482,8 @@ jobs: env: GH_TOKEN: ${{ github.token }} RELEASE_VERSION: ${{ env.DISPATCH_VERSION }} + TAG_NEEDED: ${{ steps.release-metadata.outputs.tag-needed }} + RELEASE_NEEDED: ${{ steps.release-metadata.outputs.release-needed }} run: | set -euo pipefail @@ -497,10 +507,29 @@ jobs: prerelease_flag="--prerelease" fi - if [ -z "$existing_tag_sha" ]; then - git tag "$release_tag" "$GITHUB_SHA" - git push origin "refs/tags/${release_tag}" + if [ "$TAG_NEEDED" = "true" ]; then + # Checkout deliberately leaves no credential in .git/config. Create + # the lightweight tag through the authenticated API instead of an + # unauthenticated git push, then verify the public ref before release. + if ! gh api --method POST "repos/${GITHUB_REPOSITORY}/git/refs" \ + -f ref="refs/tags/${release_tag}" -f sha="$GITHUB_SHA" >/dev/null; then + echo "::notice::Tag creation raced another writer; verifying the resulting ref" + fi fi - gh release create "$release_tag" --target "$GITHUB_SHA" --title "$release_tag" \ - --notes-file "$notes_file" ${prerelease_flag:+$prerelease_flag} + git fetch --force --tags origin + published_tag_sha="$(git rev-parse -q --verify "refs/tags/${release_tag}^{commit}" || true)" + if [ "$published_tag_sha" != "$GITHUB_SHA" ]; then + echo "::error::${release_tag} resolved to ${published_tag_sha:-nothing}, not ${GITHUB_SHA}" + exit 1 + fi + + if [ "$RELEASE_NEEDED" = "true" ]; then + if ! gh release create "$release_tag" --target "$GITHUB_SHA" --title "$release_tag" \ + --notes-file "$notes_file" ${prerelease_flag:+$prerelease_flag}; then + echo "::notice::Release creation raced another writer; verifying the resulting release" + gh release view "$release_tag" --json tagName,targetCommitish >/dev/null + fi + else + echo "::notice::GitHub Release ${release_tag} already exists at the verified tag" + fi diff --git a/tests/ci-workflows.test.ts b/tests/ci-workflows.test.ts index b7243717cb..2e83e00458 100644 --- a/tests/ci-workflows.test.ts +++ b/tests/ci-workflows.test.ts @@ -184,7 +184,7 @@ describe("GitHub Actions hardening", () => { const linuxShards = (ci.jobs?.test as { strategy?: { matrix?: { shard?: number[] } } }) ?.strategy?.matrix?.shard ?? []; expect(linuxShards).toEqual([1, 2, 3, 4]); - expect(workflow).toContain(`--shard \${{ matrix.shard }}/${linuxShards.length}`); + expect(workflow).toContain(`TEST_SHARD: \${{ matrix.shard }}/${linuxShards.length}`); // Every job that runs tests/ must fetch tags, because one of those tests reads // them. tests/release-version-line.test.ts compares package.json against the @@ -200,13 +200,23 @@ describe("GitHub Actions hardening", () => { expect(`${jobName}:${String(checkout?.with?.["fetch-tags"])}`).toBe(`${jobName}:true`); } - // Windows uses the same shard matrix after the single-leg isolate budget was - // replaced. Keep the two matrices equal so a future edit cannot reintroduce - // a partial Windows suite while Linux stays fully tiled. + // Windows uses more, smaller shards because its process-heavy suite and NTFS + // filesystem work repeatedly exhausted four hosted runners under load. Keep + // its matrix contiguous and bind the lane selector divisor to that matrix so + // load reduction cannot silently drop part of the suite. const windowsShards = (ci.jobs?.["platform-windows"] as { strategy?: { matrix?: { shard?: number[] } }; })?.strategy?.matrix?.shard ?? []; - expect(windowsShards).toEqual(linuxShards); + expect(windowsShards).toEqual([1, 2, 3, 4, 5, 6]); + expect(windowsShards).toEqual(windowsShards.map((_, index) => index + 1)); + const windowsSteps = (ci.jobs?.["platform-windows"] as { + steps?: Array<{ run?: string }>; + })?.steps ?? []; + expect(windowsSteps.some(step => + step.run?.includes(`--shard \${{ matrix.shard }}/${windowsShards.length}`), + )).toBe(true); + expect(ci.jobs?.["platform-windows"]?.name) + .toBe(`windows \${{ matrix.shard }}/${windowsShards.length}`); // The aggregate gate is the check a human trusts. Three ways to break it // silently: drop `if: always()` so it skips (and a skipped job reports @@ -951,9 +961,10 @@ describe("GitHub Actions hardening", () => { expect(createStep).toContain('notes_file="$GITHUB_WORKSPACE/.release-notes.md"'); expect(createStep).toContain('test -s "$notes_file"'); expect(createStep).not.toContain("generate-notes"); - expect(createStep).not.toContain("gh api"); + expect(createStep).toContain('gh api --method POST "repos/${GITHUB_REPOSITORY}/git/refs"'); + expect(createStep).not.toContain('git push origin "refs/tags/${release_tag}"'); expect(createStep.indexOf('test -s "$notes_file"')).toBeLessThan( - createStep.indexOf('git tag "$release_tag"'), + createStep.indexOf('gh api --method POST'), ); // The merged-only restriction remains on the service gate, whose diff --git a/tests/codex-history-lock.test.ts b/tests/codex-history-lock.test.ts index 63ba943e04..c3b7c621aa 100644 --- a/tests/codex-history-lock.test.ts +++ b/tests/codex-history-lock.test.ts @@ -10,6 +10,7 @@ import { type HistoryWritePermit, } from "../src/codex/history-lock"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "./helpers/test-budget"; const repoRoot = resolve(import.meta.dir, ".."); const sandboxes: string[] = []; @@ -55,7 +56,9 @@ afterEach(() => { for (const root of sandboxes.splice(0)) removeTreeWithRetry(root); }); -async function waitForPath(path: string, timeoutMs = 10_000): Promise { +// Same shape as codex-write-lock: gates on a spawned child reaching its marker, which +// costs 8-19 s on windows-latest (run 33930757649). Local stays at 10 s. +async function waitForPath(path: string, timeoutMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + timeoutMs; while (!existsSync(path)) { if (Date.now() > deadline) throw new Error(`timed out waiting for ${path}`); @@ -106,7 +109,7 @@ test("H excludes a second process across the whole history unit", async () => { // Once the holder is gone the lock is available again. const after = withHistoryWriteSerialization(sandbox.codexHome, sandbox.stateDb, () => "ok"); expect(after).toEqual({ kind: "completed", value: "ok" }); -}, 30_000); +}, SPAWN_BUDGET_MS); test("a permit is refused once its acquisition released, and for a foreign state database", () => { const sandbox = makeSandbox("ocx-history-permit-"); diff --git a/tests/codex-history-worker.test.ts b/tests/codex-history-worker.test.ts index c269947cdd..9dd85d0eca 100644 --- a/tests/codex-history-worker.test.ts +++ b/tests/codex-history-worker.test.ts @@ -12,6 +12,7 @@ import { type HistoryWorkerRunMessage, } from "../src/codex/history-worker"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "./helpers/test-budget"; // A held write lock otherwise costs the full production 5s busy timeout per // attempt, tripping bun's 5s default per-test timeout. @@ -342,7 +343,8 @@ test("a second holder of H makes the unit report blocked rather than wait", asyn `], { cwd: repoRoot, env: fixture.env, stdout: "pipe", stderr: "pipe" }); try { - const deadline = Date.now() + 10_000; + // The holder is a spawned child; 8-19 s to boot on windows-latest (run 33930757649). + const deadline = Date.now() + INTERNAL_DEADLINE_MS; while (!existsSync(ready)) { if (Date.now() > deadline) throw new Error("holder never acquired H"); await Bun.sleep(5); @@ -365,7 +367,7 @@ test("a second holder of H makes the unit report blocked rather than wait", asyn writeFileSync(release, "release"); expect(await holder.exited).toBe(0); } -}, 30_000); +}, SPAWN_BUDGET_MS); /** * The reason the parent can tell a false "app holds the DB" from a real one: diff --git a/tests/codex-inject-write-lock.test.ts b/tests/codex-inject-write-lock.test.ts index 044c60b1c8..36aae89967 100644 --- a/tests/codex-inject-write-lock.test.ts +++ b/tests/codex-inject-write-lock.test.ts @@ -30,12 +30,12 @@ const LOCK_CHILD = join(repoRoot, "tests", "helpers", "codex-write-lock-child.ts // Leave teardown and assertion headroom inside the surrounding test budget. A real // Bun child can take several seconds to start and settle on a loaded Windows runner. const SPAWN_TIMEOUT_MS = SPAWN_BUDGET_MS - 5_000; -// The contender uses a much shorter bound because production contention is -// fail-fast (lockTimeoutMs=0). Keep the holder alive well beyond that bound so a -// slow child launch cannot turn an intended busy result into a post-release apply. -const CONTENTION_CHILD_TIMEOUT_MS = 10_000; -const CONTENTION_HOLDER_MARGIN_MS = 5_000; -const CONTENTION_HOLD_MS = SPAWN_TIMEOUT_MS - CONTENTION_HOLDER_MARGIN_MS; +// Lock acquisition is fail-fast, not cold module loading. The holder must outlive +// marker observation plus the entire contender, and all three boots need a budget. +const CONTENTION_READY_MS = SPAWN_TIMEOUT_MS; +const CONTENTION_REAP_MS = 5_000; +const CONTENTION_HOLD_MS = CONTENTION_READY_MS + SPAWN_TIMEOUT_MS + CONTENTION_REAP_MS; +const CONTENTION_TEST_MS = 3 * SPAWN_TIMEOUT_MS + 3 * CONTENTION_REAP_MS; setDefaultTimeout(SPAWN_BUDGET_MS); @@ -307,6 +307,7 @@ describe("the lock is on the production path", () => { * must not have written its candidate bytes. */ test("a held lock makes real injection report busy and write nothing", async () => { + const fixtureRoot = root; seedNative(); // Establish the coordinator first: a clean home has no row, and the holder // needs one to contend over. @@ -325,29 +326,46 @@ describe("the lock is on the production path", () => { timeoutMs: 5_000, holdMarker, releaseMarker, - // Keep a slow Windows contender from outliving the hold, while staying - // below the 40s child bound and the 45s test budget. + // Explicit release is normal; the ceiling also covers a delayed observer + // and cold contender without releasing the lock underneath its assertion. holdMs: CONTENTION_HOLD_MS, }), }, stdout: "pipe", stderr: "pipe", }); + const holderDone = Promise.all([ + holder.exited, + new Response(holder.stdout).text(), + new Response(holder.stderr).text(), + ]).then(([exitCode, stdout, stderr]) => ({ exitCode, stdout, stderr })); + const waitForHolder = async (timeoutMs: number) => { + let timer: ReturnType | undefined; + try { + return await Promise.race([ + holderDone, + new Promise(resolve => { timer = setTimeout(() => resolve(null), timeoutMs); }), + ]); + } finally { + if (timer !== undefined) clearTimeout(timer); + } + }; let primaryFailed = false; let primaryError: unknown; let cleanupFailed = false; let cleanupError: unknown; try { - const deadline = Date.now() + 10_000; + const deadline = Date.now() + CONTENTION_READY_MS; while (!existsSync(holdMarker) && Date.now() < deadline) { - requireChildSuccess(runChild(["--eval", "Bun.sleepSync(20)"], process.env), "hold-marker wait child"); + if (holder.exitCode !== null) throw new Error(`lock holder exited before readiness: ${JSON.stringify(await holderDone)}`); + await Bun.sleep(20); } - expect(existsSync(holdMarker)).toBeTrue(); + if (!existsSync(holdMarker)) throw new Error("lock holder did not publish its ready marker"); // PROCESS-UNIQUE bytes: a different port means different candidate bytes, so // the loser's work is identifiable rather than assumed. - const contender = runInject(20200, 0, CONTENTION_CHILD_TIMEOUT_MS); + const contender = runInject(20200, 0); expect(contender.success).toBeFalse(); expect(contender.retryable).toBeTrue(); @@ -370,7 +388,18 @@ describe("the lock is on the production path", () => { // Always release and reap the holder, including when marker wait, // contender startup, or an assertion fails. Otherwise teardown races a // live child that still owns the coordinator database on Windows. - await holder.exited; + const ended = await waitForHolder(CONTENTION_REAP_MS); + if (ended === null) { + holder.kill("SIGKILL"); + const killed = await waitForHolder(CONTENTION_REAP_MS); + if (killed === null) { + const index = cleanup.indexOf(fixtureRoot); + if (index >= 0) cleanup.splice(index, 1); + throw new Error(`lock holder could not be joined; retained fixture ${fixtureRoot}`); + } + throw new Error(`lock holder required forced termination: ${JSON.stringify(killed)}`); + } + if (ended.exitCode !== 0) throw new Error(`lock holder failed: ${JSON.stringify(ended)}`); } catch (error) { if (!cleanupFailed) { cleanupFailed = true; @@ -378,9 +407,10 @@ describe("the lock is on the production path", () => { } } } + if (primaryFailed && cleanupFailed) throw new AggregateError([primaryError, cleanupError], "contention and holder cleanup failed"); if (primaryFailed) throw primaryError; if (cleanupFailed) throw cleanupError; - }, SPAWN_BUDGET_MS); + }, CONTENTION_TEST_MS); }); describe("pre-substrate home adoption", () => { diff --git a/tests/codex-prompt-route.test.ts b/tests/codex-prompt-route.test.ts index 8b13007b33..f54741b8b6 100644 --- a/tests/codex-prompt-route.test.ts +++ b/tests/codex-prompt-route.test.ts @@ -20,6 +20,7 @@ import { import type { ManagementPrincipal } from "../src/server/management-auth"; import type { OcxConfig } from "../src/types"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS } from "./helpers/test-budget"; const MARKER = "# Auto-injected by opencodex"; const config = { port: 10100, defaultProvider: "openai", providers: {} } as OcxConfig; @@ -71,7 +72,9 @@ function read(path: string): string | null { } async function waitUntil(predicate: () => boolean, detail: string): Promise { - const deadline = Date.now() + 5_000; + // Every caller gates on a spawned probe child writing a pid/start marker: 8-19 s to boot + // on windows-latest (run 33930757649). Keep this below the enclosing case budget. + const deadline = Date.now() + INTERNAL_DEADLINE_MS; while (!predicate()) { if (Date.now() >= deadline) throw new Error(`timed out waiting for ${detail}`); await Bun.sleep(10); diff --git a/tests/codex-prompt-text-probe.test.ts b/tests/codex-prompt-text-probe.test.ts index 17278b3818..92f903cc72 100644 --- a/tests/codex-prompt-text-probe.test.ts +++ b/tests/codex-prompt-text-probe.test.ts @@ -19,6 +19,7 @@ import { setPromptTextProbeCommandForTests, } from "../src/codex/prompt-text-probe"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS } from "./helpers/test-budget"; const lifecycleRoots: string[] = []; const VALID_PROBE_OUTPUT = JSON.stringify([{ @@ -32,7 +33,8 @@ function message(text: string): string { } async function waitUntil(predicate: () => boolean, detail: string): Promise { - const deadline = Date.now() + 5_000; + // Gates on a spawned child writing its pid marker or exiting: 8-19 s on windows-latest. + const deadline = Date.now() + INTERNAL_DEADLINE_MS; while (!predicate()) { if (Date.now() >= deadline) throw new Error(`timed out waiting for ${detail}`); await Bun.sleep(10); diff --git a/tests/codex-restore-app-rewrite.test.ts b/tests/codex-restore-app-rewrite.test.ts index 4aab267a8f..b041976b3c 100644 --- a/tests/codex-restore-app-rewrite.test.ts +++ b/tests/codex-restore-app-rewrite.test.ts @@ -23,9 +23,8 @@ const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.ur // Every case here spawns a real Bun subprocess (runScript); on a loaded Windows // shard the flat 15s ceilings timed out (F-03) while the child was doing exactly -// the work it claims. Same convention as codex-inject-integration/codex-journal: -// the test budget is SPAWN_BUDGET_MS and the spawn gets an internal deadline -// 5s under it, so a hung child fails as a subprocess error, not a runner timeout. +// the work it claims. The spawn is bounded to SPAWN_BUDGET_MS so a hung child +// fails as a subprocess error, not an unbounded runner process. setDefaultTimeout(SPAWN_BUDGET_MS); /** Inject, simulate the app's comment-dropping rewrite, then restore. */ @@ -138,13 +137,25 @@ const REINJECT_AFTER_USER_EDIT_RESTORE = [ ].join(String.fromCharCode(10)); function runScript(codexHome: string, script: string): { stdout: string; stderr: string; status: number } { - const result = spawnSync(process.execPath, ["--eval", script], { + // Normally disabled; reproduces a healthy child exceeding the old case limit. + const delayMs = Number(process.env.OCX_TEST_CODEX_RESTORE_DELAY_MS ?? 0); + if (!Number.isFinite(delayMs) || delayMs < 0 || delayMs > 60_000) { + throw new Error("invalid restore child delay fault"); + } + const evaluatedScript = delayMs > 0 ? `await Bun.sleep(${delayMs});\n${script}` : script; + const result = spawnSync(process.execPath, ["--eval", evaluatedScript], { cwd: repoRoot, env: { ...process.env, CODEX_HOME: codexHome }, encoding: "utf8", - timeout: SPAWN_BUDGET_MS - 5_000, + timeout: SPAWN_BUDGET_MS, + killSignal: "SIGKILL", }); - return { stdout: result.stdout?.trim() ?? "", stderr: result.stderr?.trim() ?? "", status: result.status ?? 1 }; + const stdout = result.stdout?.trim() ?? ""; + const stderr = result.stderr?.trim() ?? ""; + if (result.error || result.status !== 0 || result.signal !== null) { + throw new Error(`restore child failed: status=${result.status} signal=${result.signal ?? "none"} error=${result.error?.message ?? "none"}\nstdout=${stdout.slice(-8192)}\nstderr=${stderr.slice(-8192)}`); + } + return { stdout, stderr, status: result.status }; } describe("#1798 restore after the Codex app rewrites the config", () => { @@ -170,7 +181,7 @@ describe("#1798 restore after the Codex app rewrites the config", () => { expect(restored).not.toContain("127.0.0.1:10100"); // The user's own pre-injection content is still theirs. expect(restored).toContain("gpt-5.5"); - }); + }, 2 * SPAWN_BUDGET_MS); test("a user's own openai_base_url written before injection is preserved", () => { // Force a byte mismatch so exact journal restore cannot hide a fallback ownership bug. @@ -188,7 +199,7 @@ describe("#1798 restore after the Codex app rewrites the config", () => { const restored = readFileSync(join(testDir, "config.toml"), "utf8"); expect(restored).toContain("https://my-own-gateway.example/v1"); expect(restored).not.toContain("127.0.0.1:10100"); - }); + }, 2 * SPAWN_BUDGET_MS); test("reinjection refreshes the owned route and catalog recorded for restore", () => { writeFileSync(join(testDir, "config.toml"), 'model = "gpt-5.5"\n', "utf8"); @@ -199,7 +210,7 @@ describe("#1798 restore after the Codex app rewrites the config", () => { const recorded = JSON.parse(r.stdout) as { url: string; catalog: string }; expect(recorded.url).toBe("http://127.0.0.1:10200/v1"); expect(recorded.catalog).toBe(join(testDir, "second-catalog.json")); - }); + }, 2 * SPAWN_BUDGET_MS); test("a user setting added after first injection survives reinjection and restore", () => { writeFileSync(join(testDir, "config.toml"), 'model = "gpt-5.5"\n', "utf8"); @@ -223,7 +234,7 @@ describe("#1798 restore after the Codex app rewrites the config", () => { expect(result.afterRestore).not.toContain("openai_base_url"); expect(result.afterRestore).not.toContain("127.0.0.1:10200"); expect(result.profileExistsAfterRestore).toBe(false); - }); + }, 2 * SPAWN_BUDGET_MS); test("the routed catalog we wrote is restored even when the rewrite dropped model_catalog_json", () => { // The catalog half of #1798. Restore used to re-resolve its target from the CURRENT @@ -239,5 +250,5 @@ describe("#1798 restore after the Codex app rewrites the config", () => { const routed = (cache.models ?? []).filter((m: { slug?: string }) => typeof m.slug === "string" && m.slug.includes("/")); expect(routed).toEqual([]); expect(JSON.parse(r.stdout).catalog).toBe(cachePath); - }); + }, 2 * SPAWN_BUDGET_MS); }); diff --git a/tests/codex-retained-root-serialization.test.ts b/tests/codex-retained-root-serialization.test.ts index 5fd5238539..d821377c96 100644 --- a/tests/codex-retained-root-serialization.test.ts +++ b/tests/codex-retained-root-serialization.test.ts @@ -18,7 +18,7 @@ import { } from "../src/codex/user-identity"; import { claimOwnedServiceHome, withOwnedServiceHomePreload } from "./helpers/owned-service-home"; import { removeTreeWithRetry } from "./helpers/remove-tree"; -import { SPAWN_BUDGET_MS } from "./helpers/test-budget"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "./helpers/test-budget"; const repoRoot = resolve(import.meta.dir, ".."); const sandboxes: Sandbox[] = []; @@ -32,6 +32,8 @@ interface Sandbox { readonly env: Record; readonly serviceManagerEnv: Record; readonly preloadPath?: string; + readonly children: Set>; + readonly releaseMarkers: Set; } function nativeEntry(slug: string, visibility = "list"): Record { @@ -90,11 +92,24 @@ function makeSandbox(prefix: string): Sandbox { }, serviceManagerEnv: serviceHome.env, preloadPath: serviceHome.preloadPath, + children: new Set(), + releaseMarkers: new Set(), }; sandboxes.push(sandbox); return sandbox; } +async function teardownSandbox(sandbox: Sandbox): Promise { + for (const marker of sandbox.releaseMarkers) { + try { writeFileSync(marker, "release"); } catch { /* root may already be gone */ } + } + for (const child of sandbox.children) { + if (child.exitCode === null) child.kill(); + } + await Promise.all([...sandbox.children].map(child => child.exited)); + sandbox.children.clear(); +} + function sandboxChildEnv(sandbox: Sandbox): Record { return { ...sandbox.env, ...sandbox.serviceManagerEnv }; } @@ -111,6 +126,16 @@ async function waitForPath(path: string, timeoutMs: number): Promise { } } +async function raceBarrier(child: ReturnType, barrier: Promise): Promise { + const exitedEarly = child.exited.then(async exitCode => { + const stdout = await new Response(child.stdout).text(); + const stderr = await new Response(child.stderr).text(); + throw new Error(`sync exited before provider barrier (${exitCode})\nstdout=${stdout}\nstderr=${stderr}`); + }); + exitedEarly.catch(() => undefined); + await Promise.race([barrier, exitedEarly]); +} + async function runChild( sandbox: Sandbox, script: string, @@ -121,6 +146,7 @@ async function runChild( stdout: "pipe", stderr: "pipe", }); + sandbox.children.add(child); const [exitCode, stdout, stderr] = await Promise.all([ child.exited, new Response(child.stdout).text(), @@ -152,8 +178,10 @@ async function holdCatalogLock(sandbox: Sandbox): Promise<{ stdout: "pipe", stderr: "pipe", }); + sandbox.children.add(child); + sandbox.releaseMarkers.add(release); await waitForPath(ready, CHILD_MARKER_BUDGET_MS); - return { release: () => writeFileSync(release, "release"), child }; + return { release: () => { try { writeFileSync(release, "release"); } catch { /* teardown may have released already */ } }, child }; } function seedCatalog(sandbox: Sandbox, bytes = catalogBytes()): string { @@ -163,9 +191,10 @@ function seedCatalog(sandbox: Sandbox, bytes = catalogBytes()): string { return path; } -afterEach(() => { +afterEach(async () => { const identity = resolveEffectiveUserIdentity(); for (const sandbox of sandboxes.splice(0)) { + await teardownSandbox(sandbox); const database = resolveCodexCatalogSerializationDatabasePath(identity, sandbox.codexHome); for (const suffix of ["", "-journal", "-wal", "-shm"]) rmSync(`${database}${suffix}`, { force: true }); removeTreeWithRetry(sandbox.root); @@ -312,16 +341,11 @@ for (const publisher of ["convergence", "retained"] as const) { const response = await handleManagementAPI(req, new URL(req.url), config); console.log(JSON.stringify({ status: response.status, body: await response.json() })); `], sandbox.preloadPath)], { cwd: repoRoot, env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe" }); + sandbox.children.add(sync); const syncStdout = new Response(sync.stdout).text(); const syncStderr = new Response(sync.stderr).text(); - await Promise.race([ - waitForPath(requested, CHILD_MARKER_BUDGET_MS), - sync.exited.then(async exitCode => { - const [stdout, stderr] = await Promise.all([syncStdout, syncStderr]); - throw new Error(`sync exited before provider barrier (${exitCode})\nstdout=${stdout}\nstderr=${stderr}`); - }), - ]); + await raceBarrier(sync, waitForPath(requested, CHILD_MARKER_BUDGET_MS)); const published = await runPublisher(sandbox, publisher, config); if (published.exitCode !== 0) { throw new Error(`${publisher} publisher failed\nstdout=${published.stdout}\nstderr=${published.stderr}`); @@ -397,16 +421,11 @@ test("a persisted runtime selection moved by another process during the await bl const { syncCatalogModels } = await import("./src/codex/catalog/sync.ts"); console.log(JSON.stringify(await syncCatalogModels(config))); `], sandbox.preloadPath)], { cwd: repoRoot, env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe" }); + sandbox.children.add(sync); const syncStdout = new Response(sync.stdout).text(); const syncStderr = new Response(sync.stderr).text(); - await Promise.race([ - waitForPath(requested, CHILD_MARKER_BUDGET_MS), - sync.exited.then(async exitCode => { - const [stdout, stderr] = await Promise.all([syncStdout, syncStderr]); - throw new Error(`sync exited before provider barrier (${exitCode})\nstdout=${stdout}\nstderr=${stderr}`); - }), - ]); + await raceBarrier(sync, waitForPath(requested, CHILD_MARKER_BUDGET_MS)); // Another process selects a different Codex runtime. No catalog byte changes. writeFileSync(runtimeStatePath, `${JSON.stringify({ @@ -472,6 +491,7 @@ test("two processes at the post-approval management seam serialize instead of in const { withConfigMutationLockSync } = await import("./src/config.ts"); withConfigMutationLockSync(() => undefined); `], { cwd: repoRoot, env: sandbox.env, stdout: "pipe", stderr: "pipe" }); + sandbox.children.add(warm); expect(await warm.exited).toBe(0); const routeScript = (marker: string) => ` @@ -482,7 +502,8 @@ test("two processes at the post-approval management seam serialize instead of in // looked exactly like a production defect until the encoder said so. globalThis.fetch = async () => { writeFileSync(${JSON.stringify(barrier)} + "-" + ${JSON.stringify(marker)}, "here"); - const deadline = Date.now() + 8000; + // Two children rendezvous on markers; either may take 8-19 s to boot on windows-latest. + const deadline = Date.now() + INTERNAL_DEADLINE_MS; while (Date.now() < deadline) { if (existsSync(${JSON.stringify(barrier)} + "-a") && existsSync(${JSON.stringify(barrier)} + "-b")) break; await Bun.sleep(5); @@ -515,7 +536,8 @@ test("two processes at the post-approval management seam serialize instead of in // On macOS CI both children can still lose the config lock before approval even // after the warm-up — that proves nothing about catalog serialization. Retry // vacuous runs until at least one process reaches the post-approval seam. - const attemptDeadline = Date.now() + 20_000; + // Each attempt boots two real children; bound the retry loop by the spawn budget, not a literal. + const attemptDeadline = Date.now() + SPAWN_BUDGET_MS; let results: Array<{ exitCode: number; stdout: string; stderr: string }> | undefined; while (Date.now() < attemptDeadline) { for (const marker of ["a", "b"] as const) { @@ -527,6 +549,7 @@ test("two processes at the post-approval management seam serialize instead of in [process.execPath, ...withOwnedServiceHomePreload(["--eval", routeScript(marker)], sandbox.preloadPath)], { cwd: repoRoot, env: sandboxChildEnv(sandbox), stdout: "pipe", stderr: "pipe" }, )); + for (const child of children) sandbox.children.add(child); results = await Promise.all(children.map(async child => { const [exitCode, stdout, stderr] = await Promise.all([ diff --git a/tests/codex-sync-api.test.ts b/tests/codex-sync-api.test.ts index f74b3cf898..8be66bd429 100644 --- a/tests/codex-sync-api.test.ts +++ b/tests/codex-sync-api.test.ts @@ -17,7 +17,10 @@ const TEST_OCX_HOME = join(TEST_DIR, "ocx"); const TEST_HOME = join(TEST_DIR, "home"); const repoRoot = join(import.meta.dir, ".."); const CHILD_TIMEOUT_MS = SPAWN_BUDGET_MS - 5_000; -const NESTED_CHILD_TIMEOUT_MS = 30_000; +const COMPETING_OFF_REAP_MS = 5_000; +const COMPETING_OFF_BOOT_MS = SPAWN_BUDGET_MS - COMPETING_OFF_REAP_MS; +const COMPETING_OFF_CHILD_MS = 2 * COMPETING_OFF_BOOT_MS + COMPETING_OFF_REAP_MS; +const COMPETING_OFF_TEST_MS = COMPETING_OFF_CHILD_MS + COMPETING_OFF_REAP_MS; let prevCodexHome: string | undefined; let prevOpenCodexHome: string | undefined; let prevHome: string | undefined; @@ -349,23 +352,35 @@ describe("GUI/CLI Codex sync backend", () => { 'const { injectCodexConfig } = require("./src/codex/inject");', '(async () => {', ' const snapshot = loadConfig(); // admitted BEFORE the flip: reads as ON', + ' let flipFailure;', ' const result = await syncModelsToCodex(12345, snapshot, null, {', ' refreshCodexModelCatalog: async () => {', ' // The provider-discovery window: a second real process persists OFF.', ' // This child only flips desired state; do not propagate the service-probe flag.', + ` const flipBudgetMs = ${COMPETING_OFF_BOOT_MS};`, + ' const remainingMs = Number(process.env.OCX_TEST_COMPETING_OFF_DEADLINE) - Date.now();', + ` if (!Number.isFinite(remainingMs) || remainingMs < flipBudgetMs + ${COMPETING_OFF_REAP_MS}) {`, + ' flipFailure = new Error("competing OFF flip not started: insufficient remaining budget " + remainingMs);', + ' throw flipFailure;', + ' }', ' const flipEnv = { ...process.env }; delete flipEnv.OCX_TEST_SERVICE_HOME_PROBE;', ' const flip = spawnSync(process.execPath, ["--eval",', ' \'const { setIntegrationEnabled } = require("./src/codex/desired-state");\'', ' + \'const r = setIntegrationEnabled("codex", false);\'', ' + \'if (!r.ok) { console.error(JSON.stringify(r)); process.exit(1); }\',', - ` ], { cwd: process.cwd(), env: flipEnv, encoding: "utf8", timeout: ${NESTED_CHILD_TIMEOUT_MS}, killSignal: "SIGKILL" });`, - ' if (flip.status !== 0) throw new Error("flip failed: " + flip.stderr);', + ` ], { cwd: process.cwd(), env: flipEnv, encoding: "utf8", timeout: ${COMPETING_OFF_BOOT_MS}, killSignal: "SIGKILL", windowsHide: true });`, + ' if (flip.error || flip.signal !== null || flip.status !== 0) {', + ' flipFailure = new Error("competing OFF flip failed: status=" + flip.status + " signal=" + flip.signal + " error=" + (flip.error?.message ?? "none") + " stdout=" + flip.stdout + " stderr=" + flip.stderr);', + ' throw flipFailure;', + ' }', ' return { added: 0, path: "/tmp/none.json", catalogExists: false, catalogWritten: false, cacheSynced: false, comboOmissions: [] };', ' },', - ' injectCodexConfig, // the REAL injector; its under-lock re-read is the claim', + ' // The REAL injector remains the normal path; fixture failure must not be swallowed by discovery fallback.', + ' injectCodexConfig: (...args) => { if (flipFailure) throw flipFailure; return injectCodexConfig(...args); },', ' });', + ' if (flipFailure) throw flipFailure;', ' console.log(JSON.stringify({ status: result.status, skippedReason: result.skippedReason, ok: result.ok }));', - '})();', + '})().catch(error => { console.error(error); process.exitCode = 1; });', ].join("\n"); const before = readFileSync(join(raceCodexHome, "config.toml"), "utf8"); const child = spawnSync(process.execPath, childArgs(["--eval", script]), { @@ -375,10 +390,12 @@ describe("GUI/CLI Codex sync backend", () => { USERPROFILE: raceHome, CODEX_HOME: raceCodexHome, OPENCODEX_HOME: raceOcxHome, + OCX_TEST_COMPETING_OFF_DEADLINE: String(Date.now() + COMPETING_OFF_CHILD_MS), }), encoding: "utf8", - timeout: CHILD_TIMEOUT_MS, + timeout: COMPETING_OFF_CHILD_MS, killSignal: "SIGKILL", + windowsHide: true, }); expect(child.status).toBe(0); const line = child.stdout.trim().split("\n").filter(Boolean).pop() ?? "{}"; @@ -388,7 +405,7 @@ describe("GUI/CLI Codex sync backend", () => { } finally { removeTreeWithRetry(raceRoot); } - }, SPAWN_BUDGET_MS); + }, COMPETING_OFF_TEST_MS); test("surfaces combo catalog omissions in sync result and CLI stderr (#484)", async () => { const logs: string[] = []; diff --git a/tests/codex-write-lock.test.ts b/tests/codex-write-lock.test.ts index 2c22b1d750..a1d5c3fae8 100644 --- a/tests/codex-write-lock.test.ts +++ b/tests/codex-write-lock.test.ts @@ -24,6 +24,7 @@ import { } from "../src/codex/codex-write-lock"; import type { AdmissionSnapshot } from "../src/codex/convergence-types"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "./helpers/test-budget"; let root = ""; let codexHome = ""; @@ -310,7 +311,10 @@ describe("two real processes contend for one lock", () => { return JSON.parse(line) as { status: string; reason?: string; value?: string; lockId?: string }; } - async function waitFor(path: string, timeoutMs = 10_000): Promise { + // A spawned holder child boots in 8-19 s on a loaded windows-latest shard; the 10 s + // literal expired first on run 33930757649. Keep this internal wait below the case budget + // so its marker-specific diagnostic is reported instead of Bun's test timeout. + async function waitFor(path: string, timeoutMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (Bun.file(path).size > 0) return; @@ -343,7 +347,7 @@ describe("two real processes contend for one lock", () => { // was contention rather than a permanent refusal wearing its label. const after = await withCodexWriteLock(options({ timeoutMs: 5_000 }), publishing("parent")); expect(after.status).toBe("acquired"); - }, 30_000); + }, SPAWN_BUDGET_MS); test("both processes resolve the same lock id for one home", async () => { @@ -351,7 +355,7 @@ describe("two real processes contend for one lock", () => { expect(first.status).toBe("acquired"); const local = canonicalizeCodexHome(codexHome); expect(local.ok && first.lockId).toBe(local.ok ? local.home.lockId : "x"); - }, 30_000); + }, SPAWN_BUDGET_MS); /** * A waiting contender must actually wait rather than fail fast — and must @@ -372,7 +376,7 @@ describe("two real processes contend for one lock", () => { expect(holderResult.status).toBe("acquired"); expect(waited.status).toBe("acquired"); expect(waited.status === "acquired" && waited.waitedMs).toBeGreaterThan(0); - }, 30_000); + }, SPAWN_BUDGET_MS); /** @@ -456,6 +460,6 @@ describe("two real processes contend for one lock", () => { ); expect(after.status).toBe("acquired"); expect(after.lockId).toBe(held.lockId); - }, 30_000); + }, SPAWN_BUDGET_MS); } }); diff --git a/tests/fork/release-candidate-publish-workflow.test.ts b/tests/fork/release-candidate-publish-workflow.test.ts index 7db5fb6187..132cf30c37 100644 --- a/tests/fork/release-candidate-publish-workflow.test.ts +++ b/tests/fork/release-candidate-publish-workflow.test.ts @@ -38,4 +38,13 @@ describe("release candidate publish bridge", () => { expect(text).toContain('if [ -z "$DISPATCH_CANDIDATE_RUN_ID" ]; then'); expect(text).toContain('elif [ -z "$DISPATCH_CANDIDATE_RUN_ID" ]; then'); }); + + test("recovers exact npm-success metadata without publishing twice", () => { + expect(text).toContain("release-postpublish.cjs"); + expect(text).toContain("npm view \"${pkg_name}@${RELEASE_VERSION}\" version gitHead --json"); + expect(text).toContain("PUBLISH_NEEDED: ${{ steps.release-metadata.outputs.publish-needed }}"); + expect(text).toContain("resuming post-publish metadata only"); + expect(text).toContain('gh api --method POST "repos/${GITHUB_REPOSITORY}/git/refs"'); + expect(text).not.toContain('git push origin "refs/tags/${release_tag}"'); + }); }); diff --git a/tests/helpers/native-profile-startup-child.ts b/tests/helpers/native-profile-startup-child.ts index f474e97b4b..7459ecf9b3 100644 --- a/tests/helpers/native-profile-startup-child.ts +++ b/tests/helpers/native-profile-startup-child.ts @@ -57,6 +57,10 @@ globalThis.fetch = (async (input: RequestInfo | URL, init?: RequestInit): Promis return realFetch(input, init); }) as typeof fetch; +if (process.env.OCX_TEST_NATIVE_STARTUP_FAIL_BEFORE_LISTEN === "1") { + throw new Error("injected native startup failure before listen"); +} + const server = startServer(0, { inspectNativeCodexOwnership: () => ({ ownership: "owned", @@ -73,7 +77,11 @@ const server = startServer(0, { // The parent treats existence as readiness and parses the port immediately. Publish // through a rename so it can never observe the file between create and write. +const portDelayMs = Number(process.env.OCX_TEST_NATIVE_STARTUP_DELAY_PORT_MS ?? 0); +if (!Number.isFinite(portDelayMs) || portDelayMs < 0 || portDelayMs > 60_000) throw new Error("invalid native startup port delay fault"); +if (portDelayMs > 0) await Bun.sleep(portDelayMs); atomicWriteFile(portPath, String(server.port)); +console.info(`[native-startup] port-published elapsedMs=${Date.now() - Number(process.env.NATIVE_STARTUP_LAUNCHED_AT ?? Date.now())}`); // #1061: the parent parses this file as soon as it exists, so a partial write // surfaces as `Unexpected EOF`. atomicWriteFile publishes through a rename, so a // reader sees either nothing or the whole document. diff --git a/tests/helpers/storage-policy-api.ts b/tests/helpers/storage-policy-api.ts index 70ef03f15b..0451132f4b 100644 --- a/tests/helpers/storage-policy-api.ts +++ b/tests/helpers/storage-policy-api.ts @@ -24,6 +24,7 @@ import { import { stopStorageCleanupScheduler } from "../../src/storage/policy-scheduler"; import { drainStorageWorkers } from "../../src/storage/worker-lifecycle"; import { removeTreeWithRetry } from "./remove-tree"; +import { INTERNAL_DEADLINE_MS } from "./test-budget"; export function baseConfig(): OcxConfig { return { @@ -58,7 +59,9 @@ export function seedArchived(codexHome: string): void { export async function waitForJobIdle( serverUrl: URL, startedAt: number, - timeoutMs = 15_000, + // Polls a live server for a worker-backed job to settle; the worker's OS-thread join is + // the slow half on Windows. Named so every caller inherits the same bound. + timeoutMs = INTERNAL_DEADLINE_MS, ): Promise<{ enabled: boolean; lastRun?: { removed: number }; diff --git a/tests/multi-agent-keep-native-v1.test.ts b/tests/multi-agent-keep-native-v1.test.ts index bba1031765..a4a00d0f0b 100644 --- a/tests/multi-agent-keep-native-v1.test.ts +++ b/tests/multi-agent-keep-native-v1.test.ts @@ -90,6 +90,46 @@ function isolateHomes(): void { process.env.CODEX_HOME = mkdtempSync(join(tmpdir(), "codex-keep-native-")); } +/** + * Read the semantic `features ` triple from either argv + * shape emitted by commandInvocation. Windows .cmd shims are wrapped through + * cmd.exe, so the action is not a fixed positional argument there. + */ +function featureActionOf(args: readonly string[]): string { + const ACTION = /^(?:enable|disable)$/; + const FEATURE = /^[a-z0-9_]+$/; + + if (args.length === 3 && args[0] === "features") { + const [, action, feature] = args; + if (!ACTION.test(action!) || !FEATURE.test(feature!)) { + throw new Error(`malformed features argv: ${JSON.stringify(args)}`); + } + return `features ${action} ${feature}`; + } + + if (args.length === 4 && args[0] === "/d" && args[1] === "/s" && args[2] === "/c") { + const line = args[3]!; + if (!line.startsWith('"') || !line.endsWith('"')) { + throw new Error(`unquoted cmd line: ${line}`); + } + const inner = line.slice(1, -1); + const tokens = inner.split(/(? t.replace(/\^+"/g, "").replace(/\^ /g, " ")); + const [target, keyword, action, feature, ...rest] = tokens; + if ( + rest.length > 0 + || !/\.(cmd|bat)$/i.test(target ?? "") + || keyword !== "features" + || !ACTION.test(action ?? "") + || !FEATURE.test(feature ?? "") + ) { + throw new Error(`unrecognized cmd invocation: ${inner}`); + } + return `features ${action} ${feature}`; + } + + throw new Error(`unrecognized features invocation: ${JSON.stringify(args)}`); +} + function captureLog(): { logs: string[]; errors: string[]; log: { log: (m?: unknown) => void; error: (m?: unknown) => void } } { const logs: string[] = []; const errors: string[] = []; @@ -165,6 +205,27 @@ describe("keep-native-v1 restamp path", () => { }); describe("ocx v2 keep-native-v1", () => { + test("featureActionOf parses both launcher shapes and rejects malformed invocations", () => { + expect(featureActionOf(["features", "disable", "multi_agent_v2"])) + .toBe("features disable multi_agent_v2"); + expect(featureActionOf(["/d", "/s", "/c", + String.raw`"C:\npm\codex.cmd ^"features^" ^"disable^" ^"multi_agent_v2^""`])) + .toBe("features disable multi_agent_v2"); + expect(featureActionOf(["/d", "/s", "/c", + String.raw`"C:\Program^ Files\npm\codex.cmd ^"features^" ^"disable^" ^"multi_agent_v2^""`])) + .toBe("features disable multi_agent_v2"); + expect(featureActionOf(["/d", "/s", "/c", + String.raw`"C:\p\node_modules\.bin\codex.cmd ^^^"features^^^" ^^^"enable^^^" ^^^"multi_agent_v2^^^""`])) + .toBe("features enable multi_agent_v2"); + + expect(() => featureActionOf(["/d", "/s", "/c", + String.raw`"echo ^"features^" ^"disable^" ^"multi_agent_v2^""`])).toThrow(); + expect(() => featureActionOf(["features", "disable"])).toThrow(); + expect(() => featureActionOf(["features", "restart", "multi_agent_v2"])).toThrow(); + expect(() => featureActionOf(["/d", "/s", "/c", "features disable multi_agent_v2"])).toThrow(); + expect(() => featureActionOf(["-c", "features disable multi_agent_v2"])).toThrow(); + }); + test("enabling the native-v1 pin disables the global V2 override before catalog sync", async () => { isolateHomes(); saveConfig({ ...loadConfig(), multiAgentMode: "v2" }); @@ -174,7 +235,7 @@ describe("ocx v2 keep-native-v1", () => { const code = await cmdV2(["keep-native-v1", "on"], { execFile: (_file, args) => { - events.push(args.join(" ")); + events.push(featureActionOf(args)); writeFileSync(codexConfig, readFileSync(codexConfig, "utf8").replace("enabled = true", "enabled = false")); }, sync: async () => { events.push("sync"); }, @@ -213,7 +274,7 @@ describe("ocx v2 keep-native-v1", () => { expect(await cmdV2(["mode", "v2"], { execFile: (_file, args) => { - actions.push(args[1]!); + actions.push(featureActionOf(args).split(" ")[1]!); writeFileSync(codexConfig, readFileSync(codexConfig, "utf8").replace("enabled = true", "enabled = false")); }, sync: async () => {}, diff --git a/tests/native-codex-toggle.test.ts b/tests/native-codex-toggle.test.ts index bd15a3f2c5..86294ddce3 100644 --- a/tests/native-codex-toggle.test.ts +++ b/tests/native-codex-toggle.test.ts @@ -12,7 +12,7 @@ * act on — rather than artifacts the next start silently undoes. */ import { afterEach, beforeEach, describe, expect, test } from "bun:test"; -import { mkdtempSync, readFileSync, writeFileSync } from "node:fs"; +import { mkdirSync, mkdtempSync, readFileSync, realpathSync, writeFileSync } from "node:fs"; import { homedir, tmpdir } from "node:os"; import { join } from "node:path"; @@ -23,6 +23,8 @@ import { removeTreeWithRetry } from "./helpers/remove-tree"; let fixtureRoot = ""; let previousOpencodexHome: string | undefined; +let previousCodexHome: string | undefined; +let codexHome = ""; const cleanup: string[] = []; function baseConfig(): OcxConfig { @@ -72,9 +74,13 @@ function persistedCodexIntent(): unknown { beforeEach(() => { previousOpencodexHome = process.env.OPENCODEX_HOME; - fixtureRoot = mkdtempSync(join(tmpdir(), "ocx-codex-toggle-")); + previousCodexHome = process.env.CODEX_HOME; + fixtureRoot = realpathSync.native(mkdtempSync(join(tmpdir(), "ocx-codex-toggle-"))); + codexHome = join(fixtureRoot, "codex"); + mkdirSync(codexHome); cleanup.push(fixtureRoot); process.env.OPENCODEX_HOME = fixtureRoot; + process.env.CODEX_HOME = codexHome; writeFileSync(join(fixtureRoot, "config.json"), JSON.stringify(baseConfig(), null, 2)); writeFileSync(join(fixtureRoot, "service-state.json"), JSON.stringify({ version: 2, @@ -87,6 +93,8 @@ beforeEach(() => { afterEach(() => { if (previousOpencodexHome === undefined) delete process.env.OPENCODEX_HOME; else process.env.OPENCODEX_HOME = previousOpencodexHome; + if (previousCodexHome === undefined) delete process.env.CODEX_HOME; + else process.env.CODEX_HOME = previousCodexHome; while (cleanup.length) removeTreeWithRetry(cleanup.pop()!); }); diff --git a/tests/native-profile-manager.test.ts b/tests/native-profile-manager.test.ts index 13f26b359a..5029e546db 100644 --- a/tests/native-profile-manager.test.ts +++ b/tests/native-profile-manager.test.ts @@ -15,7 +15,7 @@ import { } from "../src/codex/native-profile-store"; import { NativeProfileError, type NativeProfileKey, type NativeProfileKeyProvider } from "../src/codex/native-profile-types"; import { codexCredentialMutationEpoch } from "../src/codex/credential-mutation-epoch"; -import { watchdogMs } from "./helpers/ci-watchdog"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "./helpers/test-budget"; import { removeTreeWithRetry } from "./helpers/remove-tree"; const roots: string[] = []; @@ -134,10 +134,11 @@ async function leavePendingJournal(f: Awaited * The first Bun child a busy windows-latest shard spawns can take several seconds just to * boot the TS helper; on run 33595585136 that alone burned a private 5 s wait while the * child was healthy. The crash case, which is the first spawn in the file, gets a wait - * scaled by the shared CI watchdog budget. On timeout the child's stderr is part of the - * error so a real crash is not mistaken for a slow start. + * sized inside its 15 s test budget. On timeout the child's stderr is part of the error so + * a real crash is not mistaken for a slow start. */ -async function waitForPath(path: string, child?: ReturnType, waitMs = 5_000): Promise { +// Gates on a spawned child reaching its marker: 8-19 s on windows-latest (run 33930757649). +async function waitForPath(path: string, child?: ReturnType, waitMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + waitMs; while (!existsSync(path) && Date.now() < deadline) await Bun.sleep(10); if (existsSync(path)) return; @@ -196,13 +197,12 @@ describe("native main profile transactions", () => { const f = fixture(); const readyPath = join(f.root, "crash-ready"); const child = spawnLockHolder(f, readyPath, join(f.root, "unused-release"), { crash: true }); - const markerWaitMs = watchdogMs(12_000); - await waitForPath(readyPath, child, markerWaitMs); + await waitForPath(readyPath, child, INTERNAL_DEADLINE_MS); expect(await child.exited).toBe(87); const successor = new NativeProfileManager({ ...f.options, lockWaitMs: 250 }); expect((await successor.recover(false)).recovered).toBe(false); - }, watchdogMs(12_000) + 3_000); + }, SPAWN_BUDGET_MS); test("a losing same-process contender cannot release another transaction's POSIX lock", async () => { if (process.platform === "win32") return; @@ -253,7 +253,7 @@ describe("native main profile transactions", () => { ...(acquiredProbe ? [acquiredProbe.exited] : []), ]); } - }, 15_000); + }, SPAWN_BUDGET_MS); test("two processes exclude each other and predecessor release cannot delete a successor lock", async () => { const f = fixture(); @@ -292,7 +292,7 @@ describe("native main profile transactions", () => { await first.exited; if (second) await second.exited; } - }, 15_000); + }, SPAWN_BUDGET_MS); test("the same canonical CODEX_HOME serializes different OpenCodex config roots", async () => { const f = fixture(); @@ -319,7 +319,7 @@ describe("native main profile transactions", () => { writeFileSync(release, "release"); await first.exited; } - }, 15_000); + }, SPAWN_BUDGET_MS); test("shares one vault while preventing another OPENCODEX_HOME from finishing or cancelling a stage", async () => { const f = fixture(); diff --git a/tests/native-profile-startup.test.ts b/tests/native-profile-startup.test.ts index d9e9123d13..aec792d61c 100644 --- a/tests/native-profile-startup.test.ts +++ b/tests/native-profile-startup.test.ts @@ -53,11 +53,15 @@ import { } from "../src/server/lifecycle"; import { startServer } from "../src/server"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "./helpers/test-budget"; const roots: string[] = []; const previousOpencodexHome = process.env.OPENCODEX_HOME; const previousCodexHome = process.env.CODEX_HOME; const OWNERSHIP_REPROBE_TEST_HOME = "ownership-reprobe-test-home"; +const CHILD_CASE_BUDGET_MS = 2 * SPAWN_BUDGET_MS; +type StartupChild = ReturnType; +const childOutputs = new WeakMap; stderr: Promise; startedAt: number; ready: boolean }>(); function restoreEnv(name: "OPENCODEX_HOME" | "CODEX_HOME", value: string | undefined): void { if (value === undefined) delete process.env[name]; @@ -225,7 +229,8 @@ async function fixture( return { root, codexHome, configDir, key, manager, target, sourceProfileId: sourceRecord.id, targetProfileId: targetRecord.id }; } -async function waitForPath(path: string, timeoutMs = 10_000): Promise { +// Gates on a spawned child reaching its marker: 8-19 s on windows-latest (run 33930757649). +async function waitForPath(path: string, timeoutMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + timeoutMs; while (!existsSync(path) && Date.now() < deadline) await Bun.sleep(10); if (!existsSync(path)) throw new Error(`Timed out waiting for ${path}`); @@ -238,15 +243,17 @@ async function waitForPath(path: string, timeoutMs = 10_000): Promise { * Wait for a port that is actually a port. */ // A spawned proxy child needs 10-18 s to reach its port file on a loaded windows-latest shard -// (runs 33601508392 and 33610501053); every caller here has a 20 s+ budget. -async function waitForPort(path: string, timeoutMs = 18_000): Promise { +// (runs 33601508392 and 33610501053), and run 33930757649 showed 19 s boots elsewhere in the +// suite. INTERNAL_DEADLINE_MS is the named in-test bound; callers carry a larger case budget. +async function waitForPort(path: string, child: StartupChild, timeoutMs = SPAWN_BUDGET_MS): Promise { const deadline = Date.now() + timeoutMs; for (;;) { + if (child.exitCode !== null) throw new Error(`startup child exited ${child.exitCode} before publishing ${path}\n${await childDiagnostic(child)}`); if (existsSync(path)) { const port = Number(readFileSync(path, "utf8").trim()); - if (Number.isInteger(port) && port > 0) return port; + if (Number.isInteger(port) && port > 0 && port <= 65_535) { childOutputs.get(child)!.ready = true; return port; } } - if (Date.now() >= deadline) throw new Error(`Timed out waiting for a real port in ${path}`); + if (Date.now() >= deadline) throw new Error(`Timed out waiting for a real port in ${path}; childExit=${child.exitCode}; elapsedMs=${Date.now() - childOutputs.get(child)!.startedAt}`); await Bun.sleep(10); } } @@ -261,8 +268,9 @@ function childPaths(f: Fixture) { }; } -function spawnChild(f: Fixture, paths: ReturnType): ReturnType { - return Bun.spawn([process.execPath, join(import.meta.dir, "helpers", "native-profile-startup-child.ts")], { +function spawnChild(f: Fixture, paths: ReturnType): StartupChild { + const startedAt = Date.now(); + const child = Bun.spawn([process.execPath, join(import.meta.dir, "helpers", "native-profile-startup-child.ts")], { cwd: join(import.meta.dir, ".."), env: { ...process.env, @@ -277,23 +285,45 @@ function spawnChild(f: Fixture, paths: ReturnType): ReturnTyp NATIVE_STARTUP_SETTLED: paths.settled, NATIVE_STARTUP_UPSTREAM: paths.upstream, NATIVE_STARTUP_STOP: paths.stop, + NATIVE_STARTUP_LAUNCHED_AT: String(startedAt), }, stdin: "ignore", stdout: "pipe", stderr: "pipe", }); + childOutputs.set(child, { stdout: new Response(child.stdout).text(), stderr: new Response(child.stderr).text(), startedAt, ready: false }); + return child; } -async function stopChild(child: ReturnType, paths: ReturnType): Promise { +async function childDiagnostic(child: StartupChild): Promise { + const output = childOutputs.get(child)!; + const [stdout, stderr] = await Promise.all([output.stdout, output.stderr]); + return `stdout=${stdout.slice(-8192)}\nstderr=${stderr.slice(-8192)}`; +} + +async function stopChild(child: StartupChild, paths: ReturnType): Promise { writeFileSync(paths.release, "release"); writeFileSync(paths.stop, "stop"); - const exit = await Promise.race([child.exited, Bun.sleep(10_000).then(() => null)]); - if (exit === null) { - child.kill(); - await child.exited; - throw new Error("startup child did not stop"); + let timer: ReturnType | undefined; + try { + const exit = await Promise.race([child.exited, new Promise(resolve => { timer = setTimeout(() => resolve(null), 10_000); })]); + if (exit === null) { + child.kill(); + await child.exited; + throw new Error(`startup child did not stop; killed and joined\n${await childDiagnostic(child)}`); + } + if (exit !== 0) throw new Error(`startup child exited ${exit}\n${await childDiagnostic(child)}`); + } finally { + if (timer !== undefined) clearTimeout(timer); } - if (exit !== 0) throw new Error(await new Response(child.stderr).text()); +} + +async function withStartupChild(f: Fixture, verify: (port: number, paths: ReturnType) => Promise): Promise { + const paths = childPaths(f); const child = spawnChild(f, paths); const errors: unknown[] = []; + try { await verify(await waitForPort(paths.port, child), paths); } catch (error) { errors.push(error); } + try { await stopChild(child, paths); } catch (error) { errors.push(error); } + if (errors.length === 1) throw errors[0]; + if (errors.length > 1) throw new AggregateError(errors); } async function mainRequest(port: number): Promise { @@ -576,13 +606,9 @@ describe("native-main startup journal gate", () => { expect(isNativeMainTrafficBlocked()).toBe(false); }); - test("fresh processes gate first admission and converge every recoverable phase/observation", async () => { - for (const scenario of recoverable) { + test.each(recoverable)("fresh processes gate first admission and converge $phase/$observation", async (scenario) => { const f = await fixture(scenario.phase, scenario.observation); - const paths = childPaths(f); - const child = spawnChild(f, paths); - try { - const port = await waitForPort(paths.port); + await withStartupChild(f, async (port, paths) => { const blocked = await mainRequest(port); expect(blocked.status).toBeGreaterThanOrEqual(400); expect(existsSync(paths.upstream)).toBe(false); @@ -597,19 +623,12 @@ describe("native-main startup journal gate", () => { expect(existsSync(paths.upstream)).toBe(true); const active = (await f.manager.list()).activeProfileId; expect(active).toBe(scenario.active === "target" ? f.targetProfileId : f.sourceProfileId); - } finally { - await stopChild(child, paths); - } - } - }, 120_000); + }); + }, CHILD_CASE_BUDGET_MS); - test("manual observations keep main closed while health and explicit recovery remain available", async () => { - for (const observation of ["unreadable", "third"] as const) { + test.each(["unreadable", "third"] as const)("manual observation %s keeps main closed while health and explicit recovery remain available", async (observation) => { const f = await fixture("prepared", observation); - const paths = childPaths(f); - const child = spawnChild(f, paths); - try { - const port = await waitForPort(paths.port); + await withStartupChild(f, async (port, paths) => { expect((await mainRequest(port)).status).toBeGreaterThanOrEqual(400); expect(existsSync(paths.upstream)).toBe(false); expect((await fetch(`http://127.0.0.1:${port}/healthz`)).status).toBe(200); @@ -629,26 +648,18 @@ describe("native-main startup journal gate", () => { expect(recovered.status).toBe(200); expect((await mainRequest(port)).status).toBe(200); expect(existsSync(paths.upstream)).toBe(true); - } finally { - await stopChild(child, paths); - } - } - }, 45_000); + }); + }, CHILD_CASE_BUDGET_MS); test("a pending native-main journal does not block an ordinary Pool account", async () => { const f = await fixture("prepared", "unreadable", true); - const paths = childPaths(f); - const child = spawnChild(f, paths); - try { - const port = await waitForPort(paths.port); + await withStartupChild(f, async (port, paths) => { expect((await mainRequest(port)).status).toBe(200); await waitForPath(paths.upstream); const receipt = JSON.parse(readFileSync(paths.upstream, "utf8").trim()); expect(receipt.authorization).toBe("Bearer pool-access"); - } finally { - await stopChild(child, paths); - } - }, 20_000); + }); + }, CHILD_CASE_BUDGET_MS); }); /* diff --git a/tests/oauth-refresh-lock-multiprocess.test.ts b/tests/oauth-refresh-lock-multiprocess.test.ts index 929c7955ef..7c9aa1d6ce 100644 --- a/tests/oauth-refresh-lock-multiprocess.test.ts +++ b/tests/oauth-refresh-lock-multiprocess.test.ts @@ -15,6 +15,7 @@ import { saveCredential, } from "../src/oauth/store"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS, SPAWN_BUDGET_MS } from "./helpers/test-budget"; const repoRoot = dirname(fileURLToPath(new URL("../package.json", import.meta.url))); const origHome = process.env.HOME; @@ -92,7 +93,8 @@ describe("slow multi-process OAuth refresh lock", () => { stderr: "pipe", }); - const deadline = Date.now() + 15_000; + // Spawned child reaching its ready marker: 8-19 s on windows-latest (run 33930757649). + const deadline = Date.now() + INTERNAL_DEADLINE_MS; while (!existsSync(readyPath) && Date.now() < deadline) { await Bun.sleep(25); } @@ -127,5 +129,5 @@ describe("slow multi-process OAuth refresh lock", () => { expect(writerExit).toBe(0); const writerOut = await new Response(writer.stdout).text(); expect(writerOut).toContain("writer-done"); - }, 30_000); + }, SPAWN_BUDGET_MS); }); diff --git a/tests/server-background-lifecycle.test.ts b/tests/server-background-lifecycle.test.ts index 88d1d52f7e..fba842c4b9 100644 --- a/tests/server-background-lifecycle.test.ts +++ b/tests/server-background-lifecycle.test.ts @@ -33,7 +33,7 @@ import { liveStorageWorkerCount, } from "../src/storage/worker-lifecycle"; import type { OcxConfig } from "../src/types"; -import { SERVER_BUDGET_MS } from "./helpers/test-budget"; +import { INTERNAL_DEADLINE_MS, SERVER_BUDGET_MS } from "./helpers/test-budget"; import { managementFetch } from "./helpers/management-auth"; import { installIsolatedCodexHome, @@ -240,7 +240,8 @@ function seedArchived(codexHome: string): void { db.close(); } -async function waitForLiveStorageWorker(timeoutMs = 10_000): Promise { +// Worker spawn behind a live server on a loaded windows-latest shard; platform floor. +async function waitForLiveStorageWorker(timeoutMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (liveStorageWorkerCount() > 0) return; diff --git a/tests/storage-mutation-race.test.ts b/tests/storage-mutation-race.test.ts index 91b4a4b60a..b37cc12f1f 100644 --- a/tests/storage-mutation-race.test.ts +++ b/tests/storage-mutation-race.test.ts @@ -45,6 +45,7 @@ import { drainStorageWorkers, } from "../src/storage/worker-lifecycle"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; +import { INTERNAL_DEADLINE_MS } from "./helpers/test-budget"; let testDir = ""; let previousHome: string | undefined; @@ -126,7 +127,8 @@ async function enablePolicyAndRun(serverUrl: string): Promise<{ startedAt: numbe async function waitForPolicyJob( serverUrl: string, startedAt: number, - timeoutMs = 20_000, + // Same wait as helpers/storage-policy-api waitForJobIdle: live server, worker-backed job. + timeoutMs = INTERNAL_DEADLINE_MS, ): Promise<{ job: { lastOutcome?: { ok?: boolean; error?: string; removed?: number } } }> { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { diff --git a/tests/storage-policy-job-responsive.test.ts b/tests/storage-policy-job-responsive.test.ts index 4de7739ba0..7d48544f32 100644 --- a/tests/storage-policy-job-responsive.test.ts +++ b/tests/storage-policy-job-responsive.test.ts @@ -20,6 +20,7 @@ import { import { stopStorageCleanupScheduler } from "../src/storage/policy-scheduler"; import { drainStorageWorkers } from "../src/storage/worker-lifecycle"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS } from "./helpers/test-budget"; let testDir = ""; let previousHome: string | undefined; @@ -137,13 +138,18 @@ describe("storage cleanup policy job responsiveness", () => { expect(sample).toBeLessThan(maxHealthMs); } - const deadline = Date.now() + 10_000; + // Polls a live server whose worker is deliberately blocked. Review of 3b431b413 found + // this loop fell through on expiry with no assertion, so a job that never returned to + // idle still passed; the expect below is what makes the wait mean something. + const deadline = Date.now() + INTERNAL_DEADLINE_MS; + let settled = false; while (Date.now() < deadline) { const got = await fetch(new URL("/api/storage/cleanup-policy", server.url)); const body = await got.json() as { job: { status: string; startedAt?: number } }; - if (body.job.status === "idle" && body.job.startedAt === runBody.job?.startedAt) break; + if (body.job.status === "idle" && body.job.startedAt === runBody.job?.startedAt) { settled = true; break; } await Bun.sleep(10); } + expect(settled).toBe(true); } finally { await drainAndShutdown(server, 5_000); await resetStorageCleanupPolicyJobForTestsAsync(); diff --git a/tests/storage-worker-lifecycle.test.ts b/tests/storage-worker-lifecycle.test.ts index 1462b67335..774aeac85e 100644 --- a/tests/storage-worker-lifecycle.test.ts +++ b/tests/storage-worker-lifecycle.test.ts @@ -34,6 +34,7 @@ import { } from "../src/storage/worker-lifecycle"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS } from "./helpers/test-budget"; let isolatedCodexHome: IsolatedCodexHome | null = null; let testDir = ""; @@ -67,7 +68,9 @@ afterEach(async () => { testDir = ""; }); -async function waitForIdle(timeoutMs = 20_000): Promise { +// Worker-thread lifecycle: Windows OS-thread join is the slow half (see +// src/storage/worker-lifecycle.ts), so the bound follows the platform floor. +async function waitForIdle(timeoutMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (getStorageCleanupPolicyJobState().status === "idle") return; @@ -77,7 +80,7 @@ async function waitForIdle(timeoutMs = 20_000): Promise { } /** Guards against a vacuous pass: assert we really did spawn a worker. */ -async function waitForLiveWorker(timeoutMs = 10_000): Promise { +async function waitForLiveWorker(timeoutMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (liveStorageWorkerCount() > 0) return; diff --git a/tests/storage-worker-teardown-isolate.test.ts b/tests/storage-worker-teardown-isolate.test.ts index a516c6442c..0f6783fa82 100644 --- a/tests/storage-worker-teardown-isolate.test.ts +++ b/tests/storage-worker-teardown-isolate.test.ts @@ -36,6 +36,7 @@ import { } from "../src/storage/worker-lifecycle"; import { installIsolatedCodexHome, type IsolatedCodexHome } from "./helpers/isolated-codex-home"; import { removeTreeWithRetry } from "./helpers/remove-tree"; +import { INTERNAL_DEADLINE_MS } from "./helpers/test-budget"; let isolatedCodexHome: IsolatedCodexHome | null = null; let testDir = ""; @@ -92,7 +93,8 @@ afterAll(async () => { await drainStorageWorkers(); }); -async function waitForLiveWorker(timeoutMs = 10_000): Promise { +// Worker spawn on a loaded windows-latest shard; bound follows the platform floor. +async function waitForLiveWorker(timeoutMs = INTERNAL_DEADLINE_MS): Promise { const deadline = Date.now() + timeoutMs; while (Date.now() < deadline) { if (liveStorageWorkerCount() > 0) return;