fix(ci): scope the docker cleanup diagnostics to what was probed (#12006

) Address review feedback on the lock-dir fallback: - The prune step blanked a job-private resolver result but still logged "skipped because the shared daemon is active", and its resolver-absent inline probe checked only the directory, so a root-owned 0400 lock file failed `exec 9>` with EACCES inside the if-condition (no bash -e abort) and the step exited 0 with that false line as its only output. The step now probes the lock file inline, carries the cause it actually observed (naming the lock dir or file), and announces the unconditional dangling prune separately so "skipped" scopes to the labelled one. Its resolver call blanks GITHUB_STEP_SUMMARY: the step discards a job-private result and opens no lock in it, so the fallback banner would lie there (the leg's calls keep it). - The resolver's fallback warning and summary claimed "docker build/prune coordination is lost" even when only a build-family lock fell back; both now scope the loss to the locks the call actually probed ($*). - Tests: pin the unwritable-dir cause text at both reach points (and fix the comment that claimed it was pinned), add the success-direction witness driving the resolver and the step's shared-dir guard together so a one-token drift between them reddens, and assert the opened-locks == probed-locks invariant as a derived set so a lock added to an exec line without a probe-list entry fails the suite.
qwen-code-dev-bot · Sep 16, 2026 · Sep 16, 2026 · Sep 16, 2026 · Sep 16, 2026 · Sep 17, 2026
commit d75b0bb00b8a59a0171e1442cbed16de57d0abed
diff --git a/.github/scripts/resolve-ci-lock-dir.sh b/.github/scripts/resolve-ci-lock-dir.sh
@@ -59,12 +59,12 @@ if ! mkdir -p "${fallback}"; then
   echo "::error::${problem}, and job-private fallback ${fallback} cannot be created; the docker sandbox locks cannot be opened" >&2
   exit 1
 fi
-echo "::warning::${problem} — using job-private lock dir ${fallback}; docker build/prune coordination on this host is lost for this job" >&2
+echo "::warning::${problem} — using job-private lock dir ${fallback} for: $*; cross-job coordination on these locks is lost for this job" >&2
 if [ -n "${GITHUB_STEP_SUMMARY:-}" ]; then
   {
     echo '### ⚠️ Docker lock-dir fallback active'
     echo
-    echo "${problem} — this job locks in the job-private \`${fallback}\`. Coordination with sibling jobs, the root \`qwen-docker-cleanup\` timer, and the release build lane is **lost** for this job: a prune can race in-flight docker work, and \`until=24h\` does not protect a reused sandbox image. The shared dir does not heal itself — clearing \`${primary}\` needs a human on the host."
+    echo "${problem} — this job locks in the job-private \`${fallback}\` for: $*. Cross-job coordination on these locks is **lost** for this job. The shared dir does not heal itself — clearing \`${primary}\` needs a human on the host."
   } >>"${GITHUB_STEP_SUMMARY}" || true
 fi
 printf '%s\n' "${fallback}"
diff --git a/.github/workflows/e2e.yml b/.github/workflows/e2e.yml
@@ -334,18 +334,40 @@ jobs:
           # lock lands on the shared dir: taken in a job-private dir it
           # excludes nothing (no other process opens that file), so the
           # prune would race sibling legs' in-flight docker work.
-          ci_lock_dir="$(bash .github/scripts/resolve-ci-lock-dir.sh docker-sandbox-daemon.lock)" || {
+          # GITHUB_STEP_SUMMARY= blanks the resolver's banner for this one
+          # caller: this step discards a job-private result and opens no
+          # lock in it, so the banner's "this job locks in <dir>" would lie.
+          ci_lock_dir="$(GITHUB_STEP_SUMMARY= bash .github/scripts/resolve-ci-lock-dir.sh docker-sandbox-daemon.lock)" || {
             ci_lock_dir="${HOME}/.cache/qwen-code-ci"
-            mkdir -p "${ci_lock_dir}" && [ -w "${ci_lock_dir}" ] || ci_lock_dir=''
+            daemon_lock="${ci_lock_dir}/docker-sandbox-daemon.lock"
+            if mkdir -p "${ci_lock_dir}" && [ -w "${ci_lock_dir}" ]; then
+              # Probe the lock FILE, not only the dir: a root-owned 0400
+              # leftover fails `exec 9>` with EACCES inside the if below,
+              # which bash -e does not abort on.
+              if [ -e "${daemon_lock}" ] && [ ! -w "${daemon_lock}" ]; then
+                skip_reason="${daemon_lock} is not writable by this job"
+                ci_lock_dir=''
+              fi
+            else
+              skip_reason="cannot create a writable ${ci_lock_dir}"
+              ci_lock_dir=''
+            fi
           }
-          [ "${ci_lock_dir}" = "${HOME}/.cache/qwen-code-ci" ] || ci_lock_dir=''
-          if [ -n "${ci_lock_dir}" ] && exec 9>"${ci_lock_dir}/docker-sandbox-daemon.lock" && flock --nonblock 9; then
+          if [ -n "${ci_lock_dir}" ] && [ "${ci_lock_dir}" != "${HOME}/.cache/qwen-code-ci" ]; then
+            skip_reason="the shared lock dir ${HOME}/.cache/qwen-code-ci is not usable by this job"
+            ci_lock_dir=''
+          fi
+          if [ -z "${ci_lock_dir}" ]; then
+            echo "Docker cleanup skipped: ${skip_reason}; clearing ${HOME}/.cache/qwen-code-ci needs a human on this host"
+          elif exec 9>"${ci_lock_dir}/docker-sandbox-daemon.lock" && flock --nonblock 9; then
             docker image prune --all --force --filter 'label=org.qwen-code.ci.sandbox=true' --filter 'until=24h' 9>&- || echo "::warning::old CI sandbox image cleanup failed on ${RUNNER_NAME:-this runner}"
           else
             echo "Docker cleanup skipped because the shared daemon is active"
           fi
           # The dangling prune removes only untagged, unreferenced images,
-          # so it needs no daemon lock and stays unconditional.
+          # so it needs no daemon lock and stays unconditional — announced
+          # separately so a "skipped" line above scopes to the labelled one.
+          echo "Running the unconditional dangling prune"
           docker image prune --force --filter 'until=24h' 9>&- || echo "::warning::docker image prune failed on ${RUNNER_NAME:-this runner}; dangling images may accumulate"
 
   e2e-test-macos:

diff --git a/scripts/tests/e2e-workflow.test.js b/scripts/tests/e2e-workflow.test.js
@@ -381,8 +381,12 @@ describe('e2e workflow', () => {
       expect(e2eRunScript).toContain(
         'ci_lock_dir="$(bash .github/scripts/resolve-ci-lock-dir.sh docker-sandbox-daemon.lock)"',
       );
+      // The prune step discards a job-private resolver result and opens no
+      // lock in it, so it blanks GITHUB_STEP_SUMMARY: the resolver's
+      // fallback banner would claim a lock dir this step never uses. The
+      // leg's calls keep the banner.
       expect(prune.run).toContain(
-        'ci_lock_dir="$(bash .github/scripts/resolve-ci-lock-dir.sh docker-sandbox-daemon.lock)"',
+        'ci_lock_dir="$(GITHUB_STEP_SUMMARY= bash .github/scripts/resolve-ci-lock-dir.sh docker-sandbox-daemon.lock)"',
       );
       // Both sites open the daemon lock through the resolved dir, so one
       // contract change cannot drift them onto different files.
@@ -405,7 +409,7 @@ describe('e2e workflow', () => {
       // docker work. The guard is for this step only — the test leg keeps
       // failing loudly, because there the leg genuinely cannot run.
       expect(prune.run).toContain(
-        'ci_lock_dir="$(bash .github/scripts/resolve-ci-lock-dir.sh docker-sandbox-daemon.lock)" || {',
+        'ci_lock_dir="$(GITHUB_STEP_SUMMARY= bash .github/scripts/resolve-ci-lock-dir.sh docker-sandbox-daemon.lock)" || {',
       );
       expect(prune.run).toContain('ci_lock_dir="${HOME}/.cache/qwen-code-ci"');
       expect(prune.run).toContain('mkdir -p "${ci_lock_dir}"');
@@ -420,10 +424,10 @@ describe('e2e workflow', () => {
       // dangling images are untagged and unreferenced, so no running leg
       // can lose its image to it.
       expect(prune.run).toContain(
-        '[ "${ci_lock_dir}" = "${HOME}/.cache/qwen-code-ci" ] || ci_lock_dir=',
+        '[ "${ci_lock_dir}" != "${HOME}/.cache/qwen-code-ci" ]',
       );
       expect(prune.run).toContain(
-        'if [ -n "${ci_lock_dir}" ] && exec 9>"${ci_lock_dir}/docker-sandbox-daemon.lock" && flock --nonblock 9; then',
+        'elif exec 9>"${ci_lock_dir}/docker-sandbox-daemon.lock" && flock --nonblock 9; then',
       );
     });
 
@@ -442,6 +446,43 @@ describe('e2e workflow', () => {
       );
     });
 
+    it('probes exactly the lock files each call site opens', () => {
+      // The string pins above and the bash cases both key on lock names
+      // they already know, so the probe list can drift away from the
+      // `exec N>` lines it covers with every gate green — and an unprobed
+      // lock then dies with EACCES on a poisoned host, the #12006
+      // signature. Deriving both sets from the same file turns the drift
+      // into a set difference. The per-commit coordinator name keeps
+      // ${GITHUB_SHA} unexpanded on both sides, so this stays a pure
+      // string-set comparison.
+      const openedLocks = (script, dirVar) =>
+        [
+          ...script.matchAll(
+            new RegExp(`exec \\d+>"\\$\\{${dirVar}\\}/([^"]+)"`, 'g'),
+          ),
+        ].map((m) => m[1]);
+      const probedLocks = (script, dirVar) => {
+        const call = script.match(
+          new RegExp(
+            `${dirVar}="\\$\\([^)]*resolve-ci-lock-dir\\.sh([^)]*)\\)"`,
+          ),
+        );
+        expect(call, dirVar).not.toBeNull();
+        return [...call[1].matchAll(/"([^"]+)"|([^\s"]+)/g)].map(
+          (m) => m[1] ?? m[2],
+        );
+      };
+      for (const [script, dirVar] of [
+        [e2eRunScript, 'ci_lock_dir'],
+        [e2eRunScript, 'ci_build_lock_dir'],
+        [prune.run, 'ci_lock_dir'],
+      ]) {
+        expect(openedLocks(script, dirVar).sort(), dirVar).toEqual(
+          probedLocks(script, dirVar).sort(),
+        );
+      }
+    });
+
     it('resolves the lock dir before any descriptor is opened', () => {
       const resolveIndex = e2eRunScript.indexOf(
         'bash .github/scripts/resolve-ci-lock-dir.sh',

diff --git a/scripts/tests/resolve-ci-lock-dir.test.js b/scripts/tests/resolve-ci-lock-dir.test.js
@@ -132,9 +132,9 @@ describe('CI docker lock dir resolution', () => {
 
   // The `cannot create` cause is the only probe message that distinguishes
   // an unwritable $HOME, a .cache that is a regular file, ENOSPC and EROFS
-  // from the unwritable-dir and unwritable-lock causes pinned above — and
-  // it is the only signal an operator gets on a pool host, so the message
-  // itself is pinned, not just the branch.
+  // from the unwritable-lock cause pinned above and the unwritable-dir
+  // cause pinned below — and it is the only signal an operator gets on a
+  // pool host, so the message itself is pinned, not just the branch.
   it('names the cause when the shared dir cannot be created', () => {
     const world = makeWorld();
     try {
@@ -186,6 +186,11 @@ describe('CI docker lock dir resolution', () => {
         expect(result.stdout.trim()).toBe(
           join(world.runnerTemp, 'qwen-code-ci-locks'),
         );
+        // Pin the cause text, not only the branch: on a pool host the
+        // ::warning:: is the only signal separating an ownership problem
+        // from ENOSPC/EROFS, and the summary body interpolates the primary
+        // path regardless of the cause, so it cannot carry this pin.
+        expect(result.stderr).toContain('is not writable');
       } finally {
         rmSync(world.dir, { recursive: true, force: true });
       }
@@ -250,13 +255,17 @@ describe('CI docker lock dir resolution', () => {
         // silent death again, now on a directory that does not exist.
         expect(result.stdout.trim()).toBe('');
         expect(result.stderr).toContain('::error::');
+        expect(result.stderr).toContain('is not writable');
       } finally {
         rmSync(world.dir, { recursive: true, force: true });
       }
     },
   );
 
-  function runDockerLeg(world, { imagePresent = true, home, runnerTemp } = {}) {
+  function runDockerLeg(
+    world,
+    { imagePresent = true, home, runnerTemp, extraEnv = {} } = {},
+  ) {
     const dockerStub = join(world.bin, 'docker');
     writeFileSync(
       dockerStub,
@@ -298,6 +307,7 @@ describe('CI docker lock dir resolution', () => {
         RUNNER_ENVIRONMENT: 'self-hosted',
         GITHUB_SHA: 'testsha12006',
         E2E_CONTAINER_OWNER: 'test-owner',
+        ...extraEnv,
       },
       encoding: 'utf8',
     });
@@ -406,7 +416,12 @@ describe('CI docker lock dir resolution', () => {
         const buildLock = join(shared, 'docker-sandbox-build.lock');
         writeFileSync(buildLock, '');
         chmodSync(buildLock, 0o400);
-        const { exitCode } = runDockerLeg(world, { imagePresent: false });
+        const summary = join(world.dir, 'summary.md');
+        writeFileSync(summary, '');
+        const { exitCode, output } = runDockerLeg(world, {
+          imagePresent: false,
+          extraEnv: { GITHUB_STEP_SUMMARY: summary },
+        });
         expect(exitCode).toBe(0);
         expect(existsSync(join(shared, 'docker-sandbox-daemon.lock'))).toBe(
           true,
@@ -421,6 +436,15 @@ describe('CI docker lock dir resolution', () => {
         ]) {
           expect(existsSync(join(fallback, name)), name).toBe(true);
         }
+        // Only the build mutex degraded here — the daemon lock stayed
+        // shared — so neither the warning nor the run-page summary may
+        // claim the prune coordination is lost. They must name the locks
+        // this call actually probed instead.
+        expect(output).toContain('docker-sandbox-build.lock');
+        expect(output).not.toMatch(/prune/);
+        const content = readFileSync(summary, 'utf8');
+        expect(content).toContain('docker-sandbox-build.lock');
+        expect(content).not.toMatch(/prune/);
       } finally {
         rmSync(world.dir, { recursive: true, force: true });
       }
@@ -462,7 +486,7 @@ describe('CI docker lock dir resolution', () => {
       (step) => step.name === 'Prune dangling docker images',
     ).run;
 
-    function runPruneStep(world, { withResolver }) {
+    function runPruneStep(world, { withResolver, extraEnv = {} }) {
       writeFileSync(
         join(world.bin, 'docker'),
         [
@@ -497,6 +521,7 @@ describe('CI docker lock dir resolution', () => {
           HOME: world.home,
           RUNNER_TEMP: world.runnerTemp,
           DOCKER_LOG: dockerLog,
+          ...extraEnv,
         },
         encoding: 'utf8',
       });
@@ -509,6 +534,29 @@ describe('CI docker lock dir resolution', () => {
       };
     }
 
+    it('locks the shared dir and prunes when the resolver is healthy', () => {
+      const world = makeWorld();
+      try {
+        const { exitCode, dockerArgv } = runPruneStep(world, {
+          withResolver: true,
+        });
+        expect(exitCode).toBe(0);
+        // The production path, driven end to end: the resolver prints the
+        // shared primary, the step's equality guard accepts it, and the
+        // labelled prune runs under the exclusive daemon lock. A one-token
+        // drift between the resolver's `primary` and the literal the step
+        // re-spells would turn the prune into a permanent no-op with every
+        // other gate green — only driving the two sides together sees it.
+        const shared = join(world.home, '.cache', 'qwen-code-ci');
+        expect(existsSync(join(shared, 'docker-sandbox-daemon.lock'))).toBe(
+          true,
+        );
+        expect(dockerArgv).toContain('image prune --all');
+      } finally {
+        rmSync(world.dir, { recursive: true, force: true });
+      }
+    });
+
     it('locks the shared dir and prunes when the resolver itself cannot run', () => {
       const world = makeWorld();
       try {
@@ -538,15 +586,81 @@ describe('CI docker lock dir resolution', () => {
             withResolver: true,
           });
           expect(exitCode).toBe(0);
-          expect(output).toContain('Docker cleanup skipped');
-          // The resolver returns the job-private fallback here; only the
-          // dangling prune (which needs no exclusion) may still run.
+          // The step discards the resolver's job-private fallback, so its
+          // own skip line must name the lock dir it could not use and point
+          // at the human cleanup — never assert the "daemon is active"
+          // cause no probe verified.
+          const skipLine = output
+            .split('\n')
+            .find((line) => line.includes('Docker cleanup skipped'));
+          expect(skipLine).toContain(shared);
+          expect(skipLine).toContain('needs a human');
+          expect(output).not.toContain('shared daemon is active');
+          // Only the dangling prune (which needs no exclusion) may run.
+          expect(dockerArgv).not.toContain('image prune --all');
+          expect(dockerArgv).toContain('image prune --force');
+        } finally {
+          rmSync(world.dir, { recursive: true, force: true });
+        }
+      },
+    );
+
+    // The resolver-absent arm of #12006: Checkout failed, the shared dir is
+    // writable, but the daemon lock itself is a root-owned 0400 leftover.
+    // The inline probe must see the lock FILE — probing only the dir lets
+    // `exec 9>` fail with EACCES inside the if-condition, which bash -e
+    // does not abort on, and the step then reports the daemon as active.
+    it.skipIf(isRoot)(
+      'names the unwritable daemon lock when the resolver cannot run',
+      () => {
+        const world = makeWorld();
+        try {
+          const shared = join(world.home, '.cache', 'qwen-code-ci');
+          mkdirSync(shared, { recursive: true });
+          const daemonLock = join(shared, 'docker-sandbox-daemon.lock');
+          writeFileSync(daemonLock, '');
+          chmodSync(daemonLock, 0o400);
+          const { exitCode, output, dockerArgv } = runPruneStep(world, {
+            withResolver: false,
+          });
+          expect(exitCode).toBe(0);
+          expect(output).toContain('docker-sandbox-daemon.lock');
+          expect(output).not.toContain('shared daemon is active');
           expect(dockerArgv).not.toContain('image prune --all');
           expect(dockerArgv).toContain('image prune --force');
         } finally {
           rmSync(world.dir, { recursive: true, force: true });
         }
       },
     );
+
+    // This step discards a job-private resolver result and opens no lock in
+    // it, so the resolver's banner ("this job locks in the job-private
+    // <dir>") would describe a lock this job never took. The call site
+    // blanks GITHUB_STEP_SUMMARY to suppress it; the leg's calls keep it.
+    it.skipIf(isRoot)(
+      'writes no fallback banner for the caller that discards the result',
+      () => {
+        const world = makeWorld();
+        try {
+          const shared = join(world.home, '.cache', 'qwen-code-ci');
+          mkdirSync(shared, { recursive: true });
+          chmodSync(shared, 0o555);
+          const summary = join(world.dir, 'summary.md');
+          writeFileSync(summary, '');
+          const { exitCode, dockerArgv } = runPruneStep(world, {
+            withResolver: true,
+            extraEnv: { GITHUB_STEP_SUMMARY: summary },
+          });
+          expect(exitCode).toBe(0);
+          expect(readFileSync(summary, 'utf8')).not.toContain(
+            'this job locks in',
+          );
+          expect(dockerArgv).not.toContain('image prune --all');
+        } finally {
+          rmSync(world.dir, { recursive: true, force: true });
+        }
+      },
+    );
   });
 });