Probe task storage path identity at runner startup (#162)

AGENTS.md

@@ -59,6 +59,7 @@ A race regression must assert that the interleaving it names actually happened (
 - Each self-review and independent-agent review pass examines the full current PR diff against the base, including changed tests, configuration and documentation, and rereads every changed function in full. Do not review only the fixes or lines changed since the previous round. Code unchanged since the first commit of the PR still gets reviewed in every pass.
 - Treat every behavioural claim the change makes, in code comments, the PR body or docs (for example "pauses every 1,000 entries", "settles only after exit", "never throws", "bounded by N seconds"), as something to verify. Trace each claim through every path that can break it, including nested loops, callbacks, error paths and early returns, and give it a test that fails if the claim is false.
 - A test's setup must leave the state the production path would: if production never runs a step (such as a commit that refreshes Git's index), the test must not run it before the behaviour under test either.
+- A fake of an external tool (a stand-in `docker`, `git` or `gh` on `PATH`, or a stubbed client) must give, for every call the code reads, each part of the answer the code reads (exit status, stdout, stderr) as the real tool gives it, error cases included. Where it leaves out output the code does not read, say so beside the fake. Check those answers against the real tool and name the version checked beside the fake. Where versions the code may meet answer differently (for example `docker rm --force` on a missing name), the code must not depend on that answer: confirm the outcome with a call whose answer is stable, and give the fake each version's answer.
 - Independent-agent review is mandatory before every Copilot review request, including the first request and every re-request, for all changes, including documentation-only changes. The author's self-review is not a substitute. Use a reviewer that did not implement the change and does not share the author's working context: `/codex review`, a separate review agent, or `/code-review` at `high` effort or above.
 - Repeat independent-agent review -> address findings -> re-run relevant validation -> independent-agent review until a complete pass on the latest head reports no new issues and no earlier valid findings remain unresolved. A pass that found issues is not clean merely because they were fixed afterward; the fixes require another independent full-diff pass. Do not stop after a fixed number of rounds.
 - Only after that independent loop is clean may Copilot review be requested on the same reviewed and validated head. If Copilot finds issues, including summary-only concerns, address them and re-run relevant validation, then repeat the independent-agent review/fix loop until clean before requesting another Copilot review. Repeat this sequence until Copilot also completes a review with no new issues and no earlier valid findings remain unresolved. Never request Copilot in parallel with an unfinished independent-review cycle.

CLAUDE.md

@@ -29,6 +29,7 @@ In QA mode, flag any code that doesn't match DESIGN.md.
 - Each self-review and independent-agent review pass examines the full current PR diff against the base, including changed tests, configuration and documentation, and rereads every changed function in full. Do not review only the fixes or lines changed since the previous round. Code unchanged since the first commit of the PR still gets reviewed in every pass.
 - Treat every behavioural claim the change makes, in code comments, the PR body or docs (for example "pauses every 1,000 entries", "settles only after exit", "never throws", "bounded by N seconds"), as something to verify. Trace each claim through every path that can break it, including nested loops, callbacks, error paths and early returns, and give it a test that fails if the claim is false.
 - A test's setup must leave the state the production path would: if production never runs a step (such as a commit that refreshes Git's index), the test must not run it before the behaviour under test either.
+- A fake of an external tool (a stand-in `docker`, `git` or `gh` on `PATH`, or a stubbed client) must give, for every call the code reads, each part of the answer the code reads (exit status, stdout, stderr) as the real tool gives it, error cases included. Where it leaves out output the code does not read, say so beside the fake. Check those answers against the real tool and name the version checked beside the fake. Where versions the code may meet answer differently (for example `docker rm --force` on a missing name), the code must not depend on that answer: confirm the outcome with a call whose answer is stable, and give the fake each version's answer.
 - Independent-agent review is mandatory before every Copilot review request, including the first request and every re-request, for all changes, including documentation-only changes. The author's self-review is not a substitute. Use a reviewer that did not implement the change and does not share the author's working context: `/codex review`, a separate review agent, or `/code-review` at `high` effort or above.
 - Repeat independent-agent review -> address findings -> re-run relevant validation -> independent-agent review until a complete pass on the latest head reports no new issues and no earlier valid findings remain unresolved. A pass that found issues is not clean merely because they were fixed afterward; the fixes require another independent full-diff pass. Do not stop after a fixed number of rounds.
 - Only after that independent loop is clean may Copilot review be requested on the same reviewed and validated head. If Copilot finds issues, including summary-only concerns, address them and re-run relevant validation, then repeat the independent-agent review/fix loop until clean before requesting another Copilot review. Repeat this sequence until Copilot also completes a review with no new issues and no earlier valid findings remain unresolved. Never request Copilot in parallel with an unfinished independent-review cycle.

