From b89d41cb4e51c1bd3da286645fe1a388b0e1466d Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sat, 31 Oct 2026 20:31:40 -0700
Subject: [PATCH 1/3] TASK-729: walk runner writes a machine-readable pass/fail
 result

The first-user walk now runs through runWalk(deps), so its production path
is testable against a fake backend. Every run overwrites a result file
(~/.commonly/state/walk-729/result.json, COMMONLY_WALK_RESULT) with the sha,
timings, each step's pass / fail / not-covered and reason, the cleanup
outcome and overall (pass | fail | incomplete). Exit codes match: 0, 1, 2.

Tests: pass signal, forced step failure, the password and email never in the
result or manifest, a missing password, and the ambiguous pod create with no
list match, which records the pod name as needs-admin-purge (carried over
from #3126's CodeRabbit thread).

Written by Forge, a Commonly agent.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-authored-by: forge <WyJmb3JnZSIsImRlZmF1bHQiXQ@agents.commonly.invalid>
Agent-Model: claude/claude-haiku-5-5/default
---
 docs/runbooks/first-user-walk.md |  28 ++
 scripts/first-user-walk.mjs      | 568 ++++++++++++++++++-------------
 scripts/first-user-walk.test.mjs | 196 +++++++++++
 3 files changed, 556 insertions(+), 236 deletions(-)
 create mode 100644 scripts/first-user-walk.test.mjs

diff --git a/docs/runbooks/first-user-walk.md b/docs/runbooks/first-user-walk.md
index 4d9e4b8e45..241012717a 100644
--- a/docs/runbooks/first-user-walk.md
+++ b/docs/runbooks/first-user-walk.md
@@ -77,6 +77,34 @@ cleanup reports `account <username>` as a survivor and the manifest is
 never holds the email. Only a run that dies before its manifest write (before
 signup starts) leaves nothing to find, and that case needs no purge.
 
+## Result file (what SRE reads)
+
+Every run, including one that fails before its first step, overwrites
+`~/.commonly/state/walk-729/result.json` (override with `COMMONLY_WALK_RESULT`).
+Write is atomic (temp file, then rename), so a reader never sees a half file.
+
+```json
+{
+  "schema": 1,
+  "sha": "<deployed sha, from WALK_SHA or the repo HEAD>",
+  "startedAt": "...", "finishedAt": "...",
+  "base": "https://api.commonly.me", "mode": "signup",
+  "walk": "walk-<stamp>",
+  "overall": "pass | fail | incomplete",
+  "exitCode": 0,
+  "steps": [{ "name": "signup", "result": "pass | fail | not-covered", "ms": 0, "reason": "only when not pass" }],
+  "cleanup": { "ok": true, "status": "clean | needs-admin-purge", "survivors": [] }
+}
+```
+
+- `overall` and the exit code agree: `pass` = `0`, `fail` = `1`, `incomplete` = `2`.
+- `incomplete` means every covered step passed and a step is not covered yet
+  (Bring your own connect). Today every full run is `incomplete`, never `pass`,
+  until that step is built. Roll back on `fail` only; treat `incomplete` as
+  not-green but not as a regression.
+- The file holds no password, token or email. A `needs-admin-purge` cleanup
+  points SRE at the manifest in `~/.commonly/state/walk-706/`.
+
 ## Daily schedule
 
 The CTO schedules the run on the host (launchd) and posts the result to the
diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs
index 2f0c9fee6d..c80a383df9 100644
--- a/scripts/first-user-walk.mjs
+++ b/scripts/first-user-walk.mjs
@@ -9,7 +9,15 @@
  * numbers exclude. Each run removes the seat, the pods it created and its own
  * account, then checks each of them is gone. What the walk's token cannot
  * reach (Scout, the hosted agent's User) is recorded by id in a per-run
- * manifest under the state dir, for SRE's admin purge. Password comes from COMMONLY_WALK_PASSWORD, never from this file.
+ * manifest under the state dir, for SRE's admin purge. Password comes from
+ * COMMONLY_WALK_PASSWORD, never from this file.
+ *
+ * Machine-readable signal (TASK-729): every run also overwrites a result file
+ * (default ~/.commonly/state/walk-729/result.json, override COMMONLY_WALK_RESULT)
+ * with the sha, timings, each step's pass / fail / not-covered and its reason,
+ * the cleanup outcome and `overall` (pass | fail | incomplete). The exit code
+ * matches: 0 pass, 1 fail, 2 incomplete. The file never holds the password, the
+ * token or the email.
  *
  * Usage:
  *   COMMONLY_WALK_PASSWORD=... node scripts/first-user-walk.mjs [--base https://api.commonly.me]
@@ -21,13 +29,15 @@
  * that account. A COMMONLY_WALK_EMAIL aimed at any other host is refused.
  *
  * Not yet covered: the Bring your own connect step (copy the command, run it
- * under a throwaway HOME, get a live reply). It is reported as NOT COVERED and
- * the run exits 2, so a green run never claims more than it checked.
+ * under a throwaway HOME, get a live reply). It is recorded as not-covered and
+ * the run is `incomplete` (exit 2), so a green run never claims more than it checked.
  */
 
-import { mkdirSync, writeFileSync } from 'node:fs';
+import { execFileSync } from 'node:child_process';
+import { mkdirSync, renameSync, writeFileSync } from 'node:fs';
 import { homedir } from 'node:os';
 import path from 'node:path';
+import { fileURLToPath } from 'node:url';
 import { cleanupRun, ownerOf, recoverRunPods } from './first-user-walk-cleanup.mjs';
 import { chooseWalkMode } from './first-user-walk-mode.mjs';
 
@@ -35,267 +45,353 @@ const WALK_PREFIX = 'walk-';
 // Every request is bounded, cleanup included, so one stalled socket cannot
 // hold the run or the report open.
 const REQUEST_TIMEOUT_MS = 20_000;
+const HOSTED_REPLY_WAIT_MS = 120_000;
 
-const args = process.argv.slice(2);
-const flag = (name, dflt) => {
-  const i = args.indexOf(`--${name}`);
-  return i >= 0 ? args[i + 1] : dflt;
-};
-const BASE = (flag('base', process.env.COMMONLY_API_URL || 'https://api.commonly.me')).replace(/\/+$/, '');
+const DEFAULT_STATE_DIR = path.join(homedir(), '.commonly', 'state', 'walk-706');
+const DEFAULT_RESULT_PATH = path.join(homedir(), '.commonly', 'state', 'walk-729', 'result.json');
 
-const now = Date.now();
-const stamp = now.toString(36);
-const username = `${WALK_PREFIX}${stamp}`;
-let email = `${username}@example.invalid`;
-const agentName = `${username}-agent`;
-const results = [];
-let step = 'init';
-// True once signup may have created the account, so cleanup knows to look for it.
-let accountMayExist = false;
-// 'signup' makes a fresh account; 'signin' uses the shared staging account and never deletes it.
-let walkMode = { mode: 'signup' };
+const NOT_COVERED = 'Bring your own connect step is not implemented yet';
 
-const api = async (method, path, { token, body } = {}) => {
-  const res = await fetch(`${BASE}${path}`, {
-    method,
-    headers: {
-      'Content-Type': 'application/json',
-      'User-Agent': 'commonly-walk/first-user',
-      ...(token ? { Authorization: `Bearer ${token}` } : {}),
-    },
-    body: body ? JSON.stringify(body) : undefined,
-    // A redirect would replay the JSON body, shared password included, to another origin.
-    redirect: 'error',
-    signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
-  });
-  let data = null;
-  try { data = await res.json(); } catch { /* non-json */ }
-  return { status: res.status, data };
-};
+const expect = (cond, msg) => { if (!cond) throw new Error(msg); };
 
-// Runs one named step. A thrown error or a falsy check is a failure; the
-// first failure stops the walk, because later steps depend on earlier ones.
-const runStep = async (name, fn) => {
-  step = name;
-  const started = Date.now();
-  try {
-    const value = await fn();
-    results.push({ name, ok: true, ms: Date.now() - started });
-    console.log(`PASS ${name} (${Date.now() - started} ms)`);
-    return value;
-  } catch (error) {
-    results.push({ name, ok: false, ms: Date.now() - started, error: error.message });
-    console.error(`FAIL ${name} (${Date.now() - started} ms): ${error.message}`);
-    throw error;
-  }
-};
+/**
+ * One walk run. Every dependency is injectable so the production path can be
+ * exercised under test without the network or the clock.
+ *   base       API origin, e.g. https://api.commonly.me
+ *   env        COMMONLY_WALK_PASSWORD, COMMONLY_WALK_EMAIL (never logged)
+ *   fetchImpl  fetch-compatible
+ *   stateDir   per-run manifest directory (ids only)
+ *   resultPath machine-readable result file (overwritten each run)
+ *   sha        deployed sha recorded in the result, or null
+ * Resolves to { exitCode, result }. It never calls process.exit.
+ */
+export const runWalk = async ({
+  base,
+  env = {},
+  fetchImpl = fetch,
+  stateDir = process.env.COMMONLY_WALK_STATE_DIR || DEFAULT_STATE_DIR,
+  resultPath = process.env.COMMONLY_WALK_RESULT || DEFAULT_RESULT_PATH,
+  sha = null,
+  now = () => Date.now(),
+  sleep = (ms) => new Promise((resolve) => { setTimeout(resolve, ms); }),
+  log = (line) => console.log(line),
+  error = (line) => console.error(line),
+}) => {
+  const BASE = String(base).replace(/\/+$/, '');
+  const startedMs = now();
+  const startedAt = new Date(startedMs).toISOString();
+  const stamp = startedMs.toString(36);
+  const username = `${WALK_PREFIX}${stamp}`;
+  const password = env.COMMONLY_WALK_PASSWORD || '';
+  let email = `${username}@example.invalid`;
+  const agentName = `${username}-agent`;
+  const results = [];
+  let step = 'init';
+  // True once signup may have created the account, so cleanup knows to look for it.
+  let accountMayExist = false;
+  // 'signup' makes a fresh account; 'signin' uses the shared staging account and never deletes it.
+  let walkMode = { mode: 'signup', deleteAccount: true };
+  // Signup returns no user id, so the manifest carries the exact username instead:
+  // SRE finds the account by that name, not by a prefix. Never the email.
+  const manifest = { mode: null, userId: null, username: null, podIds: [], unresolvedPodNames: [], agentName: null, botIds: [], survivors: [], status: 'running' };
 
-const expect = (cond, msg) => { if (!cond) throw new Error(msg); };
+  // Anything that can carry a credential is scrubbed before it reaches a log or
+  // the result file. An API error body can echo what was sent.
+  const redact = (text) => {
+    let out = String(text);
+    for (const secret of [password, email]) if (secret) out = out.split(secret).join('[redacted]');
+    return out;
+  };
 
-// Per-run manifest: ids only, outside the repo, for SRE's admin purge of what
-// the walk's own token cannot remove (Scout, the hosted agent's User). It holds
-// no password, token or email. It is named by the run stamp, not the username.
-const STATE_DIR = process.env.COMMONLY_WALK_STATE_DIR || path.join(homedir(), '.commonly', 'state', 'walk-706');
-// Signup returns no user id, so the manifest carries the exact username instead:
-// SRE finds the account by that name, not by a prefix. Never the email.
-const manifest = { mode: null, userId: null, username: null, podIds: [], unresolvedPodNames: [], agentName: null, botIds: [], survivors: [], status: 'running' };
-// A pod whose create outcome could not be checked. Its name is unique to the run,
-// so SRE can find it by exact name, the same way as the account.
-const unresolvedSurvivors = () => manifest.unresolvedPodNames.map((name) => `pod named "${name}" (create outcome unknown)`);
-const writeManifest = () => {
-  mkdirSync(STATE_DIR, { recursive: true, mode: 0o700 });
-  writeFileSync(path.join(STATE_DIR, `${stamp}.json`), `${JSON.stringify(manifest, null, 2)}\n`, { mode: 0o600 });
-};
+  const api = async (method, apiPath, { token, body } = {}) => {
+    const res = await fetchImpl(`${BASE}${apiPath}`, {
+      method,
+      headers: {
+        'Content-Type': 'application/json',
+        'User-Agent': 'commonly-walk/first-user',
+        ...(token ? { Authorization: `Bearer ${token}` } : {}),
+      },
+      body: body ? JSON.stringify(body) : undefined,
+      // A redirect would replay the JSON body, shared password included, to another origin.
+      redirect: 'error',
+      signal: AbortSignal.timeout(REQUEST_TIMEOUT_MS),
+    });
+    let data = null;
+    try { data = await res.json(); } catch { /* non-json */ }
+    return { status: res.status, data };
+  };
 
-// Returns false when anything survives, so the run fails loudly. Signup can
-// create the account before the walk has a token, so cleanup signs in once
-// more; if that fails the account is a survivor, not a silent gap.
-const cleanup = async (token, password) => {
-  let session = token;
-  if (!session && accountMayExist) {
+  // Runs one named step. A thrown error or a falsy check is a failure; the
+  // first failure stops the walk, because later steps depend on earlier ones.
+  const runStep = async (name, fn) => {
+    step = name;
+    const started = now();
     try {
-      const r = await api('POST', '/api/auth/login', { body: { email, password } });
-      if (r.status === 200 && r.data?.token) {
-        session = r.data.token;
-        manifest.userId = manifest.userId || String(r.data.user?._id || r.data.user?.id || '');
+      const value = await fn();
+      results.push({ name, result: 'pass', ms: now() - started });
+      log(`PASS ${name} (${now() - started} ms)`);
+      return value;
+    } catch (caught) {
+      const reason = redact(caught.message);
+      results.push({ name, result: 'fail', ms: now() - started, reason });
+      error(`FAIL ${name} (${now() - started} ms): ${reason}`);
+      throw caught;
+    }
+  };
+
+  // Per-run manifest: ids only, outside the repo, for SRE's admin purge of what
+  // the walk's own token cannot remove (Scout, the hosted agent's User). It holds
+  // no password, token or email. It is named by the run stamp, not the username.
+  // A pod whose create outcome could not be checked. Its name is unique to the run,
+  // so SRE can find it by exact name, the same way as the account.
+  const unresolvedSurvivors = () => manifest.unresolvedPodNames.map((name) => `pod named "${name}" (create outcome unknown)`);
+  const writeManifest = () => {
+    mkdirSync(stateDir, { recursive: true, mode: 0o700 });
+    writeFileSync(path.join(stateDir, `${stamp}.json`), `${JSON.stringify(manifest, null, 2)}\n`, { mode: 0o600 });
+  };
+
+  // Returns false when anything survives, so the run fails loudly. Signup can
+  // create the account before the walk has a token, so cleanup signs in once
+  // more; if that fails the account is a survivor, not a silent gap.
+  const cleanup = async (token) => {
+    let session = token;
+    if (!session && accountMayExist) {
+      try {
+        const r = await api('POST', '/api/auth/login', { body: { email, password } });
+        if (r.status === 200 && r.data?.token) {
+          session = r.data.token;
+          manifest.userId = manifest.userId || String(r.data.user?._id || r.data.user?.id || '');
+        }
+      } catch {
+        // A network failure here leaves no session: it is reported below as a survivor.
       }
-    } catch {
-      // A network failure here leaves no session: it is reported below as a survivor.
     }
-  }
-  if (!session) {
-    manifest.survivors = [
-      ...manifest.podIds.map((id) => `pod:${id} (no sign-in for cleanup)`),
-      ...(accountMayExist ? [`account ${manifest.username} (no sign-in for cleanup)`] : []),
-      ...unresolvedSurvivors(),
-    ];
-    manifest.status = manifest.survivors.length ? 'needs-admin-purge' : 'clean';
+    if (!session) {
+      manifest.survivors = [
+        ...manifest.podIds.map((id) => `pod:${id} (no sign-in for cleanup)`),
+        ...(accountMayExist ? [`account ${manifest.username} (no sign-in for cleanup)`] : []),
+        ...unresolvedSurvivors(),
+      ];
+      manifest.status = manifest.survivors.length ? 'needs-admin-purge' : 'clean';
+      writeManifest();
+      if (manifest.survivors.length) error(`cleanup: ${username} survives; sign-in failed`);
+      return manifest.survivors.length === 0;
+    }
+    try {
+      const result = await cleanupRun({
+        api,
+        token: session,
+        password,
+        email,
+        userId: manifest.userId,
+        agentName,
+        podIds: manifest.podIds,
+        deleteAccount: walkMode.deleteAccount,
+        // Persist each pod's bot ids before that pod is deleted, so a throw
+        // later in the loop still leaves the ids SRE needs.
+        onBotIds: (podId, bots) => {
+          manifest.botIds = [...new Set([...manifest.botIds, ...bots])];
+          writeManifest();
+        },
+      });
+      manifest.survivors = [...result.survivors, ...unresolvedSurvivors()];
+      manifest.botIds = [...new Set([...manifest.botIds, ...result.botIds])];
+      manifest.status = result.ok && manifest.survivors.length === 0 ? 'clean' : 'needs-admin-purge';
+    } catch (caught) {
+      manifest.survivors = [`cleanup error: ${redact(caught.message)}`, ...unresolvedSurvivors()];
+      manifest.status = 'needs-admin-purge';
+    }
     writeManifest();
-    if (manifest.survivors.length) console.error(`cleanup: ${username} survives; sign-in failed`);
-    return manifest.survivors.length === 0;
-  }
-  try {
-    const result = await cleanupRun({
-      api,
-      token: session,
-      password,
-      email,
-      userId: manifest.userId,
-      agentName,
-      podIds: manifest.podIds,
-      deleteAccount: walkMode.deleteAccount,
-      // Persist each pod's bot ids before that pod is deleted, so a throw
-      // later in the loop still leaves the ids SRE needs.
-      onBotIds: (podId, bots) => {
-        manifest.botIds = [...new Set([...manifest.botIds, ...bots])];
-        writeManifest();
-      },
-    });
-    manifest.survivors = [...result.survivors, ...unresolvedSurvivors()];
-    manifest.botIds = [...new Set([...manifest.botIds, ...result.botIds])];
-    manifest.status = result.ok && manifest.survivors.length === 0 ? 'clean' : 'needs-admin-purge';
-  } catch (error) {
-    manifest.survivors = [`cleanup error: ${error.message}`, ...unresolvedSurvivors()];
-    manifest.status = 'needs-admin-purge';
-  }
-  writeManifest();
-  if (manifest.status === 'clean') console.log(`cleanup: removed ${username}`);
-  else console.error(`cleanup: survivors: ${manifest.survivors.join('; ')}`);
-  return manifest.status === 'clean';
-};
+    if (manifest.status === 'clean') log(`cleanup: removed ${username}`);
+    else error(`cleanup: survivors: ${manifest.survivors.map(redact).join('; ')}`);
+    return manifest.status === 'clean';
+  };
 
-const main = async () => {
-  const password = process.env.COMMONLY_WALK_PASSWORD;
-  if (!password) throw new Error('COMMONLY_WALK_PASSWORD is not set');
-  walkMode = chooseWalkMode({ base: BASE, env: process.env });
-  if (walkMode.mode === 'signin') email = walkMode.email;
-  manifest.mode = walkMode.mode;
+  const writeResult = (exitCode, cleanupOk) => {
+    const overall = exitCode === 0 ? 'pass' : exitCode === 2 ? 'incomplete' : 'fail';
+    const result = {
+      schema: 1,
+      sha,
+      startedAt,
+      finishedAt: new Date(now()).toISOString(),
+      base: BASE,
+      mode: walkMode.mode,
+      walk: username,
+      overall,
+      exitCode,
+      steps: results,
+      cleanup: { ok: cleanupOk, status: manifest.status, survivors: manifest.survivors.map(redact) },
+    };
+    mkdirSync(path.dirname(resultPath), { recursive: true, mode: 0o700 });
+    const tmp = `${resultPath}.${process.pid}.tmp`;
+    writeFileSync(tmp, `${JSON.stringify(result, null, 2)}\n`, { mode: 0o600 });
+    renameSync(tmp, resultPath);
+    return result;
+  };
 
-  let token = null;
   let exitCode = 0;
+  let cleanupOk = true;
   try {
-    if (walkMode.mode === 'signup') {
-      await runStep('signup', async () => {
-        manifest.username = username;
+    expect(password, 'COMMONLY_WALK_PASSWORD is not set');
+    walkMode = chooseWalkMode({ base: BASE, env });
+    if (walkMode.mode === 'signin') email = walkMode.email;
+    manifest.mode = walkMode.mode;
+
+    let token = null;
+    try {
+      if (walkMode.mode === 'signup') {
+        await runStep('signup', async () => {
+          manifest.username = username;
+          writeManifest();
+          let r;
+          try {
+            r = await api('POST', '/api/auth/register', { body: { username, email, password } });
+          } catch (caught) {
+            accountMayExist = true; // the request may have landed before the failure
+            throw caught;
+          }
+          if (r.status === 201) accountMayExist = true;
+          expect(r.status === 201, `register ${r.status}: ${JSON.stringify(r.data)}`);
+        });
+      }
+
+      token = await runStep('login', async () => {
+        const r = await api('POST', '/api/auth/login', { body: { email, password } });
+        // Status and error code only: a body could echo the shared email into the log.
+        expect(r.status === 200 && r.data?.token, `login ${r.status}${r.data?.code ? ` ${r.data.code}` : ''}`);
+        expect(r.data.user?.role !== 'admin', 'walk account is admin; the walk is meaningless');
+        manifest.userId = String(r.data.user?._id || r.data.user?.id || '');
+        expect(manifest.userId, 'login returned no user id');
         writeManifest();
-        let r;
-        try {
-          r = await api('POST', '/api/auth/register', { body: { username, email, password } });
-        } catch (error) {
-          accountMayExist = true; // the request may have landed before the failure
-          throw error;
-        }
-        if (r.status === 201) accountMayExist = true;
-        expect(r.status === 201, `register ${r.status}: ${JSON.stringify(r.data)}`);
+        return r.data.token;
       });
-    }
-
-    token = await runStep('login', async () => {
-      const r = await api('POST', '/api/auth/login', { body: { email, password } });
-      // Status and error code only: a body could echo the shared email into the log.
-      expect(r.status === 200 && r.data?.token, `login ${r.status}${r.data?.code ? ` ${r.data.code}` : ''}`);
-      expect(r.data.user?.role !== 'admin', 'walk account is admin; the walk is meaningless');
-      manifest.userId = String(r.data.user?._id || r.data.user?.id || '');
-      expect(manifest.userId, 'login returned no user id');
-      writeManifest();
-      return r.data.token;
-    });
 
-    // Signup creates a workspace pod. Check it exists before the walk makes its
-    // own pod, so a failed signup workspace cannot be hidden by the second pod.
-    // Signup-only: on the shared account a workspace pod already exists, and
-    // recording it here would make cleanup delete a pod this run did not create.
-    if (walkMode.mode === 'signup') await runStep('signup-workspace', async () => {
-      const list = await api('GET', '/api/pods', { token });
-      const pods = Array.isArray(list.data) ? list.data : (list.data?.pods || []);
-      const owned = list.status === 200 ? pods.filter((p) => ownerOf(p) === manifest.userId) : [];
-      expect(owned.length > 0, `no signup workspace pod (list ${list.status}, owned ${owned.length})`);
-      for (const p of owned) {
-        if (!manifest.podIds.includes(String(p._id))) manifest.podIds.push(String(p._id));
-      }
-      writeManifest();
-    });
+      // Signup creates a workspace pod. Check it exists before the walk makes its
+      // own pod, so a failed signup workspace cannot be hidden by the second pod.
+      // Signup-only: on the shared account a workspace pod already exists, and
+      // recording it here would make cleanup delete a pod this run did not create.
+      if (walkMode.mode === 'signup') await runStep('signup-workspace', async () => {
+        const list = await api('GET', '/api/pods', { token });
+        const pods = Array.isArray(list.data) ? list.data : (list.data?.pods || []);
+        const owned = list.status === 200 ? pods.filter((p) => ownerOf(p) === manifest.userId) : [];
+        expect(owned.length > 0, `no signup workspace pod (list ${list.status}, owned ${owned.length})`);
+        for (const p of owned) {
+          if (!manifest.podIds.includes(String(p._id))) manifest.podIds.push(String(p._id));
+        }
+        writeManifest();
+      });
 
-    const pod = await runStep('first-pod', async () => {
-      const podName = `${username} workspace`;
-      // An error, or a reply without an id, may still mean the pod was made.
-      // Look for it by its unique name before the step can fail, so cleanup
-      // removes it, or the name is reported as unresolved.
-      const recover = async () => {
-        if (!await recoverRunPods({ api, token, userId: manifest.userId, podName, podIds: manifest.podIds })) {
-          manifest.unresolvedPodNames.push(podName);
+      const pod = await runStep('first-pod', async () => {
+        const podName = `${username} workspace`;
+        // An error, or a reply without an id, may still mean the pod was made.
+        // Look for it by its unique name before the step can fail, so cleanup
+        // removes it, or the name is reported as unresolved.
+        const recover = async () => {
+          if (!await recoverRunPods({ api, token, userId: manifest.userId, podName, podIds: manifest.podIds })) {
+            manifest.unresolvedPodNames.push(podName);
+          }
+          writeManifest();
+        };
+        let r;
+        try {
+          r = await api('POST', '/api/pods', { token, body: { name: podName, type: 'team' } });
+        } catch (caught) {
+          await recover();
+          throw caught;
         }
+        const created = r.data?.pod || r.data;
+        // Any reply without a created pod may still have committed one, including a
+        // 4xx or 5xx, so the outcome is checked by name rather than trusted.
+        if (!(r.status < 300 && created?._id)) await recover();
+        expect(r.status < 300 && created?._id, `pod create ${r.status}: ${JSON.stringify(r.data)}`);
+        manifest.podIds.push(String(created._id));
         writeManifest();
-      };
-      let r;
-      try {
-        r = await api('POST', '/api/pods', { token, body: { name: podName, type: 'team' } });
-      } catch (error) {
-        await recover();
-        throw error;
-      }
-      const created = r.data?.pod || r.data;
-      // Any reply without a created pod may still have committed one, including a
-      // 4xx or 5xx, so the outcome is checked by name rather than trusted.
-      if (!(r.status < 300 && created?._id)) await recover();
-      expect(r.status < 300 && created?._id, `pod create ${r.status}: ${JSON.stringify(r.data)}`);
-      manifest.podIds.push(String(created._id));
-      writeManifest();
-      return created;
-    });
+        return created;
+      });
 
-    await runStep('hosted-install', async () => {
-      manifest.agentName = agentName;
-      writeManifest();
-      const r = await api('POST', '/api/registry/install', {
-        token,
-        body: { agentName, podId: pod._id, displayName: agentName, config: { runtime: { runtimeType: 'hosted' } }, scopes: [] },
+      await runStep('hosted-install', async () => {
+        manifest.agentName = agentName;
+        writeManifest();
+        const r = await api('POST', '/api/registry/install', {
+          token,
+          body: { agentName, podId: pod._id, displayName: agentName, config: { runtime: { runtimeType: 'hosted' } }, scopes: [] },
+        });
+        expect(r.status < 400, `install ${r.status}: ${JSON.stringify(r.data)}`);
+        const p = await api('POST', '/api/hosted/provision', { token, body: { agentName } });
+        expect(p.status === 200 && p.data?.provisioned, `provision ${p.status}: ${JSON.stringify(p.data)}`);
       });
-      expect(r.status < 400, `install ${r.status}: ${JSON.stringify(r.data)}`);
-      const p = await api('POST', '/api/hosted/provision', { token, body: { agentName } });
-      expect(p.status === 200 && p.data?.provisioned, `provision ${p.status}: ${JSON.stringify(p.data)}`);
-    });
 
-    await runStep('hosted-reply', async () => {
-      const m = await api('POST', `/api/messages/${pod._id}`, {
-        token,
-        body: { content: `@${agentName} walk check: reply with one short sentence containing KINGFISHER.` },
+      await runStep('hosted-reply', async () => {
+        const m = await api('POST', `/api/messages/${pod._id}`, {
+          token,
+          body: { content: `@${agentName} walk check: reply with one short sentence containing KINGFISHER.` },
+        });
+        expect(m.status === 200, `mention ${m.status}: ${JSON.stringify(m.data)}`);
+        expect((m.data?.agentDelivery?.enqueued ?? 0) >= 1, 'mention was not enqueued to the agent');
+        const deadline = now() + HOSTED_REPLY_WAIT_MS;
+        while (now() < deadline) {
+          const msgs = await api('GET', `/api/messages/${pod._id}`, { token });
+          const list = Array.isArray(msgs.data) ? msgs.data : (msgs.data?.messages || []);
+          if (list.some((x) => x.username === agentName && /KINGFISHER/i.test(x.content || ''))) return;
+          await sleep(3000);
+        }
+        throw new Error('no agent reply containing KINGFISHER within 120s');
       });
-      expect(m.status === 200, `mention ${m.status}: ${JSON.stringify(m.data)}`);
-      expect((m.data?.agentDelivery?.enqueued ?? 0) >= 1, 'mention was not enqueued to the agent');
-      const deadline = Date.now() + 120_000;
-      while (Date.now() < deadline) {
-        const msgs = await api('GET', `/api/messages/${pod._id}`, { token });
-        const list = Array.isArray(msgs.data) ? msgs.data : (msgs.data?.messages || []);
-        if (list.some((x) => x.username === agentName && /KINGFISHER/i.test(x.content || ''))) return;
-        await new Promise((resolve) => { setTimeout(resolve, 3000); });
-      }
-      throw new Error('no agent reply containing KINGFISHER within 120s');
-    });
 
-    // API only: this checks the invite preview endpoint. The link is not opened
-    // in a browser, so a PASS here does not show that the rendered page loads.
-    await runStep('invite-preview-api', async () => {
-      const r = await api('POST', `/api/pods/${pod._id}/invites`, { token, body: {} });
-      expect(r.status < 300 && r.data?.token, `invite create ${r.status}: ${JSON.stringify(r.data)}`);
-      const preview = await api('GET', `/api/invites/${r.data.token}/preview`);
-      expect(preview.status === 200, `invite preview ${preview.status}: ${JSON.stringify(preview.data)}`);
-    });
+      // API only: this checks the invite preview endpoint. The link is not opened
+      // in a browser, so a PASS here does not show that the rendered page loads.
+      await runStep('invite-preview-api', async () => {
+        const r = await api('POST', `/api/pods/${pod._id}/invites`, { token, body: {} });
+        expect(r.status < 300 && r.data?.token, `invite create ${r.status}: ${JSON.stringify(r.data)}`);
+        const preview = await api('GET', `/api/invites/${r.data.token}/preview`);
+        expect(preview.status === 200, `invite preview ${preview.status}: ${JSON.stringify(preview.data)}`);
+      });
 
-    await runStep('byo-connect', async () => {
-      throw new Error('NOT COVERED: Bring your own connect step is not implemented yet');
-    }).catch(() => { exitCode = 2; });
-  } catch {
+      // Recorded, not run: a step that is not covered never counts as a pass.
+      results.push({ name: 'byo-connect', result: 'not-covered', ms: 0, reason: NOT_COVERED });
+      log(`NOT COVERED byo-connect: ${NOT_COVERED}`);
+      exitCode = 2;
+    } catch {
+      exitCode = 1;
+    } finally {
+      cleanupOk = await cleanup(token);
+      if (!cleanupOk) exitCode = 1;
+    }
+  } catch (caught) {
+    // Setup failed before any step ran (for example, no password was given).
+    error(`FAIL ${step}: ${redact(caught.message)}`);
+    results.push({ name: step, result: 'fail', ms: 0, reason: redact(caught.message) });
     exitCode = 1;
-  } finally {
-    if (!await cleanup(token, password)) exitCode = 1;
   }
 
-  console.log(JSON.stringify({ walk: username, steps: results }, null, 2));
-  process.exit(exitCode);
+  const result = writeResult(exitCode, cleanupOk);
+  log(JSON.stringify({ walk: username, overall: result.overall, steps: results }, null, 2));
+  return { exitCode, result };
+};
+
+const parseArgs = (argv) => {
+  const i = argv.indexOf('--base');
+  return i >= 0 ? argv[i + 1] : undefined;
 };
 
-main().catch((error) => {
-  console.error(`FAIL ${step}: ${error.message}`);
-  process.exit(1);
-});
+// Deployed sha for the result: an explicit WALK_SHA from the scheduler (the
+// deploy being walked), else the HEAD of the repo this script runs from.
+const shaFromEnv = (env) => {
+  if (env.WALK_SHA) return env.WALK_SHA;
+  try {
+    return execFileSync('git', ['rev-parse', 'HEAD'], { cwd: path.dirname(fileURLToPath(import.meta.url)), stdio: ['ignore', 'pipe', 'ignore'] }).toString().trim();
+  } catch {
+    return null;
+  }
+};
+
+const isEntry = process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url);
+if (isEntry) {
+  const base = parseArgs(process.argv.slice(2)) || process.env.COMMONLY_API_URL || 'https://api.commonly.me';
+  runWalk({ base, env: process.env, sha: shaFromEnv(process.env) })
+    .then(({ exitCode }) => process.exit(exitCode))
+    .catch((caught) => {
+      console.error(`FAIL walk: ${caught.message}`);
+      process.exit(1);
+    });
+}
diff --git a/scripts/first-user-walk.test.mjs b/scripts/first-user-walk.test.mjs
new file mode 100644
index 0000000000..842776dd75
--- /dev/null
+++ b/scripts/first-user-walk.test.mjs
@@ -0,0 +1,196 @@
+// Run: node --test scripts/first-user-walk.test.mjs
+// Drives runWalk through its production path against a fake backend, so the
+// result file, the manifest and the exit code are the ones a real run writes.
+import test from 'node:test';
+import assert from 'node:assert/strict';
+import { existsSync, mkdtempSync, readdirSync, readFileSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import path from 'node:path';
+import { runWalk } from './first-user-walk.mjs';
+
+const PASSWORD = 'pw-secret-for-tests-9f2';
+const USER_ID = 'user-walk';
+const TOKEN = 'token-for-tests';
+
+// Options:
+//   preview       status of the invite preview GET (500 forces a step failure)
+//   create        'ok' creates the pod; 'error' returns 500 and creates nothing
+const fakeBackend = ({ preview = 200, create = 'ok' } = {}) => {
+  const state = { accountExists: false, pods: [], lastAgent: null, calls: [] };
+  const reply = (status, data) => ({ status, json: async () => data });
+  const fetchImpl = async (url, init = {}) => {
+    const u = new URL(url);
+    const method = init.method || 'GET';
+    const p = u.pathname;
+    const body = init.body ? JSON.parse(init.body) : {};
+    state.calls.push(`${method} ${p}`);
+
+    if (method === 'POST' && p === '/api/auth/register') {
+      state.accountExists = true;
+      // The walk's own workspace pod, as signup creates it.
+      state.pods.push({ _id: 'ws1', name: 'Workspace', createdBy: USER_ID, members: [] });
+      return reply(201, {});
+    }
+    if (method === 'POST' && p === '/api/auth/login') {
+      if (state.accountExists && body.password === PASSWORD) {
+        return reply(200, { token: TOKEN, user: { _id: USER_ID, role: 'user' } });
+      }
+      return reply(401, { code: 'bad_credentials' });
+    }
+    if (method === 'GET' && p === '/api/pods') return reply(200, state.pods.map((x) => ({ ...x })));
+    if (method === 'POST' && p === '/api/pods') {
+      if (create !== 'ok') return reply(500, { error: 'boom' });
+      const pod = { _id: `p${state.pods.length + 1}`, name: body.name, createdBy: USER_ID, members: [] };
+      state.pods.push(pod);
+      return reply(201, pod);
+    }
+    const podMatch = p.match(/^\/api\/pods\/([^/]+)$/);
+    if (podMatch) {
+      const pod = state.pods.find((x) => x._id === podMatch[1]);
+      if (method === 'GET') return pod ? reply(200, pod) : reply(404, {});
+      if (method === 'DELETE') {
+        state.pods = state.pods.filter((x) => x._id !== podMatch[1]);
+        return reply(200, {});
+      }
+    }
+    if (method === 'POST' && p === '/api/registry/install') return reply(200, {});
+    if (method === 'POST' && p === '/api/hosted/provision') return reply(200, { provisioned: true });
+    if (method === 'POST' && p === '/api/hosted/deprovision') return reply(404, {});
+    if (method === 'DELETE' && p.startsWith('/api/registry/agents/')) return reply(404, {});
+    if (method === 'POST' && /^\/api\/messages\//.test(p)) {
+      state.lastAgent = (body.content.match(/@(\S+) walk check/) || [])[1] || null;
+      return reply(200, { agentDelivery: { enqueued: 1 } });
+    }
+    if (method === 'GET' && /^\/api\/messages\//.test(p)) {
+      const list = state.lastAgent ? [{ username: state.lastAgent, content: 'Sure, KINGFISHER.' }] : [];
+      return reply(200, list);
+    }
+    if (method === 'POST' && /\/invites$/.test(p)) return reply(201, { token: 'inv1' });
+    if (method === 'GET' && p === '/api/invites/inv1/preview') return reply(preview, { ok: true });
+    if (method === 'DELETE' && p === '/api/users/me') {
+      state.accountExists = false;
+      return reply(200, {});
+    }
+    return reply(500, { error: `unhandled ${method} ${p}` });
+  };
+  return { fetchImpl, state };
+};
+
+const dirs = () => {
+  const root = mkdtempSync(path.join(tmpdir(), 'walk-729-'));
+  return { stateDir: path.join(root, 'state'), resultPath: path.join(root, 'result', 'result.json') };
+};
+
+const run = (fetchImpl, { stateDir, resultPath }) => runWalk({
+  base: 'https://api.commonly.me',
+  env: { COMMONLY_WALK_PASSWORD: PASSWORD },
+  fetchImpl,
+  stateDir,
+  resultPath,
+  sha: 'abc123def',
+  sleep: async () => {},
+  log: () => {},
+  error: () => {},
+});
+
+test('a clean run writes the machine-readable signal: every covered step passes, byo is not-covered, overall incomplete', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl, state } = fakeBackend();
+  const { exitCode, result } = await run(fetchImpl, dirsFor);
+
+  assert.equal(exitCode, 2);
+  assert.equal(result.overall, 'incomplete');
+  assert.equal(result.sha, 'abc123def');
+  assert.equal(result.mode, 'signup');
+  assert.equal(result.cleanup.ok, true);
+  assert.equal(result.cleanup.status, 'clean');
+
+  const names = result.steps.map((s) => s.name);
+  assert.deepEqual(names, ['signup', 'login', 'signup-workspace', 'first-pod', 'hosted-install', 'hosted-reply', 'invite-preview-api', 'byo-connect']);
+  assert.ok(result.steps.filter((s) => s.name !== 'byo-connect').every((s) => s.result === 'pass'));
+  assert.equal(result.steps.at(-1).result, 'not-covered');
+  assert.match(result.steps.at(-1).reason, /not implemented/);
+
+  // The file on disk is the one a consumer reads, and it is valid JSON.
+  const onDisk = JSON.parse(readFileSync(dirsFor.resultPath, 'utf8'));
+  assert.deepEqual(onDisk, result);
+  // The account was removed; nothing survives.
+  assert.equal(state.accountExists, false);
+  assert.equal(state.pods.length, 0);
+});
+
+test('a forced step failure writes overall fail, names the step, and exits 1', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl } = fakeBackend({ preview: 500 });
+  const { exitCode, result } = await run(fetchImpl, dirsFor);
+
+  assert.equal(exitCode, 1);
+  assert.equal(result.overall, 'fail');
+  const failed = result.steps.filter((s) => s.result === 'fail');
+  assert.equal(failed.length, 1);
+  assert.equal(failed[0].name, 'invite-preview-api');
+  assert.match(failed[0].reason, /invite preview 500/);
+  // The byo step never ran after the failure.
+  assert.equal(result.steps.some((s) => s.name === 'byo-connect'), false);
+  assert.equal(result.cleanup.ok, true);
+  assert.equal(JSON.parse(readFileSync(dirsFor.resultPath, 'utf8')).overall, 'fail');
+});
+
+test('the result and manifest never carry the password or the email', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl } = fakeBackend({ preview: 500 });
+  await run(fetchImpl, dirsFor);
+  const result = readFileSync(dirsFor.resultPath, 'utf8');
+  assert.equal(result.includes(PASSWORD), false);
+  assert.equal(result.includes('@example.invalid'), false);
+  const [manifestFile] = readdirSync(dirsFor.stateDir);
+  const manifest = readFileSync(path.join(dirsFor.stateDir, manifestFile), 'utf8');
+  assert.equal(manifest.includes(PASSWORD), false);
+  assert.equal(manifest.includes('@example.invalid'), false);
+});
+
+test('ambiguous pod create with no match in the list: the manifest records the pod name as needs-admin-purge', async () => {
+  const dirsFor = dirs();
+  // The create answers 500 and commits nothing, and the list has no pod with
+  // this run's name: the outcome cannot be checked, so the name is recorded.
+  const { fetchImpl, state } = fakeBackend({ create: 'error' });
+  const { exitCode, result } = await run(fetchImpl, dirsFor);
+
+  assert.equal(exitCode, 1);
+  assert.equal(result.steps.find((s) => s.name === 'first-pod').result, 'fail');
+  assert.match(result.steps.find((s) => s.name === 'first-pod').reason, /pod create 500/);
+
+  const [manifestFile] = readdirSync(dirsFor.stateDir);
+  const manifest = JSON.parse(readFileSync(path.join(dirsFor.stateDir, manifestFile), 'utf8'));
+  const podName = `${manifest.username} workspace`;
+  assert.deepEqual(manifest.unresolvedPodNames, [podName]);
+  assert.equal(manifest.status, 'needs-admin-purge');
+  assert.ok(manifest.survivors.some((s) => s.includes(`"${podName}"`) && s.includes('create outcome unknown')));
+
+  // The result file carries the same cleanup verdict.
+  assert.equal(result.cleanup.status, 'needs-admin-purge');
+  assert.equal(result.cleanup.ok, false);
+  assert.equal(JSON.parse(readFileSync(dirsFor.resultPath, 'utf8')).overall, 'fail');
+  // The walk still removed its own account and the workspace pod it recorded.
+  assert.equal(state.accountExists, false);
+});
+
+test('a missing password fails before any request and still writes the signal', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl } = fakeBackend();
+  let requests = 0;
+  const counting = (url, init) => { requests += 1; return fetchImpl(url, init); };
+  const { exitCode, result } = await runWalk({
+    base: 'https://api.commonly.me',
+    env: {},
+    fetchImpl: counting,
+    ...dirsFor,
+    sleep: async () => {},
+    log: () => {},
+    error: () => {},
+  });
+  assert.equal(exitCode, 1);
+  assert.equal(result.overall, 'fail');
+  assert.equal(requests, 0);
+  assert.equal(existsSync(dirsFor.resultPath), true);
+});

From 5d5582345197c87f31e975f664ea841c154fe83e Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sat, 31 Oct 2026 20:36:47 -0700
Subject: [PATCH 2/3] TASK-729: fix the walk entry guard and close review gaps

- Entry guard resolves argv[1] with realpath: a symlinked path ran nothing
  and exited 0, which the result would read as a pass.
- Scrub the JSON-escaped form of the password and email too.
- A refused mode is reported as not-chosen, not signup; cleanup reports
  not-run (ok null) when it never started.
- sha comes only from WALK_SHA; the local checkout HEAD is not the deploy.
- Tests: signin mode never deletes the shared account; a refused mode makes
  no request; an echoed password with quotes and backslashes is scrubbed; a
  create that throws, and one that commits then answers 500.

Written by Forge, a Commonly agent.

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-authored-by: forge <WyJmb3JnZSIsImRlZmF1bHQiXQ@agents.commonly.invalid>
Agent-Model: claude/claude-haiku-5-5/default
---
 docs/runbooks/first-user-walk.md |   6 +-
 scripts/first-user-walk.mjs      |  38 ++++++-----
 scripts/first-user-walk.test.mjs | 112 ++++++++++++++++++++++++++++---
 3 files changed, 128 insertions(+), 28 deletions(-)

diff --git a/docs/runbooks/first-user-walk.md b/docs/runbooks/first-user-walk.md
index 241012717a..dc58407b78 100644
--- a/docs/runbooks/first-user-walk.md
+++ b/docs/runbooks/first-user-walk.md
@@ -86,22 +86,24 @@ Write is atomic (temp file, then rename), so a reader never sees a half file.
 ```json
 {
   "schema": 1,
-  "sha": "<deployed sha, from WALK_SHA or the repo HEAD>",
+  "sha": "<deployed sha from WALK_SHA, or null when the scheduler did not set it>",
   "startedAt": "...", "finishedAt": "...",
   "base": "https://api.commonly.me", "mode": "signup",
   "walk": "walk-<stamp>",
   "overall": "pass | fail | incomplete",
   "exitCode": 0,
   "steps": [{ "name": "signup", "result": "pass | fail | not-covered", "ms": 0, "reason": "only when not pass" }],
-  "cleanup": { "ok": true, "status": "clean | needs-admin-purge", "survivors": [] }
+  "cleanup": { "ok": true, "status": "clean | needs-admin-purge | not-run", "survivors": [] }
 }
 ```
 
+- The result is written at the end of a run. A run killed midway leaves the previous file, so check `finishedAt`.
 - `overall` and the exit code agree: `pass` = `0`, `fail` = `1`, `incomplete` = `2`.
 - `incomplete` means every covered step passed and a step is not covered yet
   (Bring your own connect). Today every full run is `incomplete`, never `pass`,
   until that step is built. Roll back on `fail` only; treat `incomplete` as
   not-green but not as a regression.
+- `cleanup.status` is `not-run` (and `ok` null) when the run stopped before cleanup could start, for example a refused mode.
 - The file holds no password, token or email. A `needs-admin-purge` cleanup
   points SRE at the manifest in `~/.commonly/state/walk-706/`.
 
diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs
index c80a383df9..646d153474 100644
--- a/scripts/first-user-walk.mjs
+++ b/scripts/first-user-walk.mjs
@@ -33,8 +33,7 @@
  * the run is `incomplete` (exit 2), so a green run never claims more than it checked.
  */
 
-import { execFileSync } from 'node:child_process';
-import { mkdirSync, renameSync, writeFileSync } from 'node:fs';
+import { mkdirSync, realpathSync, renameSync, writeFileSync } from 'node:fs';
 import { homedir } from 'node:os';
 import path from 'node:path';
 import { fileURLToPath } from 'node:url';
@@ -90,7 +89,9 @@ export const runWalk = async ({
   // True once signup may have created the account, so cleanup knows to look for it.
   let accountMayExist = false;
   // 'signup' makes a fresh account; 'signin' uses the shared staging account and never deletes it.
-  let walkMode = { mode: 'signup', deleteAccount: true };
+  // Null until chooseWalkMode accepts the run, so a refused run is never reported as signup.
+  let walkMode = null;
+  let cleanupRan = false;
   // Signup returns no user id, so the manifest carries the exact username instead:
   // SRE finds the account by that name, not by a prefix. Never the email.
   const manifest = { mode: null, userId: null, username: null, podIds: [], unresolvedPodNames: [], agentName: null, botIds: [], survivors: [], status: 'running' };
@@ -99,7 +100,11 @@ export const runWalk = async ({
   // the result file. An API error body can echo what was sent.
   const redact = (text) => {
     let out = String(text);
-    for (const secret of [password, email]) if (secret) out = out.split(secret).join('[redacted]');
+    for (const secret of [password, email]) {
+      if (!secret) continue;
+      // The JSON-escaped form too: a body echo puts a quote or backslash in escaped form.
+      for (const form of new Set([secret, JSON.stringify(secret).slice(1, -1)])) out = out.split(form).join('[redacted]');
+    }
     return out;
   };
 
@@ -215,12 +220,15 @@ export const runWalk = async ({
       startedAt,
       finishedAt: new Date(now()).toISOString(),
       base: BASE,
-      mode: walkMode.mode,
+      mode: walkMode ? walkMode.mode : 'not-chosen',
       walk: username,
       overall,
       exitCode,
       steps: results,
-      cleanup: { ok: cleanupOk, status: manifest.status, survivors: manifest.survivors.map(redact) },
+      // 'not-run' and ok null when cleanup never ran (a refused or failed setup).
+      cleanup: cleanupRan
+        ? { ok: cleanupOk, status: manifest.status, survivors: manifest.survivors.map(redact) }
+        : { ok: null, status: 'not-run', survivors: [] },
     };
     mkdirSync(path.dirname(resultPath), { recursive: true, mode: 0o700 });
     const tmp = `${resultPath}.${process.pid}.tmp`;
@@ -354,6 +362,7 @@ export const runWalk = async ({
     } catch {
       exitCode = 1;
     } finally {
+      cleanupRan = true;
       cleanupOk = await cleanup(token);
       if (!cleanupOk) exitCode = 1;
     }
@@ -374,18 +383,13 @@ const parseArgs = (argv) => {
   return i >= 0 ? argv[i + 1] : undefined;
 };
 
-// Deployed sha for the result: an explicit WALK_SHA from the scheduler (the
-// deploy being walked), else the HEAD of the repo this script runs from.
-const shaFromEnv = (env) => {
-  if (env.WALK_SHA) return env.WALK_SHA;
-  try {
-    return execFileSync('git', ['rev-parse', 'HEAD'], { cwd: path.dirname(fileURLToPath(import.meta.url)), stdio: ['ignore', 'pipe', 'ignore'] }).toString().trim();
-  } catch {
-    return null;
-  }
-};
+// The deployed sha is the one the scheduler names (WALK_SHA). The local checkout's
+// HEAD is not the deploy, so a rollback must never target it: no WALK_SHA, no sha.
+const shaFromEnv = (env) => env.WALK_SHA || null;
 
-const isEntry = process.argv[1] && path.resolve(process.argv[1]) === fileURLToPath(import.meta.url);
+// import.meta.url is the resolved path, so argv[1] is resolved the same way: a
+// symlinked path (/tmp on macOS) must still run the walk, not exit 0 silently.
+const isEntry = process.argv[1] && realpathSync(process.argv[1]) === fileURLToPath(import.meta.url);
 if (isEntry) {
   const base = parseArgs(process.argv.slice(2)) || process.env.COMMONLY_API_URL || 'https://api.commonly.me';
   runWalk({ base, env: process.env, sha: shaFromEnv(process.env) })
diff --git a/scripts/first-user-walk.test.mjs b/scripts/first-user-walk.test.mjs
index 842776dd75..eebdc48f4e 100644
--- a/scripts/first-user-walk.test.mjs
+++ b/scripts/first-user-walk.test.mjs
@@ -11,12 +11,19 @@ import { runWalk } from './first-user-walk.mjs';
 const PASSWORD = 'pw-secret-for-tests-9f2';
 const USER_ID = 'user-walk';
 const TOKEN = 'token-for-tests';
+const SHARED_EMAIL = 'walk-shared@commonly.test';
 
 // Options:
 //   preview       status of the invite preview GET (500 forces a step failure)
-//   create        'ok' creates the pod; 'error' returns 500 and creates nothing
-const fakeBackend = ({ preview = 200, create = 'ok' } = {}) => {
-  const state = { accountExists: false, pods: [], lastAgent: null, calls: [] };
+//   create        'ok' creates the pod; 'error' returns 500 and creates nothing;
+//                 'throw' fails the request; 'commit-then-error' commits, then returns 500
+//   password      the password the fake account accepts
+//   echoRegister  a refused register echoes the request body back, as a bad error would
+//   seeded        the account already exists (the shared staging sign-in account)
+const fakeBackend = ({
+  preview = 200, create = 'ok', password = PASSWORD, echoRegister = false, seeded = false,
+} = {}) => {
+  const state = { accountExists: seeded, pods: [], lastAgent: null, calls: [] };
   const reply = (status, data) => ({ status, json: async () => data });
   const fetchImpl = async (url, init = {}) => {
     const u = new URL(url);
@@ -26,19 +33,26 @@ const fakeBackend = ({ preview = 200, create = 'ok' } = {}) => {
     state.calls.push(`${method} ${p}`);
 
     if (method === 'POST' && p === '/api/auth/register') {
+      if (echoRegister) return reply(400, { error: 'bad request', echo: body });
       state.accountExists = true;
       // The walk's own workspace pod, as signup creates it.
       state.pods.push({ _id: 'ws1', name: 'Workspace', createdBy: USER_ID, members: [] });
       return reply(201, {});
     }
     if (method === 'POST' && p === '/api/auth/login') {
-      if (state.accountExists && body.password === PASSWORD) {
+      if (state.accountExists && body.password === password) {
         return reply(200, { token: TOKEN, user: { _id: USER_ID, role: 'user' } });
       }
       return reply(401, { code: 'bad_credentials' });
     }
     if (method === 'GET' && p === '/api/pods') return reply(200, state.pods.map((x) => ({ ...x })));
     if (method === 'POST' && p === '/api/pods') {
+      if (create === 'throw') throw new Error('socket hang up');
+      if (create === 'commit-then-error') {
+        // The pod is committed, and the reply still says 500.
+        state.pods.push({ _id: 'p-committed', name: body.name, createdBy: USER_ID, members: [] });
+        return reply(500, { error: 'timeout after commit' });
+      }
       if (create !== 'ok') return reply(500, { error: 'boom' });
       const pod = { _id: `p${state.pods.length + 1}`, name: body.name, createdBy: USER_ID, members: [] };
       state.pods.push(pod);
@@ -81,9 +95,9 @@ const dirs = () => {
   return { stateDir: path.join(root, 'state'), resultPath: path.join(root, 'result', 'result.json') };
 };
 
-const run = (fetchImpl, { stateDir, resultPath }) => runWalk({
-  base: 'https://api.commonly.me',
-  env: { COMMONLY_WALK_PASSWORD: PASSWORD },
+const run = (fetchImpl, { stateDir, resultPath }, { password = PASSWORD, env = {}, base = 'https://api.commonly.me' } = {}) => runWalk({
+  base,
+  env: { COMMONLY_WALK_PASSWORD: password, ...env },
   fetchImpl,
   stateDir,
   resultPath,
@@ -149,6 +163,55 @@ test('the result and manifest never carry the password or the email', async () =
   assert.equal(manifest.includes('@example.invalid'), false);
 });
 
+test('a password with quotes and backslashes is scrubbed even when a body echoes it escaped', async () => {
+  const dirsFor = dirs();
+  const tricky = 'pa"ss\\word-7';
+  const { fetchImpl } = fakeBackend({ password: tricky, echoRegister: true });
+  const { result } = await run(fetchImpl, dirsFor, { password: tricky });
+
+  const signup = result.steps.find((s) => s.name === 'signup');
+  assert.equal(signup.result, 'fail');
+  assert.match(signup.reason, /register 400/);
+  // Neither the raw nor the JSON-escaped form of the password reaches the reason.
+  assert.equal(signup.reason.includes('pa"ss'), false);
+  assert.equal(signup.reason.includes('pa\\"ss'), false);
+  assert.equal(signup.reason.includes('word-7'), false);
+  assert.equal(readFileSync(dirsFor.resultPath, 'utf8').includes('word-7'), false);
+});
+
+test('signin mode never deletes the shared account, and the result says so', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl, state } = fakeBackend({ seeded: true });
+  const { exitCode, result } = await run(fetchImpl, dirsFor, {
+    base: 'https://api-staging.commonly.me',
+    env: { COMMONLY_WALK_EMAIL: SHARED_EMAIL },
+  });
+
+  assert.equal(result.mode, 'signin');
+  assert.equal(result.cleanup.ok, true);
+  assert.equal(exitCode, 2);
+  assert.equal(state.calls.includes('DELETE /api/users/me'), false);
+  assert.equal(state.calls.includes('POST /api/auth/register'), false);
+  assert.equal(state.accountExists, true);
+  // The run removed the pod it made, and nothing else.
+  assert.equal(state.pods.length, 0);
+  assert.equal(result.steps.some((s) => s.name === 'signup-workspace'), false);
+});
+
+test('a refused mode (shared email aimed at prod) makes no request, is not reported as signup, and exits 1', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl, state } = fakeBackend({ seeded: true });
+  const { exitCode, result } = await run(fetchImpl, dirsFor, { env: { COMMONLY_WALK_EMAIL: SHARED_EMAIL } });
+
+  assert.equal(exitCode, 1);
+  assert.equal(result.mode, 'not-chosen');
+  assert.equal(result.overall, 'fail');
+  assert.equal(result.cleanup.status, 'not-run');
+  assert.equal(result.cleanup.ok, null);
+  assert.equal(state.calls.length, 0);
+  assert.equal(JSON.parse(readFileSync(dirsFor.resultPath, 'utf8')).mode, 'not-chosen');
+});
+
 test('ambiguous pod create with no match in the list: the manifest records the pod name as needs-admin-purge', async () => {
   const dirsFor = dirs();
   // The create answers 500 and commits nothing, and the list has no pod with
@@ -171,11 +234,12 @@ test('ambiguous pod create with no match in the list: the manifest records the p
   assert.equal(result.cleanup.status, 'needs-admin-purge');
   assert.equal(result.cleanup.ok, false);
   assert.equal(JSON.parse(readFileSync(dirsFor.resultPath, 'utf8')).overall, 'fail');
-  // The walk still removed its own account and the workspace pod it recorded.
+  // The walk removed its own account and the workspace pod it recorded.
   assert.equal(state.accountExists, false);
+  assert.equal(state.pods.some((x) => x._id === 'ws1'), false);
 });
 
-test('a missing password fails before any request and still writes the signal', async () => {
+test('a missing password fails before any request, reports cleanup as not-run, and still writes the signal', async () => {
   const dirsFor = dirs();
   const { fetchImpl } = fakeBackend();
   let requests = 0;
@@ -191,6 +255,36 @@ test('a missing password fails before any request and still writes the signal',
   });
   assert.equal(exitCode, 1);
   assert.equal(result.overall, 'fail');
+  assert.equal(result.cleanup.status, 'not-run');
   assert.equal(requests, 0);
   assert.equal(existsSync(dirsFor.resultPath), true);
 });
+
+test('a create that throws, with no match in the list, is recorded as unresolved and needs-admin-purge', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl, state } = fakeBackend({ create: 'throw' });
+  const { exitCode, result } = await run(fetchImpl, dirsFor);
+
+  assert.equal(exitCode, 1);
+  assert.match(result.steps.find((s) => s.name === 'first-pod').reason, /socket hang up/);
+  const [manifestFile] = readdirSync(dirsFor.stateDir);
+  const manifest = JSON.parse(readFileSync(path.join(dirsFor.stateDir, manifestFile), 'utf8'));
+  assert.deepEqual(manifest.unresolvedPodNames, [`${manifest.username} workspace`]);
+  assert.equal(manifest.status, 'needs-admin-purge');
+  assert.equal(state.accountExists, false);
+});
+
+test('a create that commits then answers 500 is found by name, removed, and leaves no survivor', async () => {
+  const dirsFor = dirs();
+  const { fetchImpl, state } = fakeBackend({ create: 'commit-then-error' });
+  const { exitCode, result } = await run(fetchImpl, dirsFor);
+
+  assert.equal(exitCode, 1);
+  assert.match(result.steps.find((s) => s.name === 'first-pod').reason, /pod create 500/);
+  const [manifestFile] = readdirSync(dirsFor.stateDir);
+  const manifest = JSON.parse(readFileSync(path.join(dirsFor.stateDir, manifestFile), 'utf8'));
+  assert.deepEqual(manifest.unresolvedPodNames, []);
+  assert.ok(manifest.podIds.includes('p-committed'));
+  assert.equal(state.pods.some((x) => x._id === 'p-committed'), false);
+  assert.equal(result.cleanup.status, 'clean');
+});

From 48f943c0aefd610f32c2eda5fb4b8581688dba47 Mon Sep 17 00:00:00 2001
From: Lily Shen <115414357+lilyshen0722@users.noreply.github.com>
Date: Sat, 31 Oct 2026 20:40:40 -0700
Subject: [PATCH 3/3] TASK-729: a throwing cleanup is never reported ok;
 document not-chosen

Co-Authored-By: Claude Haiku 5.5 <noreply@anthropic.com>
Co-authored-by: forge <WyJmb3JnZSIsImRlZmF1bHQiXQ@agents.commonly.invalid>
Agent-Model: claude/claude-haiku-5-5/default
---
 docs/runbooks/first-user-walk.md | 1 +
 scripts/first-user-walk.mjs      | 3 ++-
 2 files changed, 3 insertions(+), 1 deletion(-)

diff --git a/docs/runbooks/first-user-walk.md b/docs/runbooks/first-user-walk.md
index dc58407b78..2724dd5698 100644
--- a/docs/runbooks/first-user-walk.md
+++ b/docs/runbooks/first-user-walk.md
@@ -103,6 +103,7 @@ Write is atomic (temp file, then rename), so a reader never sees a half file.
   (Bring your own connect). Today every full run is `incomplete`, never `pass`,
   until that step is built. Roll back on `fail` only; treat `incomplete` as
   not-green but not as a regression.
+- `mode` is `signup` or `signin`, or `not-chosen` when the run was refused before a mode was accepted (no request was made).
 - `cleanup.status` is `not-run` (and `ok` null) when the run stopped before cleanup could start, for example a refused mode.
 - The file holds no password, token or email. A `needs-admin-purge` cleanup
   points SRE at the manifest in `~/.commonly/state/walk-706/`.
diff --git a/scripts/first-user-walk.mjs b/scripts/first-user-walk.mjs
index 646d153474..c9a1b8f259 100644
--- a/scripts/first-user-walk.mjs
+++ b/scripts/first-user-walk.mjs
@@ -238,7 +238,8 @@ export const runWalk = async ({
   };
 
   let exitCode = 0;
-  let cleanupOk = true;
+  // Starts false: a cleanup that throws must not be reported as ok.
+  let cleanupOk = false;
   try {
     expect(password, 'COMMONLY_WALK_PASSWORD is not set');
     walkMode = chooseWalkMode({ base: BASE, env });