From cf3443796ce23fba3f7495b64a2cde2a8cd5afd7 Mon Sep 17 00:00:00 2001 From: Vincent Koc <25068+vincentkoc@users.noreply.github.com> Date: Fri, 10 Jul 2026 20:48:28 -0700 Subject: [PATCH] fix(testbox): sync source on every lease reuse --- docs/reference/test.md | 7 +++--- scripts/crabbox-wrapper.mjs | 16 -------------- scripts/testbox-lease-freshness.mjs | 16 +++----------- test/scripts/testbox-lease-freshness.test.ts | 23 +------------------- 4 files changed, 7 insertions(+), 55 deletions(-) diff --git a/docs/reference/test.md b/docs/reference/test.md index 46ed426089d6..b44abcda821f 100644 --- a/docs/reference/test.md +++ b/docs/reference/test.md @@ -28,10 +28,9 @@ After the first successful reuse, the wrapper records the lease's base, dependency, and Testbox workflow fingerprint under `.crabbox/testbox-leases/`. Source-only edits keep reusing the warmed box. A changed merge base, lockfile, package-manager input, wrapper, or Testbox workflow fails closed and requires a -fresh lease. `OPENCLAW_TESTBOX_ALLOW_STALE=1` is only for intentional -diagnostics, not release proof. Repeating a targeted run from the exact clean -HEAD that already succeeded skips source sync; changed gates, a new commit, or -a dirty edit sync normally. +fresh lease. Every run still syncs the current checkout. +`OPENCLAW_TESTBOX_ALLOW_STALE=1` is only for intentional diagnostics, not +release proof. Local test commands below are for human workflows or an explicit agent fallback requested by the user. Remote-provider unavailability must be reported; it is diff --git a/scripts/crabbox-wrapper.mjs b/scripts/crabbox-wrapper.mjs index 1ec4cda43309..b73310ed0edf 100755 --- a/scripts/crabbox-wrapper.mjs +++ b/scripts/crabbox-wrapper.mjs @@ -3184,16 +3184,6 @@ function injectFullCheckoutLeaseReclaim(commandArgs) { return normalizedArgs; } -function injectReusableTestboxNoSync(commandArgs) { - if (hasOption(commandArgs, "--no-sync")) { - return commandArgs; - } - const normalizedArgs = [...commandArgs]; - const { optionEnd } = runCommandBounds(normalizedArgs); - normalizedArgs.splice(optionEnd, 0, "--no-sync"); - return normalizedArgs; -} - function injectRemoteTestboxCi(commandArgs, providerName) { if (commandArgs[0] !== "run" || canonicalProviderName(providerName) !== "blacksmith-testbox") { return commandArgs; @@ -3299,12 +3289,6 @@ try { provider: canonicalProvider, repoRoot, }); - if (testboxLeaseFreshness?.skipSync && !isChangedGateCommand(runCommandArgs(normalizedArgs))) { - normalizedArgs = injectReusableTestboxNoSync(normalizedArgs); - console.error( - `[crabbox] Testbox ${optionValue(normalizedArgs, "--id")} already has clean HEAD ${testboxLeaseFreshness.current.headSha}; skipping source sync`, - ); - } } catch (error) { console.error(`[crabbox] ${error instanceof Error ? error.message : String(error)}`); process.exit(2); diff --git a/scripts/testbox-lease-freshness.mjs b/scripts/testbox-lease-freshness.mjs index 22e1da91ba8f..dfa2a1c6c968 100644 --- a/scripts/testbox-lease-freshness.mjs +++ b/scripts/testbox-lease-freshness.mjs @@ -86,12 +86,6 @@ export function buildTestboxLeaseFingerprint(repoRoot, args) { }; } -export function testboxLeaseCanSkipSync(saved, current) { - return Boolean( - saved?.syncedCleanHead && current.workingTreeClean && saved.syncedCleanHead === current.headSha, - ); -} - export function testboxLeaseStaleReasons(saved, current) { if (!saved || saved.version !== STATE_VERSION) { return ["state schema"]; @@ -121,9 +115,9 @@ export function prepareTestboxLeaseFreshness({ args, env, provider, repoRoot }) `Testbox ${id} is stale (${staleReasons.join(", ")}); stop it and warm a fresh lease, or set OPENCLAW_TESTBOX_ALLOW_STALE=1 for an intentional diagnostic reuse`, ); } - return { current, path, skipSync: testboxLeaseCanSkipSync(saved, current) }; + return { current, path }; } - return { current, path, skipSync: false }; + return { current, path }; } export function recordTestboxLeaseFreshness(prepared) { @@ -132,10 +126,6 @@ export function recordTestboxLeaseFreshness(prepared) { } mkdirSync(resolve(prepared.path, ".."), { recursive: true }); const temporaryPath = `${prepared.path}.tmp-${process.pid}`; - const state = { - ...prepared.current, - syncedCleanHead: prepared.current.workingTreeClean ? prepared.current.headSha : "", - }; - writeFileSync(temporaryPath, `${JSON.stringify(state, null, 2)}\n`); + writeFileSync(temporaryPath, `${JSON.stringify(prepared.current, null, 2)}\n`); renameSync(temporaryPath, prepared.path); } diff --git a/test/scripts/testbox-lease-freshness.test.ts b/test/scripts/testbox-lease-freshness.test.ts index fe5cc9cc107e..12574b671cef 100644 --- a/test/scripts/testbox-lease-freshness.test.ts +++ b/test/scripts/testbox-lease-freshness.test.ts @@ -1,8 +1,5 @@ import { describe, expect, it } from "vitest"; -import { - testboxLeaseCanSkipSync, - testboxLeaseStaleReasons, -} from "../../scripts/testbox-lease-freshness.mjs"; +import { testboxLeaseStaleReasons } from "../../scripts/testbox-lease-freshness.mjs"; const fingerprint = { version: 1, @@ -21,24 +18,6 @@ describe("Testbox lease freshness", () => { expect(testboxLeaseStaleReasons(fingerprint, { ...fingerprint })).toEqual([]); }); - it("skips sync only for the exact clean HEAD already proven on the lease", () => { - expect( - testboxLeaseCanSkipSync( - { ...fingerprint, syncedCleanHead: fingerprint.headSha }, - fingerprint, - ), - ).toBe(true); - expect( - testboxLeaseCanSkipSync( - { ...fingerprint, syncedCleanHead: fingerprint.headSha }, - { ...fingerprint, workingTreeClean: false }, - ), - ).toBe(false); - expect( - testboxLeaseCanSkipSync({ ...fingerprint, syncedCleanHead: "e".repeat(40) }, fingerprint), - ).toBe(false); - }); - it("rotates a lease when base, dependency, or workflow inputs drift", () => { expect( testboxLeaseStaleReasons(fingerprint, {