README.md

@@ -51,7 +51,7 @@ A configuration with a `github` block may also add a `runner` block. The runner
 - `diagnosticsCapBytes`: the size retention trims that directory back to. It is a target, not a hard limit: the file just saved is always kept, even when it alone passes the cap. The default is 256 MiB.
 - `limits`: task storage limits (`workBytes`, `workInodes`, `metadataBytes`, `metadataInodes`).
 
-The runner needs Docker and `CLAUDE_CODE_OAUTH_TOKEN`. At start, before the server opens, codeboost recovers what an earlier run left and builds the agent image; a recovery it cannot finish safely stops startup with what to do. `POST /api/runner` with `start` or `resume` runs the plan; the review screen has no runner buttons yet. A task paused for an out-of-scope change stays paused until the plan is amended, every changed path is declared, the remaining items validate against the runner's audited tree, and a person sends `approve-continuation` with the current `expectedStateVersion`, `expectedReviewVersion`, and an action ID. Then `resume` starts at the first unfinished item. `GET /api/runner` reports the checkpoint and whether the continuation is approved. When a run has completed every item, codeboost pushes the task head to a `codeboost/…` branch with your `gh` credentials and opens a ready pull request into `baseBranch`; a task that needs a person gets a draft pull request with its problems. Before merge, `prepare-merge` refreshes the published PR's exact base/head, rebases onto a moved base, refreshes attribution and approvals, and runs allowed `cmd:` checks in a credential-free read-only container. The check result counts only for that exact head and command list. A publish that was refused (for example, the branch holds a commit codeboost did not make) is retried with the `publish` action, and `GET /api/runner` shows the last outcome under `publish`. When a task is cancelled, codeboost closes the open pull requests it opened for it (the branch is kept). A pull request a person moved is left open and reported, and `close-pull-requests` retries a close that failed. Merge PR then merges that pull request, so `github.pullRequest` can be left out; if it is set, it must name the task's pull request, or merging is blocked. Demos never publish.
+The runner needs Docker and `CLAUDE_CODE_OAUTH_TOKEN`. At start, before the server opens, codeboost recovers what an earlier run left and builds the agent image; a recovery it cannot finish safely stops startup with what to do. It then checks `pathIdentity` against the task storage the agent edits, a Linux tmpfs (`caseSensitive: true`, `unicodeNormalization: "none"`), and refuses to start when they differ. `POST /api/runner` with `start` or `resume` runs the plan; the review screen has no runner buttons yet. A task paused for an out-of-scope change stays paused until the plan is amended, every changed path is declared, the remaining items validate against the runner's audited tree, and a person sends `approve-continuation` with the current `expectedStateVersion`, `expectedReviewVersion`, and an action ID. Then `resume` starts at the first unfinished item. `GET /api/runner` reports the checkpoint and whether the continuation is approved. When a run has completed every item, codeboost pushes the task head to a `codeboost/…` branch with your `gh` credentials and opens a ready pull request into `baseBranch`; a task that needs a person gets a draft pull request with its problems. Before merge, `prepare-merge` refreshes the published PR's exact base/head, rebases onto a moved base, refreshes attribution and approvals, and runs allowed `cmd:` checks in a credential-free read-only container. The check result counts only for that exact head and command list. A publish that was refused (for example, the branch holds a commit codeboost did not make) is retried with the `publish` action, and `GET /api/runner` shows the last outcome under `publish`. When a task is cancelled, codeboost closes the open pull requests it opened for it (the branch is kept). A pull request a person moved is left open and reported, and `close-pull-requests` retries a close that failed. Merge PR then merges that pull request, so `github.pullRequest` can be left out; if it is set, it must name the task's pull request, or merging is blocked. Demos never publish.
 
 ## Library
 

agents/container/storage.ts

@@ -3,10 +3,11 @@ import { randomUUID } from 'node:crypto';
 import { lstatSync } from 'node:fs';
 import { join } from 'node:path';
 import type { TaskClone } from '../contract.ts';
+import type { PathIdentity } from '../../core/path-identity.ts';
 import { assertTaskClone } from '../../git/clone.ts';
 import { assertBuiltAgentImage } from './image.ts';
 import { createOutcomeUnknown, DOCKER_ID } from '../client-outcome.ts';
-import { DockerError, pause, runDocker, type DockerOutcome } from '../docker.ts';
+import { DockerError, NOT_STARTED, pause, runDocker, type DockerOutcome } from '../docker.ts';
 import { MAXIMUM_TIMER_MS, runInProcessGroup, type ProcessGroup } from '../process-group.ts';
 import { runTrackedProcess, type ProcessGroupLifecycle } from '../tracked-docker.ts';
 import { MAXIMUM_DECLARED_LINKS, MAXIMUM_NAME_BYTES, TREE_SCRIPT } from './tree-script.ts';
