F2b: connect the real lane D workspace (#87, part 1)

.github/workflows/agent-isolation.yml

@@ -8,6 +8,14 @@ on:
       - 'test/agent-*.test.ts'
       - 'runner/question-*.ts'
       - 'runner/questions.ts'
+      - 'runner/workspace.ts'
+      - 'runner/runner-repository.ts'
+      - 'runner/execution.ts'
+      - 'core/run-audit.ts'
+      - 'runner/review.ts'
+      - 'runner/store.ts'
+      - 'git/history.ts'
+      - 'test/runner-workspace.test.ts'
       - '.github/workflows/agent-isolation.yml'
   pull_request:
     paths:
@@ -16,6 +24,14 @@ on:
       - 'test/agent-*.test.ts'
       - 'runner/question-*.ts'
       - 'runner/questions.ts'
+      - 'runner/workspace.ts'
+      - 'runner/runner-repository.ts'
+      - 'runner/execution.ts'
+      - 'core/run-audit.ts'
+      - 'runner/review.ts'
+      - 'runner/store.ts'
+      - 'git/history.ts'
+      - 'test/runner-workspace.test.ts'
       - '.github/workflows/agent-isolation.yml'
 permissions:
   contents: read
@@ -33,4 +49,4 @@ jobs:
       - run: npm ci --ignore-scripts
       - run: npm run typecheck
       # The Docker suites share one image tag and daemon, so run test files one at a time.
-      - run: npx vitest run --no-file-parallelism test/agent-contract.test.ts test/agent-clone.test.ts test/agent-container.test.ts test/agent-network.test.ts test/agent-policy.test.ts test/agent-proxy.test.ts test/agent-adapter.test.ts test/agent-supervisor.test.ts test/agent-output.test.ts test/agent-gate.test.ts test/agent-question.test.ts
+      - run: npx vitest run --no-file-parallelism test/agent-contract.test.ts test/agent-clone.test.ts test/agent-container.test.ts test/agent-network.test.ts test/agent-policy.test.ts test/agent-proxy.test.ts test/agent-adapter.test.ts test/agent-supervisor.test.ts test/agent-output.test.ts test/agent-gate.test.ts test/agent-question.test.ts test/runner-workspace.test.ts

.github/workflows/ci.yml

@@ -18,6 +18,6 @@ jobs:
       - run: npm run typecheck
       # The Docker agent suites run one file at a time in the Agent isolation workflow; running them here
       # would put them in parallel against the same image tag and daemon.
-      - run: npm test -- --exclude test/agent-container.test.ts --exclude test/agent-network.test.ts --exclude test/agent-adapter.test.ts --exclude test/agent-supervisor.test.ts --exclude test/agent-gate.test.ts --exclude test/agent-question.test.ts
+      - run: npm test -- --exclude test/agent-container.test.ts --exclude test/agent-network.test.ts --exclude test/agent-adapter.test.ts --exclude test/agent-supervisor.test.ts --exclude test/agent-gate.test.ts --exclude test/agent-question.test.ts --exclude test/runner-workspace.test.ts
       - run: npx playwright install --with-deps chromium
       - run: npm run test:browser

agents/container/changes.ts

@@ -39,6 +39,12 @@ export interface TaskChange {
   readonly oldOid?: string;
   readonly newOid?: string;
   readonly newLinkTarget?: string;
+  /**
+   * For a change whose new entry is a symlink: whether its target, resolved one part at a time as the kernel would in
+   * an agent container, passes through another link on the way or is a link itself. A target outside the work tree or
+   * in the metadata counts as `true`, since D cannot check it.
+   */
+  readonly linkTargetTraversesLink?: boolean;
   /** Whether a part of the path is named `.git` (in any case): Git will never commit it. */
   readonly underGit: boolean;
   /**

agents/container/storage.ts

@@ -494,6 +494,27 @@ export function removeTaskFilesystems(filesystems: TaskFilesystems | RecoveredTa
   liveAllocations.delete(owner.allocationId);
 }
 
+/**
+ * `removeTaskFilesystems` without blocking the event loop: the same removal, each Docker call in its own process group.
+ * It is never cancelled: it runs to its own deadline, so nothing is dropped, and settles only after every call exited.
+ */
+export async function removeTaskFilesystemsAsync(filesystems: TaskFilesystems | RecoveredTaskStorage): Promise<void> {
+  const recovered = recoveredStorage.get(filesystems as RecoveredTaskStorage);
+  if (recovered) {
+    const handle = filesystems as RecoveredTaskStorage;
+    await runStepsAsync(cleanup(recovered.keeperId ? [recovered.keeperId] : [],
+      [handle.metadataVolume, handle.workVolume].filter((name): name is string => name !== undefined), recovered.owner), {});
+    recoveredStorage.delete(handle);
+    return;
+  }
+  const allocated = filesystems as TaskFilesystems;
+  assertTaskFilesystems(allocated);
+  const owner = taskFilesystemOwner(allocated);
+  await runStepsAsync(cleanup([allocated.keeper], [allocated.metadataVolume, allocated.workVolume], owner), {});
+  allocations.delete(allocated);
+  liveAllocations.delete(owner.allocationId);
+}
+
 /** At most this much diff is returned; the caller saves it as a stopped attempt's partial output. */
 export const MAXIMUM_EXPORT_BYTES = 1024 * 1024;
 export interface TaskDiff {

agents/container/tree-script.ts

@@ -304,21 +304,25 @@ sub anchored {
   }
   return { status => $status, anchor => target_entry($anchor eq "" ? "." : $anchor) };
 }
+# With $unchecked, names are kept as bytes and never checked: only the status is wanted.
 sub resolve {
-  my ($link, $walk) = @_; my %record = (link => text($link, "declared path"));
+  my ($link, $walk, $unchecked) = @_; my $name = $unchecked ? sub { $_[0] } : \&text;
+  # Unchecked, a link on the way is reported by status alone: its anchor entry would check the anchor's names.
+  my $through = sub { $unchecked ? { status => "through-link" } : anchored("through-link", $_[0]) };
+  my %record = (link => $name->($link, "declared path"));
   my @parents = split m{/}, $link; pop @parents; my $prefix = "";
   # A declared path that is not there, or not a link, is not-a-link whether or not its directory exists: adding or
   # deleting a declared file must not look like a change to a link. A directory on the way that is a link still makes
   # it through-link, since anything written there lands elsewhere.
   for my $part (@parents) {
     $prefix = join_path($prefix, $part); my @stat = lstat $prefix;
     return { %record, status => "not-a-link" } unless @stat;
-    return { %record, %{ anchored("through-link", $prefix) } } if -l _;
+    return { %record, %{ $through->($prefix) } } if -l _;
     return { %record, status => "not-a-link" } unless -d _;
   }
   my @stat = lstat $link;
   return { %record, status => "not-a-link" } unless @stat && -l _;
-  my $raw = readlink $link; $record{linkTarget} = text($raw, "the link target of " . shown($link));
+  my $raw = readlink $link; $record{linkTarget} = $name->($raw, "the link target of " . shown($link));
   # From the filesystem root: ("work", ...) is inside the work tree.
   my @at = $raw =~ m{^/} ? () : ("work", @parents);
   my @parts = grep { $_ ne "" && $_ ne "." } split m{/}, $raw;
@@ -333,15 +337,15 @@ sub resolve {
     return { %record, status => "metadata" } if @at > 1 && $at[1] eq ".git";
     next if @at == 1;
     my $path = join "/", @at[1 .. $#at]; my @here = lstat $path;
-    return { %record, %{ anchored("through-link", $path) } } if @here && -l _;
+    return { %record, %{ $through->($path) } } if @here && -l _;
   }
   return { %record, status => "outside" } if @at < 2;
   my $target = join "/", @at[1 .. $#at];
   # A dangling target, past a missing directory or not, is watched like any other: creating it, or what leads to it, is
   # a change to it. The target's own state (its entries, or its anchor) is reported once per target, not per link.
   my @final = lstat $target; $resolved_targets{$target} = 1;
   my $status = $walk ? target_state($target)->{status} : @final ? "present" : "absent";
-  return { %record, target => text($target, "path"), status => $status };
+  return { %record, target => $name->($target, "path"), status => $status };
 }
 # Each distinct target is walked once per run, so two declared links to one target count its entries once.
 my %target_states;
@@ -623,6 +627,15 @@ for my $change (@changes) {
   $change->{path} = text($change->{path}, "path");
   $change->{oldPath} = text($change->{oldPath}, "path") if defined $change->{oldPath};
 }
+# Where each changed link's new target leads, resolved one part at a time as the kernel would in an agent container:
+# whether a part on the way, or the target itself, is another link. One that leaves the work tree or enters the
+# metadata counts as traversing, since what is there cannot be checked from here.
+for my $i (0 .. $#changes) {
+  next unless ($changes[$i]{newType} // "") eq "symlink";
+  # The joined target can be longer than either name the manifest carries, so nothing here is checked as a name.
+  my $status = resolve($raw[$i]{path}, 0, 1)->{status};
+  $changes[$i]{linkTargetTraversesLink} = $status eq "present" || $status eq "absent" ? JSON::PP::false : JSON::PP::true;
+}
 
 my $head = git("rev-parse", "--verify", "HEAD"); chomp $head;
 my @agent_commits;

core/run-audit.ts

@@ -17,13 +17,24 @@ export interface ManifestChange {
   /** D reports whether resolving the new target would traverse another symlink. */
   linkTargetTraversesLink?: boolean;
   underGit: boolean;
+  /** Whether base's own ignore rules ignore this new path. Such a file is never committed (#87 decision 2). */
+  ignored: boolean;
 }
+/** A difference in a declared link's target since the pre-run snapshot (D's `LinkTargetChange`). */
+export interface LinkTargetChange {
+  link: string;
+  target: string;
+  path: string;
+  /** `retargeted`: the link now resolves elsewhere, which is an edit of the link itself; every other kind is a write through it. */
+  change: 'content' | 'mode' | 'type' | 'created' | 'deleted' | 'identity' | 'status' | 'retargeted';
+}
+/** D's `TaskChangeManifest`, as the audit reads it. */
 export interface ChangeManifest {
   changes: readonly ManifestChange[];
   agentCommits: readonly string[];
   metadataChanged: boolean;
   /** Differences from the pre-run snapshot of declared symlink targets. */
-  linkTargetChanges: readonly string[];
+  linkTargetChanges: readonly LinkTargetChange[];
   nestedGitlinkContent: readonly string[];
 }
 export type AuditOutcome =
@@ -51,6 +62,7 @@ export function isDotGit(part: string): boolean {
 const q = (text: string) => JSON.stringify(text.length > 300 ? `${text.slice(0, 300)}…` : text);
 const list = (items: readonly string[]) => items.slice(0, 5).map(q).join(', ') + (items.length > 5 ? ` and ${items.length - 5} more` : '');
 const TYPES = new Set(['file', 'symlink', 'gitlink', 'directory', 'other']);
+const LINK_CHANGES = new Set(['content', 'mode', 'type', 'created', 'deleted', 'identity', 'status', 'retargeted']);
 
 /**
  * Every field the audit reads, checked before it reads any (AGENTS.md: partial records fail closed). A missing boolean
@@ -59,8 +71,12 @@ const TYPES = new Set(['file', 'symlink', 'gitlink', 'directory', 'other']);
 function malformed(manifest: ChangeManifest): string | null {
   if (!manifest || typeof manifest !== 'object') return 'The change report is missing.';
   if (!Array.isArray(manifest.changes)) return 'The change report has no change list.';
-  for (const field of ['agentCommits', 'linkTargetChanges', 'nestedGitlinkContent'] as const)
+  for (const field of ['agentCommits', 'nestedGitlinkContent'] as const)
     if (!Array.isArray(manifest[field]) || manifest[field].some(entry => typeof entry !== 'string')) return `The change report has no ${field} list.`;
+  if (!Array.isArray(manifest.linkTargetChanges)) return 'The change report has no linkTargetChanges list.';
+  for (const entry of manifest.linkTargetChanges)
+    if (!entry || typeof entry !== 'object' || [entry.link, entry.target, entry.path].some(value => typeof value !== 'string')
+      || !LINK_CHANGES.has(entry.change)) return 'The change report has a malformed link target change.';
   if (typeof manifest.metadataChanged !== 'boolean') return 'The change report does not say whether Git metadata changed.';
   let bytes = 0;
   for (const change of manifest.changes) {
@@ -69,6 +85,7 @@ function malformed(manifest: ChangeManifest): string | null {
     if (change.kind === 'rename' ? typeof change.oldPath !== 'string' || !change.oldPath : change.oldPath !== undefined) return `The change at ${q(change.path)} has an invalid old path.`;
     if (!KINDS.has(change.kind)) return `The change at ${q(change.path)} has an unknown kind.`;
     if (typeof change.underGit !== 'boolean') return `The change at ${q(change.path)} does not say whether it is under .git.`;
+    if (typeof change.ignored !== 'boolean') return `The change at ${q(change.path)} does not say whether base ignores it.`;
     // add has only a new entry, delete only an old one, every other kind both.
     const needsOld = change.kind !== 'add', needsNew = change.kind !== 'delete';
     if ((needsOld && !TYPES.has(change.oldType as string)) || (!needsOld && change.oldType !== undefined)) return `The change at ${q(change.path)} has an invalid old entry type.`;
@@ -113,7 +130,11 @@ export function auditRun(item: PlanItem, manifest: ChangeManifest, pathKey: (pat
   if (manifest.metadataChanged) violations.push('The agent changed Git metadata under .git.');
   // Agents never commit: the metadata volume is read-only to them, so any agent commit is a violation, never undone (#66).
   if (manifest.agentCommits.length) violations.push(`The agent made its own commits: ${list(manifest.agentCommits)}.`);
-  if (manifest.linkTargetChanges.length) violations.push(`A declared symlink target changed: ${list(manifest.linkTargetChanges)}.`);
+  // A write through a declared link changes what it points at. A retargeted link is an edit of the link itself, judged
+  // below by the link rules, so the manifest must list that edit; without one, the way to its target changed.
+  const changedPaths = new Set(manifest.changes.flatMap(change => [change.path, ...(change.oldPath ? [change.oldPath] : [])]).map(pathKey));
+  const targetChanges = manifest.linkTargetChanges.filter(entry => entry.change !== 'retargeted' || !changedPaths.has(pathKey(entry.link)));
+  if (targetChanges.length) violations.push(`A declared symlink target changed: ${list(targetChanges.map(entry => `${entry.link} -> ${entry.path} (${entry.change})`))}.`);
   if (manifest.nestedGitlinkContent.length) violations.push(`Content appeared under a gitlink: ${list(manifest.nestedGitlinkContent)}.`);
   const declared = new Set(item.files.flatMap(file => [file.path, ...(file.renamed_from ? [file.renamed_from] : [])]).map(pathKey));
   // Each path appears once, under the trusted path identity: two entries for one path contradict each other.
@@ -142,6 +163,8 @@ export function auditRun(item: PlanItem, manifest: ChangeManifest, pathKey: (pat
     if (paths.some(path => path.startsWith('/') || posix.normalize(path).startsWith('../') || ['.', '..'].includes(posix.normalize(path)) || path.includes('\0')))
       { violations.push(`Invalid path in the change report: ${q(change.path)}.`); continue; }
     if (change.oldType === 'gitlink' || change.newType === 'gitlink') { violations.push(`Plan items cannot change gitlinks: ${q(change.path)}.`); continue; }
+    // A file base's own rules ignore (a build output, a .env) is never committed (#87 decision 2).
+    if (change.ignored) { violations.push(`The agent added ${q(change.path)}, which the repository ignores.`); continue; }
     // Only a declared pre-existing link may change at all: deleting it, turning it into a file, or renaming it from an
     // undeclared path is a link change too.
     if (change.oldType === 'symlink' && !declared.has(pathKey(change.oldPath ?? change.path)))