diff --git a/docs/AGENTS.md b/docs/AGENTS.md index f4768bf1e..dc600a9d5 100644 --- a/docs/AGENTS.md +++ b/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/.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` | diff --git a/docs/vitest.config.ts.AGENTS.md b/docs/vitest.config.ts.AGENTS.md index c7bd83305..cc2fe99a6 100644 --- a/docs/vitest.config.ts.AGENTS.md +++ b/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. diff --git a/tests/AGENTS.md b/tests/AGENTS.md new file mode 100644 index 000000000..acf65a373 --- /dev/null +++ b/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. | diff --git a/tests/e2e/AGENTS.md b/tests/e2e/AGENTS.md index 88f0b8f54..00627206a 100644 --- a/tests/e2e/AGENTS.md +++ b/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`. 30 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, reserved keys (`__proto__`) persisted as own properties, 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//`, falls back to the newest `openspec/changes/archive/-/`, else THROWS; `findChangeDir`, `recordMeasurement(name, key, value, repoRoot?)` (`Object.defineProperty`, never `obj[key]=`), `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` | diff --git a/tests/e2e/helpers/__tests__/evidence-path.test.ts b/tests/e2e/helpers/__tests__/evidence-path.test.ts new file mode 100644 index 000000000..8a9eef6d0 --- /dev/null +++ b/tests/e2e/helpers/__tests__/evidence-path.test.ts @@ -0,0 +1,188 @@ +/** + * 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, recordMeasurement, 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/` 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("every key is persisted as an own property", () => { + // `current[key] = v` with key "__proto__" mutates the prototype instead of + // adding a property, so the measurement never reaches the JSON. Asserted + // through the round-trip to disk, not on the in-memory object. + it.each(["__proto__", "constructor", "prototype", "toString"])("records the reserved key %o", (key) => { + const dir = changeDir(CHANGE); + recordMeasurement(CHANGE, key, { n: 1 }, root); + + const onDisk = JSON.parse(readFileSync(join(dir, EVIDENCE_FILENAME), "utf8")); + expect(Object.hasOwn(onDisk, key)).toBe(true); + expect(onDisk[key]).toEqual({ n: 1 }); + }); + + it("a reserved key does not displace an ordinary measurement", () => { + const dir = changeDir(CHANGE); + recordMeasurement(CHANGE, "F1", 42, root); + recordMeasurement(CHANGE, "__proto__", { n: 1 }, root); + + const onDisk = JSON.parse(readFileSync(join(dir, EVIDENCE_FILENAME), "utf8")); + expect(onDisk.F1).toBe(42); + expect(Object.hasOwn(onDisk, "__proto__")).toBe(true); + }); +}); + +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); + }); +}); diff --git a/tests/e2e/helpers/evidence-path.ts b/tests/e2e/helpers/evidence-path.ts new file mode 100644 index 000000000..ba1f44b75 --- /dev/null +++ b/tests/e2e/helpers/evidence-path.ts @@ -0,0 +1,170 @@ +import { readdirSync, readFileSync, statSync, writeFileSync } from "node:fs"; +import { dirname, join, resolve } from "node:path"; +import { fileURLToPath } from "node:url"; + +/** + * Resolve where a change's recorded measurements live — WITHOUT creating it. + * + * Both measurement specs used to hardcode + * `openspec/changes/verify-subagent-pull-under-load/measurements.json` and call + * `mkdirSync(..., { recursive: true })`. Once the change was archived, that + * combination CONJURED the pre-archive directory back into existence: tooling + * that enumerates `openspec/changes/*` then saw a phantom active change, and — + * worse for evidence integrity — the write landed somewhere other than the + * archived `heap-evidence.md`'s source of truth, so a re-measure looked like it + * left the archived numbers unchanged while actually recording different ones + * elsewhere. That is precisely the staleness the evidence file exists to + * prevent. See issue #549. + * + * So this resolver never creates anything. It reports where the change + * genuinely is, and throws when it cannot tell — a misdirected write must be + * loud, not silent. + */ + +/** The recorded-measurements file inside a change directory. */ +export const EVIDENCE_FILENAME = "measurements.json"; + +/** Repo root, derived from this file rather than from cwd (specs run from varying cwds). */ +export const REPO_ROOT = resolve(dirname(fileURLToPath(import.meta.url)), "..", "..", ".."); + +/** + * Archived changes are `archive/-`. Anchored at both ends so + * `verify-x` does not match `verify-x-followup`. + */ +function archiveEntryMatcher(changeName: string): RegExp { + const escaped = changeName.replace(/[.*+?^${}()|[\]\\]/g, "\\$&"); + return new RegExp(`^\\d{4}-\\d{2}-\\d{2}-${escaped}$`); +} + +/** + * `changeName` must be ONE path component. Without this, `join(changesDir, + * "../..")` escapes to the repo root — a real directory — so a traversal name + * resolves successfully and the measurement lands outside `openspec/`. + */ +function assertSingleComponent(changeName: string): void { + const bad = + changeName.length === 0 || + changeName === "." || + changeName === ".." || + changeName.includes("/") || + changeName.includes("\\"); + if (bad) { + throw new Error( + `Invalid change name ${JSON.stringify(changeName)}: expected a single directory name ` + + "under openspec/changes/, with no path separators.", + ); + } +} + +function isDirectory(path: string): boolean { + try { + return statSync(path).isDirectory(); + } catch { + return false; + } +} + +/** + * Directory of `changeName`: the active one, else the NEWEST archived one. + * Returns `null` when the change exists in neither place. + */ +export function findChangeDir(changeName: string, repoRoot: string = REPO_ROOT): string | null { + assertSingleComponent(changeName); + const changesDir = join(repoRoot, "openspec", "changes"); + + const active = join(changesDir, changeName); + if (isDirectory(active)) return active; + + const archiveDir = join(changesDir, "archive"); + let entries: string[]; + try { + entries = readdirSync(archiveDir); + } catch { + return null; + } + + const matcher = archiveEntryMatcher(changeName); + // A reopened change can be archived more than once; the date prefix is fixed + // width, so a lexicographic sort puts the newest last. + const matches = entries + .filter((entry) => matcher.test(entry)) + .filter((entry) => isDirectory(join(archiveDir, entry))) + .sort(); + + const newest = matches.at(-1); + return newest ? join(archiveDir, newest) : null; +} + +/** + * Absolute path of `changeName`'s measurements file. + * Throws when the change is in neither the active nor the archived location — + * never creates the directory, which is what produced the phantom change dir. + */ +export function resolveEvidencePath(changeName: string, repoRoot: string = REPO_ROOT): string { + const dir = findChangeDir(changeName, repoRoot); + if (dir === null) { + throw new Error( + `Cannot record measurements: change "${changeName}" was found in neither\n` + + ` ${join(repoRoot, "openspec", "changes", changeName)}\n` + + ` ${join(repoRoot, "openspec", "changes", "archive")}/-${changeName}\n` + + "Refusing to create it — a measurement written to a conjured directory is invisible " + + "to the evidence file it is supposed to update (see issue #549).", + ); + } + return join(dir, EVIDENCE_FILENAME); +} + +/** + * Append one recorded measurement so the change's evidence file is transcribed, + * not invented. Merges into any existing keys; throws (never creates) when the + * change directory is gone. + */ +export function recordMeasurement( + changeName: string, + key: string, + value: unknown, + repoRoot: string = REPO_ROOT, +): void { + const path = resolveEvidencePath(changeName, repoRoot); + const current = readEvidence(path); + // NOT `current[key] = value`: for key "__proto__" that mutates the prototype + // instead of adding a property, and the measurement never reaches the JSON. + Object.defineProperty(current, key, { value, enumerable: true, configurable: true, writable: true }); + writeFileSync(path, `${JSON.stringify(current, null, 2)}\n`); +} + +/** + * Existing measurements at `path`, or `{}` when the file does not exist yet. + * + * Every OTHER failure throws. A blanket `catch` here would turn an unreadable + * or half-written evidence file into a silent truncation: the next write would + * persist only the newest key and drop every measurement recorded before it. + * Losing recorded evidence quietly is the failure mode this module exists to + * prevent, so a damaged file must stop the run instead of being overwritten. + */ +export function readEvidence(path: string): Record { + let raw: string; + try { + raw = readFileSync(path, "utf8"); + } catch (err) { + if ((err as NodeJS.ErrnoException).code === "ENOENT") return {}; + throw err; + } + + let parsed: unknown; + try { + parsed = JSON.parse(raw); + } catch (err) { + throw new Error(`Refusing to overwrite unparseable evidence at ${path}: ${(err as Error).message}`); + } + + if (typeof parsed !== "object" || parsed === null || Array.isArray(parsed)) { + throw new Error(`Refusing to overwrite evidence at ${path}: expected a JSON object, got ${describe(parsed)}.`); + } + return parsed as Record; +} + +function describe(value: unknown): string { + if (value === null) return "null"; + return Array.isArray(value) ? "an array" : typeof value; +} diff --git a/tests/e2e/subagent-pull-measurements.spec.ts b/tests/e2e/subagent-pull-measurements.spec.ts index 107e7a87a..b5511f046 100644 --- a/tests/e2e/subagent-pull-measurements.spec.ts +++ b/tests/e2e/subagent-pull-measurements.spec.ts @@ -1,7 +1,6 @@ -import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname } from "node:path"; import type { BrowserContext, Locator, Page } from "@playwright/test"; import { expect, test } from "./fixtures.js"; +import { recordMeasurement } from "./helpers/evidence-path.js"; import { collectSubagentWire, type SubagentWireCollector, @@ -43,22 +42,15 @@ const WATCHED_BY_BUS: Record = { 250: "[[faux:subagent-watched-growth]] go", 1000: "[[faux:subagent-watched-growth-bus1000]] go", }; -const EVIDENCE_PATH = "openspec/changes/verify-subagent-pull-under-load/measurements.json"; +/** Resolved at write time — the change may be active or already archived. */ +const CHANGE_NAME = "verify-subagent-pull-under-load"; const STRIP_OFF = process.env.PI_DASHBOARD_SUBAGENT_STRIP === "0"; /** Fixture runtime: 240 ticks @ 50 ms. */ const RUNTIME_MS = 12_000; const WINDOW_MS = 6_000; function record(key: string, value: unknown): void { - mkdirSync(dirname(EVIDENCE_PATH), { recursive: true }); - let current: Record = {}; - try { - current = JSON.parse(readFileSync(EVIDENCE_PATH, "utf8")) as Record; - } catch { - /* first write */ - } - current[key] = value; - writeFileSync(EVIDENCE_PATH, `${JSON.stringify(current, null, 2)}\n`); + recordMeasurement(CHANGE_NAME, key, value); } async function startWatchedRun( diff --git a/tests/e2e/subagent-pull-under-load.spec.ts b/tests/e2e/subagent-pull-under-load.spec.ts index 1f3bbe540..b3b19bf1c 100644 --- a/tests/e2e/subagent-pull-under-load.spec.ts +++ b/tests/e2e/subagent-pull-under-load.spec.ts @@ -1,7 +1,6 @@ -import { mkdirSync, readFileSync, writeFileSync } from "node:fs"; -import { dirname } from "node:path"; import type { Locator, Page } from "@playwright/test"; import { expect, test } from "./fixtures.js"; +import { recordMeasurement } from "./helpers/evidence-path.js"; import { collectSubagentWire, type SubagentFrameSample, @@ -65,22 +64,15 @@ const END_ENTRIES = 30; /** Matches the fixture's entry text, so a DOM count needs no production hook. */ const ENTRY_TEXT = /^faux-entry \d+$/; const CADENCE_BASE_MS = 2_000; -const EVIDENCE_PATH = "openspec/changes/verify-subagent-pull-under-load/measurements.json"; +/** Resolved at write time — the change may be active or already archived. */ +const CHANGE_NAME = "verify-subagent-pull-under-load"; /** True on the harness start that disables the bridge strip (the push arm). */ const STRIP_OFF = process.env.PI_DASHBOARD_SUBAGENT_STRIP === "0"; /** Append one recorded measurement so heap-evidence.md is transcribed, not invented. */ function record(key: string, value: unknown): void { - mkdirSync(dirname(EVIDENCE_PATH), { recursive: true }); - let current: Record = {}; - try { - current = JSON.parse(readFileSync(EVIDENCE_PATH, "utf8")) as Record; - } catch { - /* first write */ - } - current[key] = value; - writeFileSync(EVIDENCE_PATH, `${JSON.stringify(current, null, 2)}\n`); + recordMeasurement(CHANGE_NAME, key, value); } /** Start a watched run and return its session id + wire collector. */ diff --git a/tests/vitest.config.ts b/tests/vitest.config.ts new file mode 100644 index 000000000..f0bcce580 --- /dev/null +++ b/tests/vitest.config.ts @@ -0,0 +1,22 @@ +import { defineConfig } from "vitest/config"; + +/** + * Vitest project for repo-root /tests. + * + * Deliberately narrow: it collects ONLY unit tests for the pure helpers under + * `e2e/helpers/__tests__/`. Playwright specs (`e2e/*.spec.ts`) must never be + * collected here — they need a running docker harness and a browser, and are + * driven by `npm run test:e2e`. + * + * Added because the evidence-path resolver is pure logic used by two specs + * that are BOTH opt-in (`PI_SYNTH_AGENT_TICKS=1`), so nothing in normal CI + * could execute it. See issue #549. + */ +export default defineConfig({ + test: { + include: ["e2e/helpers/__tests__/**/*.test.ts"], + environment: "node", + pool: "forks", + maxWorkers: "50%", + }, +}); diff --git a/vitest.config.ts b/vitest.config.ts index 5f2a5e543..595dd94be 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -52,6 +52,11 @@ export default defineConfig({ "packages/blackhole-plugin", "packages/mcp-server-plugin", "scripts", + // Pure helpers under tests/e2e/helpers/. NOT the Playwright specs — the + // project's include glob is scoped to `e2e/helpers/__tests__/`. Added + // because the evidence-path resolver is used only by opt-in specs, so + // normal CI could never execute it. See issue #549. + "tests", // ship-it's pure decision helpers. Added by wire-local-review-gate: they // gate real ship decisions but were collected by no project before. ".pi/skills/ship-it",