@@ -265,6 +266,10 @@ function* allocationObjects(allocationId: string, expected: number, remaining: (
   }
 }
 
+/** The local-driver options of every task storage volume: a size- and inode-limited tmpfs owned by the storage user. */
+const TASK_VOLUME_OPTIONS = (bytes: number, inodes: number): readonly string[] =>
+  ['type=tmpfs', 'device=tmpfs', `o=size=${bytes},nr_inodes=${inodes},uid=10001,gid=10001,mode=0755,nosuid,nodev`];
+
 /** The allocation, as a sequence of Docker calls. Both variants run these same steps; only how each call runs differs. */
 function* allocation(clone: TaskClone, limits: TaskStorageLimits, imageId: string, owner: ResourceOwner,
   timeoutMs: number): Steps<TaskFilesystems> {
@@ -306,8 +311,7 @@ function* allocation(clone: TaskClone, limits: TaskStorageLimits, imageId: strin
   try {
     for (const [kind, name, bytes, inodes] of [['work', workVolume, limits.workBytes, limits.workInodes],
       ['metadata', metadataVolume, limits.metadataBytes, limits.metadataInodes]] as const) {
-      yield* allocate(name, ['volume', 'create', '--driver', 'local', '--opt', 'type=tmpfs', '--opt', 'device=tmpfs',
-        '--opt', `o=size=${bytes},nr_inodes=${inodes},uid=10001,gid=10001,mode=0755,nosuid,nodev`,
+      yield* allocate(name, ['volume', 'create', '--driver', 'local', ...TASK_VOLUME_OPTIONS(bytes, inodes).flatMap(option => ['--opt', option]),
         '--label', `io.codeboost.task-storage=${kind}`, ...labels, name]);
       // The check above and this create are not atomic across processes. Once our first object exists, it must be
       // the only one carrying the ID: of two racing allocations, the later check sees both and backs out.
@@ -400,6 +404,90 @@ export async function prepareTaskFilesystemsAsync(clone: TaskClone, limits: Task
   }
 }
 
+// In the directory given as $1, writes `case` and NFC `caf\u00e9`, then asks whether `CASE` and NFD `cafe\u0301` name
+// the same entries. printf writes the UTF-8 bytes, so the script itself stays ASCII. Exported so a test can run it on a
+// host folder whose answer is known.
+export const PATH_IDENTITY_SCRIPT = ['set -eu', 'cd "$1"', ': > case', ': > "$(printf \'caf\\303\\251\')"',
+  'if [ -e CASE ]; then c=false; else c=true; fi', 'if [ -e "$(printf \'cafe\\314\\201\')" ]; then u=NFC; else u=none; fi',
+  'printf \'codeboost-path-identity %s %s\\n\' "$c" "$u"'].join('; ');
+/** The probe's `/work`: an anonymous local volume with task storage's tmpfs options, as one `--mount` value. */
+export const PROBE_VOLUME_MOUNT = ['type=volume', 'target=/work', 'volume-driver=local',
+  ...TASK_VOLUME_OPTIONS(1024 * 1024, 64).map(option => `volume-opt=${option}`)]
+  // A field with a comma in it is quoted, as Docker's CSV parser reads --mount.
+  .map(field => field.includes(',') ? `"${field}"` : field).join(',');
+/**
+ * The path identity of task storage (#162): how the filesystem the agent edits, where the runner inspects task changes, tells two
+ * spellings of a path apart. It is measured, not assumed, on a volume made with the same options as task storage, in a
+ * container hardened like the keeper. The volume is anonymous and the container `--rm`, so once the container has
+ * started the daemon removes both when it exits, even if this process does not live to see it. A client stopped before
+ * then (its deadline) may leave the container created but never started, which `--rm` does not remove, so a failed
+ * probe removes its container, and that volume, by the unique name it gave it. Not covered: a create that lands after
+ * the settle window, or a runner that dies before the removal; the probe carries no owner labels, so recovery does not
+ * find that container (`docker ps -a --filter name=codeboost-path-identity-` does). Nor is the volume's removal
+ * confirmed: `--rm` and `rm --volumes` remove it, but Docker only logs a failure to. A recovery sweep is #181.
+ */
+export async function probeTaskStoragePathIdentity(imageId: string, timeoutMs = 60_000, cleanupMs = PROBE_CLEANUP_MS): Promise<PathIdentity> {
+  if (!/^sha256:[0-9a-f]{64}$/.test(imageId)) throw new Error('The path identity probe requires the immutable built image ID.');
+  assertBuiltAgentImage(imageId);
+  const name = `codeboost-path-identity-${randomUUID()}`;
+  const args = ['run', '--rm', '--name', name, '--read-only', '--user', '10001:10001', '--network=none', '--cap-drop=ALL',
+    '--security-opt=no-new-privileges', '--security-opt=seccomp=builtin', '--runtime=runc', '--pids-limit=32', '--memory=128m',
+    '--cpus=.25', '--mount', PROBE_VOLUME_MOUNT, '--entrypoint', 'sh', imageId, '-c', PATH_IDENTITY_SCRIPT, 'probe', '/work'];
+  const outcome = await runInProcessGroup('docker', args, { env: dockerEnvironment(), timeoutMs });
+  if (outcome.status !== 0) {
+    const failure = new DockerError(args, outcome);
+    // A client that never started sent nothing to the daemon, so there is nothing to remove.
+    const error = outcome.error as NodeJS.ErrnoException | undefined;
+    if (outcome.status === null && (error?.code === NOT_STARTED || error?.syscall?.startsWith('spawn'))) throw failure;
+    // Only a refusal from the daemon itself settles at once. Any other outcome (a killed client, a connection lost after
+    // the create was sent, exit 125 from the client) leaves the daemon's outcome unknown: a create may still land, so
+    // only absence after the settle window counts.
+    const refused = outcome.status !== null && /Error response from daemon/.test(outcome.stderr);
+    try { await removeProbeContainer(name, refused ? 0 : PROBE_SETTLE_MS, cleanupMs); }
+    catch (error) {
+      throw new Error(`${failure.message} The probe container ${name} may remain; remove it with docker rm --force --volumes ${name}.`,
+        { cause: new AggregateError([failure, error]) });
+    }
+    throw failure;
+  }
+  const answer = /^codeboost-path-identity (true|false) (none|NFC)$/.exec(outcome.stdout.trim());
+  if (!answer) throw new Error('The path identity probe did not report an identity.');
+  return { caseSensitive: answer[1] === 'true', unicodeNormalization: answer[2] as PathIdentity['unicodeNormalization'] };
+}
+
+/** One cleanup call's deadline: the time left before giving up, capped at 10 s and at least 1 s. */
+export const probeCleanupCallMs = (leftMs: number): number => Math.min(10_000, Math.max(1_000, Math.ceil(leftMs)));
+/** How long a failed probe's cleanup waits for an outcome Docker did not report (a killed client's create may land). */
+export const PROBE_SETTLE_MS = CREATE_SETTLE_MS;
+/** How long a failed probe's cleanup keeps asking after its settle window before it gives up. */
+export const PROBE_CLEANUP_MS = 30_000;
+/**
+ * Remove a failed probe's container and its anonymous volume by name. `rm --force` exits 0 for a name that does not
+ * exist, so only an inspect that reports the name absent, after the settle window, proves the probe is gone.
+ */
+async function removeProbeContainer(name: string, settleMs: number, cleanupMs: number): Promise<void> {
+  const settleBy = performance.now() + settleMs, giveUpAt = settleBy + cleanupMs;
+  let last: DockerOutcome | undefined;
+  // Each call gets the time left before giving up, capped at 10 s and at least 1 s (so the last ask can still be
+  // answered), and after the first call none starts once that time has passed: a daemon that stops answering
+  // stretches the bound by at most one call's floor, its stop grace and one 250 ms pause.
+  const ask = async (args: readonly string[]) => {
+    if (last && performance.now() >= giveUpAt)
+      throw new Error(`Docker did not confirm that ${name} is gone.`, { cause: last });
+    return last = await runInProcessGroup('docker', args, { env: dockerEnvironment(),
+      timeoutMs: probeCleanupCallMs(giveUpAt - performance.now()) });
+  };
+  for (;;) {
+    // Whatever rm answered (older CLIs exit non-zero for a missing name, newer ones 0), the inspect decides.
+    await ask(['rm', '--force', '--volumes', name]);
+    // The window is judged by when the inspect was asked: a slow answer to an ask made inside it proves nothing.
+    const asked = performance.now();
+    if (absent(await ask(['container', 'inspect', name])) && asked >= settleBy) return;
+    // Still there (a removal already in progress), not yet landed, or Docker did not answer: ask again.
+    await pause(250);
+  }
+}
+
 /**
  * Task storage that `recoverLeftovers` found after a restart, when the `TaskFilesystems` value that allocated it is
  * gone. A crash during allocation can leave any subset of the parts, so each is optional. D issues a handle only after