w759: fix the Windows EPERM and VM 'Domain is already active' test flakes (and main's red vault-rename check)

CHANGELOG.md

@@ -10,6 +10,24 @@ Add your change under **[Unreleased]** in the same pull request. `npm run releas
 
 ## [Unreleased]
 
+- **Unit tests on Windows no longer fail now and then on "EPERM, Permission denied" removing a test machine's folder, and
+  the VM end-to-end test no longer fails on "Domain is already active"** (w759, lothsahn: "the Windows EPERM temp-folder
+  failures hit different tests on each run ... A "Domain is already active" failure on #245 passed on rerun").
+  - EPERM: `TestMachine.stop()` deleted the machine's folder while a `git -C <worktree> status` or `log` its daemon's
+    pool had started was still running in it (seen in all 6 failures traced on BEAST), and the daemon's stop waits for
+    none of them. The test machine's pool now starts no git once it is stopping and `stop()` waits for the git still
+    running. CI did not see it: its Node 24.21 retries Windows' access-denied in `rmSync`, which BEAST's Node 23.7 does
+    not (nodejs/node#64698). Before: every one of 7 full runs on BEAST had 1 to 3 such failures; after: none in 10.
+  - Two tests (`server/providerLedger.test.ts`, the w278 PR summary in `server/devRequests.test.ts`) wrote a sandbox's PR
+    into the portal's record, and the daemon's next report of the sandbox took it away (4 of 10 runs on BEAST). The test
+    machine's git look now reports it (`TestMachine.gitLook`).
+  - "Domain is already active": `dom_state` in `deploy/vm/host/lib.sh` read `virsh domstate | head -n 1`. virsh writes a
+    blank line after the state in a second write; when `head` had already exited, virsh died of SIGPIPE, pipefail turned
+    that into "missing", and the answer was "running" and "missing" on two lines, so the second (idempotent) host install
+    started the running VM. libvirtd logged "End of file while reading data" in that same second each time (3 failures,
+    all on the ubuntu-26.04 host, 2026-10-06 and 2026-10-09). `dom_state` now reads virsh's whole answer;
+    `deploy/vm/test/fff-vm-nightly.test.sh` case 10 makes the blank line late.
+
 - **The Claude accounts list shows only accounts in use, with shorter names, and the vault entries are renamed `vault-<person>[-<n>]`**
   (w748, Lothsahn). An account appears only when a role or machine is set to use it or an agent runs on it now: LothDesktop's own
   login is left out while `machines.claudeFromVault` includes it (and returns when it does not), likewise a machine on the host

CONTRIBUTING.md

@@ -34,6 +34,17 @@ Tests live next to the code as `*.test.ts` and use `node:test`. A few (`updateSt
 Windows. `server/agentSession.test.ts` shows how to test session behaviour without Claude: it swaps
 the Agent SDK's `query()` for the scripted fake in `e2e/fakeAgent.ts` (`setQueryForTesting`).
 
+Tests with a machine (`server/testMachine.ts`, a real daemon with git worktrees) share two rules (w759):
+
+- **A sandbox's git is what the daemon's git look reports**: set it with `TestMachine.gitLook(name, { branch, pr })` and
+  wait for the portal to have it. Written straight into the portal's machine record, the daemon's next report of the
+  sandbox takes it away again, whenever that comes (on Windows under load, often mid-test).
+- **Nothing still runs in a folder that is being deleted.** `TestMachine.stop()` waits for the git the daemon's pool
+  started (a `git -C <worktree>` sits in that folder, and Windows refuses to delete a folder a live process is in:
+  `EPERM, Permission denied`). `rmSync`'s `maxRetries` does not cover it on Node before 24.21 / 26.8: those retry
+  EBUSY, ENOTEMPTY and EPERM but not Windows' access-denied (`std::errc::permission_denied`), and wait 0 ms between tries
+  (nodejs/node#64698). CI runs Node 24.21 or later; a Windows PC with an older Node shows the failures CI does not.
+
 ### End-to-end tests (Playwright)
 
 ```bash

deploy/vm/host/lib.sh

@@ -121,7 +121,15 @@ notify() {
 v() { virsh --connect qemu:///system "$@"; }
 
 dom_exists() { v dominfo "$VM_NAME" >/dev/null 2>&1; }
-dom_state() { v domstate "$VM_NAME" 2>/dev/null | head -n 1 || echo missing; }
+# dom_state: the domain's state ("running", "shut off", ...), or "missing". It reads virsh's whole answer (w759): virsh
+# prints the state, then a blank line in a second write; "| head -n 1" could exit in between, virsh died of SIGPIPE, and
+# pipefail made the answer "running" and "missing" on two lines. install.sh then started a running VM and failed on
+# "error: Domain is already active" (CI's 26.04 hosts, the second install).
+dom_state() {
+  local s
+  s=$(v domstate "$VM_NAME" 2>/dev/null) || { echo missing; return 0; }
+  printf '%s\n' "${s%%$'\n'*}"
+}
 dom_is_ours() { v dumpxml "$VM_NAME" 2>/dev/null | matches -F "$FFF_VM_MARK"; }
 net_exists() { v net-info "$NET_NAME" >/dev/null 2>&1; }
 net_is_ours() { v net-dumpxml "$NET_NAME" 2>/dev/null | matches -F "<bridge name='$NET_BRIDGE'" && v net-dumpxml "$NET_NAME" | matches -F "address='$NET_HOST_IP'"; }

deploy/vm/test/fff-vm-nightly.test.sh

@@ -33,7 +33,13 @@ echo "$cmd $*" >>"$D/calls"
 st=$(cat "$D/state")
 mib() { awk -F"[<>]" '$2 ~ /^memory( |$)/ {print int($3 / 1024)}' "$1"; }
 case "$cmd" in
-  domstate) echo "$st"; echo ;;
+  domstate)
+    [ -z "${FAKE_NO_DOMAIN:-}" ] || { echo "error: failed to get domain 'fff-portal'" >&2; exit 1; }
+    # Like virsh: the state, then a blank line in a second write; FAKE_LATE_BLANK holds it back so a reader that stopped
+    # after the first line has gone (w759).
+    echo "$st"
+    [ -z "${FAKE_LATE_BLANK:-}" ] || /bin/sleep 0.3
+    echo ;;
   dominfo) printf 'Name:           fff-portal\nAutostart:      enable\n' ;;
   domuuid) cat "$D/uuid"; echo ;;
   dumpxml)
@@ -272,4 +278,13 @@ matches -F 'fff-vm.conf changes the VM (memory 8192 -> 4096 MiB)' "$T/out" || fa
 [ "$(size live)" = "4096 2" ] || fail "9: runs $(size live)"
 ok "if-required: a pending size is a reason for tonight's cold restart"
 
+# 10. dom_state reads virsh's whole answer (w759): "| head -n 1" left virsh to die of SIGPIPE on its blank line, and
+# pipefail made a running VM "running" and "missing"; install.sh then started it again ("Domain is already active").
+host VM_MEMORY_MB=8192 VM_VCPUS=2
+st=$(FAKE_LATE_BLANK=1 bash -c '. "$FFF_VM_LIB/lib.sh"; load_conf "$FFF_VM_CONF"; dom_state')
+[ "$st" = running ] || fail "10: a running VM read as $(printf %q "$st")"
+st=$(FAKE_NO_DOMAIN=1 bash -c '. "$FFF_VM_LIB/lib.sh"; load_conf "$FFF_VM_CONF"; dom_state')
+[ "$st" = missing ] || fail "10: no domain read as $(printf %q "$st")"
+ok "dom_state: one line, whenever virsh writes its blank line; missing when virsh finds no domain"
+
 echo "fff-vm-nightly.test.sh: $PASS passed"

docs/ops-worker.md

@@ -173,7 +173,7 @@ process of the portal: it would get the portal's account and everything that acc
 | run the worker installer on a machine | ssh, above. The installer runs on the machine, with the machine's disk and network |
 | copy files to and from the machines with scp and sftp (w612) | `/usr/bin/scp` and `/usr/bin/sftp` run as `fff-ops` (its PATH's `scp` and `sftp` add `-S fff-ops-scp-ssh`), so the local side of a copy is only what `fff-ops` may read and write: its scratch, never `/srv/fff` or the vault key. `fff-ops-scp-ssh` takes the arguments scp and sftp give their ssh, lets through only their own fixed safe settings (no `-o ProxyCommand`, `-F`, `-i`, `-J`, `-S`, port 22 only) and hands the machine alone to `fff-ops-ssh --sftp TARGET`, the machine's sftp subsystem (or, for `scp -O`, scp's own `scp -t`/`scp -f` command). The machine side is the same ssh as above: the portal's key, pinned host keys, the same network. The guard refuses scp's ssh options and the portal's files as a source or destination, with a reason |
 | read the portal's state | `fff-ops-priv` (`status`, `state`, `units [--check]`, `logs N` redacted, `machine-ssh --check`, `credential list`) and the read-only tools `list_machines`, `list_sandboxes` and `system_status` |
-| read the vault, machine credentials and the migration's usage (w745) | `fffctl vault list` (the entries: name, kind, env, last four characters, a 12-character fingerprint of the value's hash, grants and the Claude pools' meters), `fffctl vault list --names`, `fffctl vault help`, `fffctl machine-credential list` (the same as `credential list`) and `fffctl migrate --help`. `fff-ops-priv` passes exactly these forms. Never a value: `vault list` shows what `show` in `server/vaultCli.ts` shows anyone who runs it, and there is no `vault get`. Every other vault form (`init`, `export-key`, `add`, `put`, `rotate`, `grant`, `remove`, `new-key`), `machine-credential issue` and `revoke`, and every `migrate` mode stay a person's |
+| read the vault, machine credentials and the migration's usage (w745) | `fffctl vault list` (the entries: name, kind, env, last four characters, a 12-character fingerprint of the value's hash, grants and the Claude pools' meters), `fffctl vault list --names`, `fffctl vault help`, `fffctl machine-credential list` (the same as `credential list`) and `fffctl migrate --help`. `fff-ops-priv` passes exactly these forms. Never a value: `vault list` shows what `show` in `server/vaultCli.ts` shows anyone who runs it, and there is no `vault get`. Every other vault form (`init`, `export-key`, `add`, `put`, `rotate`, `grant`, `remove`, `rename`, `new-key`), `machine-credential issue` and `revoke`, and every `migrate` mode stay a person's |
 | read the portal's critical units (w743) | `fffctl units` and `fffctl units --check` (its PATH's `fffctl` is `sudo -n fff-ops-priv`; the worker types no `sudo`, which its guard refuses). Both are `fffctl units` as root and read only: every critical unit's active and enabled state, the watchdog's verdict and its last restarts; `--check` prints `down: unit=state ...` and exits 1 when one is not active and enabled, else prints that every unit is and exits 0. `fff-ops-priv` passes exactly those two forms: `units --fix`, `units --check extra`, any other word and every `fffctl watchdog` form (run, pause, resume, reset) are refused with exit 2, and the unit restarts stay a person's. CI runs the allowed and refused forms against a fake `fffctl` (`deploy/vm/test/fff-ops.test.sh`) |
 | see the portal's ssh to the machines and its public key line (w676) | `fff-machine-ssh --check` and `--key` (its PATH's `fff-machine-ssh` is `sudo -n fff-ops-priv machine-ssh`): `fff-ops-priv` runs `/usr/local/lib/fff/fff-machine-ssh` as `fff` with the installed `machines.ssh`. It prints `id_ed25519.pub` and never reads the private key; CI looks for the private key in its output |
 | pin a new machine's host key (w676) | `fff-machine-ssh --pin USER@HOST FINGERPRINT`, the same way. As `fff`, it writes the key into the portal account's `~/.ssh/known_hosts` and `~/.ssh/fff-pins.ssh` (0600 `fff`, which `fff-ops` cannot read or write) and the alias into the managed block of `~/.ssh/config`, only when the key HOST shows over the tailnet now is FINGERPRINT and nothing pins HOST yet (`machines.ssh`, an earlier pin, `known_hosts` or `known_hosts2`). `fff-ops-priv` passes only `--check`, `--key` and `--pin` with exactly two words: never `--fix` or `--data` (a file of the worker's would be pinned) |

server/devRequests.test.ts

@@ -20,7 +20,7 @@ import { SETTABLE_KEYS, checkDevRequests } from './appConfig.ts';
 import { LEDGER } from './boardMatch.fixtures.ts';
 import { MockConnector } from '../e2e/mockConnector.ts';
 import type { Config } from './config.ts';
-import type { Machine, Requester, ServerEvent, SessionInfo, TranscriptEvent, UserInfo, WorkItem } from '../shared/types.ts';
+import type { GitStatus, Requester, ServerEvent, SessionInfo, TranscriptEvent, UserInfo, WorkItem } from '../shared/types.ts';
 import { fakeQuery } from '../e2e/fakeAgent.ts';
 import { startTestMachine, type TestMachine } from './testMachine.ts';
 
@@ -158,16 +158,21 @@ async function setup(t: { after: (fn: () => void | Promise<void>) => void }, ext
   const heard = (id: string) => store.readTranscript(id).filter((e): e is UserEv => e.kind === 'user' && e.from === 'system');
   const chat = (r: Requester) => o.personalFor(r);
   const work = () => [...store.work.values()];
-  return { dir, cfg, store, sessions, machines, agents, o, files, alpha, prs, pm: () => pm, connect, restart, call, heard, chat, work, dispatcher: () => sessions.get(agents.dispatcherId) };
+  return { dir, cfg, store, sessions, machines, agents, o, files, alpha, pc, prs, pm: () => pm, connect, restart, call, heard, chat, work, dispatcher: () => sessions.get(agents.dispatcherId) };
 }
 
 const setupOnMachine = (t: { after: (fn: () => void | Promise<void>) => void }, extra: Parameters<typeof setup>[1] = {}) => setup(t, { ...extra, onMachine: true });
 
-/** A machine sandbox's PR, as its daemon's next git look would report it. */
-function setMachinePr(store: Store, sandbox: string, git: NonNullable<NonNullable<Machine['sandboxes']>[number]['git']>) {
-  const m = store.machines.get('pc')!;
-  m.sandboxes!.find((x) => x.id === sandbox)!.git = git;
-  store.putMachine(m);
+/**
+ * A machine sandbox's branch and PR, as its daemon's git look reports them; back once the portal has them. Written into
+ * the portal's record instead, the daemon's next report of the sandbox took them away again (w759).
+ */
+async function setMachinePr(pc: TestMachine | undefined, store: Store, sandbox: string, git: { branch: string; pr: NonNullable<GitStatus['pr']> }) {
+  await pc!.gitLook(sandbox, git);
+  await until(`PR ${git.pr.number} on ${sandbox} in the portal`, () => {
+    const g = store.machines.get('pc')!.sandboxes!.find((x) => x.id === sandbox)!.git;
+    return g?.branch === git.branch && JSON.stringify(g.pr) === JSON.stringify(git.pr);
+  });
 }
 
 /** A dev_request's body, with what a test changes. */
@@ -708,7 +713,7 @@ async function settleWorker(store: Store, started: string, wid: string) {
 }
 
 test('w272: the request is followed to its result with dev_updates (branch and PR, merge, release), never a routing line, resent until dev_received', async (t) => {
-  const { connect, chat, call, pm, store, o, dispatcher } = await setupOnMachine(t);
+  const { connect, chat, call, pm, pc, store, o, dispatcher } = await setupOnMachine(t);
   const c = await connect();
   const req = devRequest('dev-done');
   const conversation = (req.conversation as { id: string }).id;
@@ -722,7 +727,7 @@ test('w272: the request is followed to its result with dev_updates (branch and P
   const started = await call(dispatcher().info, 'start_agent', { sandbox: 'pc/alpha', prompt: 'Build the filter.', title: 'Filter', work_id: wid });
   assert.equal(started.isError, false, started.text);
   await settleWorker(store, started.text, wid);
-  setMachinePr(store, 'alpha', { branch: 'sandbox/filter', dirty: 0, untracked: 0, pr: { number: 901, url: 'https://github.com/Final-Factory/FinalFactory/pull/901', title: 'Filter', draft: false }, at: T0 });
+  await setMachinePr(pc, store, 'alpha', { branch: 'sandbox/filter', pr: { number: 901, url: 'https://github.com/Final-Factory/FinalFactory/pull/901', title: 'Filter', draft: false } });
   pm().dev!.recheck();
   u = await nextUpdate(c, (x) => (x.watch as { pr?: number } | undefined)?.pr === 901 && !x.question);
   assert.equal(u.status, 'open');
@@ -832,7 +837,7 @@ test('w278: summaries and questions for a Discord thread carry results, never in
 });
 
 test('w278: a fix up on a PR sends its summary once the PR is ready, once; a draft sends nothing', async (t) => {
-  const { connect, store, prs, pm, call, dispatcher } = await setupOnMachine(t);
+  const { connect, store, prs, pm, pc, call, dispatcher } = await setupOnMachine(t);
   const c = await connect();
   c.send(devRequest('dev-pr'));
   await c.next('dev_ack');
@@ -841,7 +846,7 @@ test('w278: a fix up on a PR sends its summary once the PR is ready, once; a dra
   const started = await call(dispatcher().info, 'start_agent', { sandbox: 'pc/alpha', prompt: 'Build the filter.', title: 'Filter', work_id: wid });
   assert.equal(started.isError, false, started.text);
   await settleWorker(store, started.text, wid);
-  setMachinePr(store, 'alpha', { branch: 'sandbox/filter', dirty: 0, untracked: 0, pr: { number: 901, url: 'https://github.com/Final-Factory/FinalFactory/pull/901', title: 'Filter', draft: true }, at: T0 });
+  await setMachinePr(pc, store, 'alpha', { branch: 'sandbox/filter', pr: { number: 901, url: 'https://github.com/Final-Factory/FinalFactory/pull/901', title: 'Filter', draft: true } });
   // A draft: the PR is followed, nothing to say yet.
   prs.set(901, { number: 901, url: 'https://github.com/Final-Factory/FinalFactory/pull/901', title: 'Filter fix', body: PR_BODY, draft: true, autoMerge: false });
   pm().dev!.recheck();

server/opsWorker.ts

@@ -105,7 +105,7 @@ export const FFFCTL_FORMS: Record<string, { allowed: string[]; changes: string[]
   help: { allowed: [''], changes: [] },
   units: { allowed: ['', '--check'], changes: [] },
   logs: { allowed: ['', 'N'], changes: ['-f'] },
-  vault: { allowed: ['list', 'list --names', 'help'], changes: ['init', 'export-key', 'add', 'put', 'rotate', 'grant', 'remove', 'new-key'] },
+  vault: { allowed: ['list', 'list --names', 'help'], changes: ['init', 'export-key', 'add', 'put', 'rotate', 'grant', 'remove', 'rename', 'new-key'] },
   'machine-credential': { allowed: ['list'], changes: ['issue', 'revoke'] },
   migrate: { allowed: ['--help'], changes: ['--key', '--dry-run-copy', '--rollback-dry-run', '--cut-over'] },
   watchdog: { allowed: [], changes: ['run', 'pause', 'resume', 'reset'] },

server/providerLedger.test.ts

@@ -107,13 +107,13 @@ async function setup(t: { after: (fn: () => void | Promise<void>) => void }) {
     const r = await tool.handler(args);
     return { text: r.content.map((c) => c.text).join('\n'), isError: !!r.isError };
   };
-  return { store, agents, o, intake, pm, connect, dispatcher, call };
+  return { store, agents, o, intake, pm, pc, connect, dispatcher, call };
 }
 
 const conv = (o: Partial<ProviderConversation>): ProviderConversation => ({ id: '41', source: 'discord', opener: 'player', title: 'Belts stop', state: 'idle', agentClass: 'ffagent', createdAt: T0, updatedAt: T0, ...o });
 
 test('ledger check both ways: handshake, exact thread keys, what to watch, and the answer pushed as it changes', async (t) => {
-  const { store, o, intake, pm, connect, dispatcher, call } = await setup(t);
+  const { store, o, intake, pm, pc, connect, dispatcher, call } = await setup(t);
   const ben = o.personalFor(BEN);
   ben.lastFrom = 'human';
   // Ben's request is the work for the thread (its subjects, w343); an older one (filed before discord keys existed) names
@@ -148,11 +148,10 @@ test('ledger check both ways: handshake, exact thread keys, what to watch, and t
   // A worker starts on it in sandbox/lag-lead, and opens PR 812: the answer is pushed again, with the branch to watch.
   const started = await call(dispatcher().info, 'start_agent', { sandbox: 'pc/lag', prompt: 'Fix the lag.', title: 'Lag', work_id: a.id });
   assert.equal(started.isError, false, started.text);
-  // Its PR, as the daemon's next git look would report it.
-  const m = store.machines.get('pc')!;
-  const sb = m.sandboxes!.find((x) => x.id === 'lag')!;
-  sb.git = { branch: 'sandbox/lag-lead', dirty: 0, untracked: 0, pr: { number: 812, url: 'https://github.com/Final-Factory/FinalFactory/pull/812', title: 'Lag', draft: false }, at: T0 };
-  store.putMachine(m);
+  // Its PR, as the daemon's git look reports it. Written into the portal's record instead, the daemon's next report of
+  // the sandbox took it away again, and the answer was pushed a second time (w759).
+  await pc.gitLook('lag', { pr: { number: 812, url: 'https://github.com/Final-Factory/FinalFactory/pull/812', title: 'Lag', draft: false } });
+  await until('the PR in the portal', () => store.machines.get('pc')!.sandboxes!.find((x) => x.id === 'lag')!.git?.pr?.number === 812, 10_000);
   assert.equal(intake.recheckBoards(), 1, 'only the answer that changed goes');
   board = (await c.next('board')) as typeof board;
   assert.equal(board.update, true);