fix(e2e): resolve the measurement evidence path instead of hardcoding it (#549)

docs/AGENTS.md

@@ -61,4 +61,4 @@ Files in this directory. One row per file. Topic docs + repo-root config (root c
 | `skills-as-subagents.md` | Skill↔subagent bridge analysis. Wrap skill via thin `.pi/agents/<Name>.md` (model role, `inherit_context`,… → see `skills-as-subagents.md.AGENTS.md` |
 | `slash-command.md` | Bridge routes typed `/foo` chat text to pi handlers. `parseSendPrompt` + `bridge.ts::sessionPrompt` 11-step order. Extension command dispatch fix (`pi.dispatchCommand`, RPC keeper). |
 | `ui-contract.md` | (repo root) Cross-screen design control plane. Single source of truth for visual consistency. References design tokens only, never redefines, forbids raw hex/px. Token authority `packages/client/src/index.css`. Two themes ship: `:root` (dark, default) + `[data-theme="light"]` (32-token override). Theme-system skill "4 themes" claim stale — no such selectors. Prefer `--severity-{error,warning,success,info,neutral}-{bg,fg,border}`, `--status-*`, `--warn-*`; new surfaces use severity, not raw `--accent-*`. Carries spacing scale, type scale (floor: below `text-[11px]` non-essential), radius, elevation (`--elevation-rim` inset signature), component invariants, motion, WCAG 2.2 AA invariants. Debt, do-not-copy: `SessionCard.tsx` off-token `border-blue-500/30`; `BranchSwitchDialog.tsx` missing `role`/`aria-modal`/labelled title. Created by frontend-mockup-loop skill CONTRACT step. See change: share-session-archives. |
-| `vitest.config.ts` | (repo root) Root Vitest config. `defineConfig`. Vitest 4 dropped `vitest.workspace.ts`; projects live under… → see `vitest.config.ts.AGENTS.md` |
+| `vitest.config.ts` | (repo root) Root Vitest config. `defineConfig`. Vitest 4 dropped `vitest.workspace.ts`. Projects live under `test.projects`. New `tests` project. Points at `tests/vitest.config.ts`. Collects `tests/e2e/helpers/__tests__/` only. Playwright specs excluded. Need docker harness + browser. Run via `npm run test:e2e`. Added for pure helper. Used only by opt-in specs (`PI_SYNTH_AGENT_TICKS=1`). No CI job executed it. See issue #549. → see `vitest.config.ts.AGENTS.md` |

docs/vitest.config.ts.AGENTS.md

@@ -1,3 +1,3 @@
 # vitest.config.ts — index
 
-(repo root) Root Vitest config. `defineConfig`. Vitest 4 dropped `vitest.workspace.ts`; projects live under `test.projects`. `test.projects` array registers each per-package `vitest.config.ts`; each carries own environment (jsdom client / node server+shared+extension), include globs, pool settings. `packages/shell` registered in `test.projects`. `.pi/skills/ship-it` registered in `test.projects` alongside `packages/*` and `scripts`; before it nothing under `.pi/skills/` was collected by any project, so ship-it decision helpers `manifest.ts` + `no-weakening.ts` shipped untested while gating real ship decisions. Root-level `globalSetup: ["./scripts/mutation-journal-global-setup.mjs"]` reconciles `.mutation-journal/` before ANY project runs; root placement deliberate — per-project setup races concurrent forks. Throw on conflict aborts whole run. See change: add-server-keypair-pairing, wire-local-review-gate, harden-mutation-harness-restore.
+(repo root) Root Vitest config. `defineConfig`. Vitest 4 dropped `vitest.workspace.ts`. Projects live under `test.projects`. `test.projects` array registers each per-package `vitest.config.ts`. Each carries own environment (jsdom client / node server+shared+extension), include globs, pool settings. `packages/shell` registered in `test.projects`. `.pi/skills/ship-it` registered in `test.projects` alongside `packages/` and `scripts/`. New `tests` project registered right after `scripts`. Points at `tests/vitest.config.ts`. Include `tests/e2e/helpers/__tests__/` only. Playwright specs in `tests/e2e/` never collected. Need docker harness + browser. Driven by `npm run test:e2e`. Added because pure helper `tests/e2e/helpers/evidence-path.ts` used only by two opt-in specs (`PI_SYNTH_AGENT_TICKS=1`). No CI job could execute its logic. See issue #549. Before it nothing under `.pi/skills/` was collected by any project, so ship-it decision helpers `manifest.ts` + `no-weakening.ts` shipped untested while gating real ship decisions. Root-level `globalSetup: ["./scripts/mutation-journal-global-setup.mjs"]` reconciles `.mutation-journal/` before ANY project runs; root placement deliberate — per-project setup races concurrent forks. Throw on conflict aborts whole run. See change: add-server-keypair-pairing, wire-local-review-gate, harden-mutation-harness-restore.

tests/AGENTS.md

@@ -0,0 +1,8 @@
+# DOX — tests
+
+Files in this directory. One row per file. Non-source area. Subdirectories own their own rows (`e2e/AGENTS.md`, `e2e-electron/AGENTS.md`).
+
+| File | Purpose |
+|------|---------|
+| `tsconfig.json` | TS config for the E2E suites. Typechecked by `npm run lint:e2e` (`tsc -p tests/tsconfig.json --noEmit`); the root tsconfig only includes `packages/*/src`, so nothing under `tests/` is seen without it. |
+| `vitest.config.ts` | Vitest project for repo-root `/tests`, registered as `"tests"` in the root `vitest.config.ts`. Include glob is deliberately narrow — `e2e/helpers/__tests__/**/*.test.ts` ONLY; Playwright specs (`e2e/*.spec.ts`) need a docker harness + browser and must never be collected here (`npm run test:e2e` drives those). Added so pure helper logic used exclusively by opt-in specs is still exercised by normal CI. See issue #549. |

tests/e2e/AGENTS.md

@@ -46,6 +46,8 @@ Files in this directory. One row per file. Non-source area (migrated from `docs/
 | `global-setup.ts` | Playwright globalSetup. `PW_E2E_USE_RUNNING=1` → only verify `/api/health` (30s). → see `global-setup.ts.AGENTS.md` |
 | `global-teardown.ts` | Playwright globalTeardown. Managed (marker present, not fast path) → run `docker/test-down.sh` with… → see `global-teardown.ts.AGENTS.md` |
 | `headless-reload-dispatch.spec.ts` | L3 for `fix-out-of-band-reload` (#F1–#F3). Bare `/reload` on a headless harness session: one terminal… → see `headless-reload-dispatch.spec.ts.AGENTS.md` |
+| `helpers/__tests__/evidence-path.test.ts` | Unit tests (vitest `tests` project, NOT Playwright) for `helpers/evidence-path.ts`. 25 cases: active-dir preference, newest-archive pick, prefix-collision rejection, single-component `changeName` guard (`../..` must not resolve to repo root), evidence preserved on malformed/non-object JSON, and the fail-loud path. Runs in normal CI because both consuming specs are opt-in. See issue #549. |
+| `helpers/evidence-path.ts` | Resolves a change's `measurements.json` WITHOUT creating it. `resolveEvidencePath(name, root?)` prefers `openspec/changes/<name>/`, falls back to the newest `openspec/changes/archive/<YYYY-MM-DD>-<name>/`, else THROWS; `findChangeDir`, `recordMeasurement(name, key, value)`, `readEvidence(path)` (ENOENT → `{}`; malformed/non-object JSON THROWS so a damaged file is never overwritten), `EVIDENCE_FILENAME`, `REPO_ROOT`. `changeName` must be a single path component. Replaces the hardcoded pre-archive path + `mkdirSync` that conjured a phantom active-change dir and silently misdirected evidence writes. See issue #549. |
 | `helpers/index.ts` | E2E helpers. `gotoDashboard(page)` navigates `/`, waits for `header-app-bar`, and arms a one-per-page… → see `helpers/index.ts.AGENTS.md` |
 | `helpers/openspec-board.ts` | OpenSpec-board drop-targeting E2E helpers. Fixture `/fixtures/openspec-board` (64 generated `board-card-NN`… → see `helpers/openspec-board.ts.AGENTS.md` |
 | `history-backfill-gap.spec.ts` | L3 for `fix-lazy-history-backfill-ux` (F1–F6, F8–F11, X3, X5 — twelve scenarios). → see `history-backfill-gap.spec.ts.AGENTS.md` |

tests/e2e/helpers/__tests__/evidence-path.test.ts

@@ -0,0 +1,164 @@
+/**
+ * Unit tests for the measurement-evidence path resolver (#549).
+ *
+ * These run under the `tests` vitest project, NOT Playwright: the resolver is
+ * pure filesystem logic and must be exercised by normal CI, because both specs
+ * that use it are opt-in (`PI_SYNTH_AGENT_TICKS=1`) and therefore never run
+ * there.
+ *
+ * The negative cases carry the weight. The bug this fixes was a SILENT
+ * misdirected write: `mkdirSync(..., { recursive: true })` conjured a phantom
+ * active-change directory once the change was archived, so a re-measure
+ * appeared to leave the archived numbers untouched while recording different
+ * ones elsewhere. "Throws when neither location exists" is the whole point —
+ * a resolver that quietly invents a path reintroduces the bug.
+ */
+import { chmodSync, existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from "node:fs";
+import { tmpdir } from "node:os";
+import { join } from "node:path";
+import { afterEach, beforeEach, describe, expect, it } from "vitest";
+
+import { EVIDENCE_FILENAME, readEvidence, resolveEvidencePath } from "../evidence-path.js";
+
+const CHANGE = "verify-subagent-pull-under-load";
+
+let root: string;
+
+beforeEach(() => {
+  root = mkdtempSync(join(tmpdir(), "evidence-path-"));
+});
+
+afterEach(() => {
+  rmSync(root, { recursive: true, force: true });
+});
+
+/** Create `openspec/changes/<rel>` under the fake repo root. */
+function changeDir(rel: string): string {
+  const dir = join(root, "openspec", "changes", rel);
+  mkdirSync(dir, { recursive: true });
+  return dir;
+}
+
+describe("active change directory", () => {
+  it("resolves into the active dir when it exists", () => {
+    const dir = changeDir(CHANGE);
+    expect(resolveEvidencePath(CHANGE, root)).toBe(join(dir, EVIDENCE_FILENAME));
+  });
+
+  it("prefers the active dir over an archived copy", () => {
+    const active = changeDir(CHANGE);
+    changeDir(`archive/2026-08-24-${CHANGE}`);
+    expect(resolveEvidencePath(CHANGE, root)).toBe(join(active, EVIDENCE_FILENAME));
+  });
+});
+
+describe("archived change directory", () => {
+  it("falls back to the archived dir once the change is archived", () => {
+    const archived = changeDir(`archive/2026-08-24-${CHANGE}`);
+    expect(resolveEvidencePath(CHANGE, root)).toBe(join(archived, EVIDENCE_FILENAME));
+  });
+
+  it("picks the NEWEST archive when a reopened change was archived twice", () => {
+    changeDir(`archive/2026-08-24-${CHANGE}`);
+    const newer = changeDir(`archive/2026-09-02-${CHANGE}`);
+    expect(resolveEvidencePath(CHANGE, root)).toBe(join(newer, EVIDENCE_FILENAME));
+  });
+
+  it("does not match a different change that merely shares a prefix", () => {
+    changeDir(`archive/2026-08-24-${CHANGE}-followup`);
+    expect(() => resolveEvidencePath(CHANGE, root)).toThrow();
+  });
+
+  it("ignores a plain file that happens to match the archive pattern", () => {
+    mkdirSync(join(root, "openspec", "changes", "archive"), { recursive: true });
+    writeFileSync(join(root, "openspec", "changes", "archive", `2026-08-24-${CHANGE}`), "not a dir");
+    expect(() => resolveEvidencePath(CHANGE, root)).toThrow();
+  });
+});
+
+describe("changeName must be a single path component", () => {
+  // `join(changesDir, "../..")` escapes to the repo root, which IS a directory,
+  // so without this guard a traversal name resolves to a real path and the
+  // measurement lands outside openspec/ entirely.
+  it.each(["..", "../..", "a/b", "a\\b", "/abs", ""])("rejects %o", (name) => {
+    changeDir(CHANGE); // a valid change exists, so only the name can be at fault
+    expect(() => resolveEvidencePath(name, root)).toThrow();
+  });
+
+  it("a traversal name cannot resolve to the repo root", () => {
+    changeDir(CHANGE);
+    let resolved: string | null = null;
+    try {
+      resolved = resolveEvidencePath("../..", root);
+    } catch {
+      /* expected */
+    }
+    expect(resolved).toBeNull();
+  });
+});
+
+describe("existing evidence is preserved, never silently replaced", () => {
+  function withEvidence(contents: string): string {
+    const dir = changeDir(CHANGE);
+    const file = join(dir, EVIDENCE_FILENAME);
+    writeFileSync(file, contents);
+    return file;
+  }
+
+  it("returns {} when the file does not exist yet (first write)", () => {
+    changeDir(CHANGE);
+    expect(readEvidence(resolveEvidencePath(CHANGE, root))).toEqual({});
+  });
+
+  it("reads back existing keys", () => {
+    const file = withEvidence('{"P1": 1}\n');
+    expect(readEvidence(file)).toEqual({ P1: 1 });
+  });
+
+  it("THROWS on malformed JSON instead of discarding the file", () => {
+    const file = withEvidence("{ truncated mid-writ");
+    expect(() => readEvidence(file)).toThrow(/parse|JSON/i);
+    // The evidence that could not be parsed must still be on disk.
+    expect(readFileSync(file, "utf8")).toBe("{ truncated mid-writ");
+  });
+
+  it.each(['["a"]', "42", '"text"', "null"])("THROWS on non-object JSON %s", (json) => {
+    const file = withEvidence(json);
+    expect(() => readEvidence(file)).toThrow(/object/i);
+  });
+
+  it("rethrows a read error that is not ENOENT", () => {
+    const file = withEvidence('{"P1": 1}');
+    chmodSync(file, 0o000);
+    try {
+      // Root ignores mode bits; skip rather than assert a false expectation.
+      if (process.getuid?.() === 0) return;
+      expect(() => readEvidence(file)).toThrow();
+    } finally {
+      chmodSync(file, 0o644);
+    }
+  });
+});
+
+describe("neither location exists — fail loud, never create", () => {
+  it("throws instead of returning a path", () => {
+    mkdirSync(join(root, "openspec", "changes"), { recursive: true });
+    expect(() => resolveEvidencePath(CHANGE, root)).toThrow(/verify-subagent-pull-under-load/);
+  });
+
+  it("names both searched locations so the failure is actionable", () => {
+    mkdirSync(join(root, "openspec", "changes"), { recursive: true });
+    expect(() => resolveEvidencePath(CHANGE, root)).toThrow(/archive/);
+  });
+
+  it("throws when openspec/ is absent entirely", () => {
+    expect(() => resolveEvidencePath(CHANGE, root)).toThrow();
+  });
+
+  it("creates no directory as a side effect of failing", () => {
+    mkdirSync(join(root, "openspec", "changes"), { recursive: true });
+    expect(() => resolveEvidencePath(CHANGE, root)).toThrow();
+    // The phantom active-change directory in #549 is exactly what must NOT appear.
+    expect(existsSync(join(root, "openspec", "changes", CHANGE))).toBe(false);
+  });
+});