From 87f55f5fc89efc28075b9abce90a60d3467b458c Mon Sep 17 00:00:00 2001 From: Scott Lowe Date: Wed, 30 Sep 2026 21:28:57 -0700 Subject: [PATCH] test: restore the suite after the hash-store migration and run it in CI npm test was not wired into any workflow, so 20 tests failed from the hash-store migration (#41) onward without anyone noticing. Every failure was a stale test, not an engine bug: fixtures still put lastPulledHash / lastPushedHash into state, checkDriftForUpdate calls lacked the new env argument, and one test pinned both-diverged for the converged edge that classifyDrift now deliberately treats as clean. - add .github/workflows/ci.yml: typecheck + npm test on every PR and on pushes to main, on Node 20 and 22 - move test baselines into the hash store: spawn-based tests seed /.vapi-state-hash, drift tests seed via writeBaseline under a throwaway org slug, and audit gains a baselineReader DI seam - push-stale-baseline-noop seeds a credential so push no longer runs a bootstrap pull that overwrote the stale baseline before the drift check, and asserts it; removing the agree-gate now fails this test - rewrite the classifier short-circuit regression around the hash store - tool-assistant-cycle deletes the baseline it wrote into the real store - improvements.md #32 Co-Authored-By: Claude Opus 5.5 --- .github/workflows/ci.yml | 40 +++++ improvements.md | 65 +++++++ src/audit.ts | 19 +- tests/audit.test.ts | 18 +- tests/drift.test.ts | 180 +++++++------------ tests/pull-rename-preserves-filename.test.ts | 8 +- tests/pull-same-name-clobber.test.ts | 8 +- tests/push-stale-baseline-noop.test.ts | 22 ++- tests/reconcile-state-key.test.ts | 16 +- tests/state-migration.test.ts | 47 +++-- tests/tool-assistant-cycle.test.ts | 8 +- 11 files changed, 271 insertions(+), 160 deletions(-) create mode 100644 .github/workflows/ci.yml diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml new file mode 100644 index 0000000..a92fd55 --- /dev/null +++ b/.github/workflows/ci.yml @@ -0,0 +1,40 @@ +name: CI + +on: + pull_request: + push: + branches: [main] + +permissions: + contents: read + +concurrency: + group: ci-${{ github.ref }} + cancel-in-progress: ${{ github.event_name == 'pull_request' }} + +jobs: + test: + name: Typecheck and test (Node ${{ matrix.node }}) + runs-on: ubuntu-latest + timeout-minutes: 15 + strategy: + fail-fast: false + matrix: + # Both majors package.json's engines field claims to support. + node: [20, 22] + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: ${{ matrix.node }} + cache: npm + + # The optional audio deps (mic, speaker) only matter for `npm run call`; + # npm tolerates their native build failing on a headless runner. + - run: npm ci + + - run: npm run build + + - run: npm test diff --git a/improvements.md b/improvements.md index 1e8fa53..5cc60ad 100644 --- a/improvements.md +++ b/improvements.md @@ -83,6 +83,7 @@ you which stack PR closes the row.** | 29 | SO linking sent filtered `assistantIds` arrays | Silent unlink of live-but-untracked assistants | None | RESOLVED 2026-08-03 (#51) | | 30 | Tool-linking pass could PATCH a raw assistant slug | Mid-push 400 naming the wrong resource | None | RESOLVED 2026-08-03 (#51) | | 31 | Unresolved references handled 3 inconsistent ways, no dangling-ref check | Same authoring mistake, three different failure modes | None | Open | +| 32 | Test suite never ran in CI; 20 tests rotted after the hash store | Regression guards for #22/#23 silently stopped running | None | RESOLVED 2026-09-30 (#56) | **Active backlog after cleanup:** `#2`, `#6`, `#8`, `#12`, `#20`, `#24–#26`, `#31`, and the open remainder of `#27` (wiring the listing-completeness verdict into push/delete/audit, and moving `cleanup.ts` onto the shared pager). Resolved entries stay in this file as historical incident notes per the maintenance directive; stale superseded backlog rows are not duplicated. @@ -1646,6 +1647,70 @@ three behaviors gets a chance to run. --- +## 32. The test suite never ran in CI, so 20 tests rotted unnoticed after the hash-store migration + +**[RESOLVED 2026-09-30] (#56)** + +### Problem + +`npm test` was not wired into any workflow — `.github/workflows/` held only +`promotion.yml` — so nothing stopped a merge that broke tests. The +hash-store migration (#41) moved drift baselines out of the state file into +`.vapi-state-hash//` and made the sync commands refuse legacy +state, and 20 tests failed from that merge onward without anyone noticing. + +### Current behavior (Verified, before the fix) + +- All 20 failures were stale tests, not engine bugs: fixtures still wrote + `lastPulledHash` / `lastPushedHash` into state (now stripped by + `asResourceState` / `upsertState`, or refused by the legacy-state gate), + the `checkDriftForUpdate` tests omitted the new required `env`, and one + test pinned `both-diverged` for the converged edge that + `src/drift.ts:50` now deliberately classifies as `clean`. +- Three of the dead tests were the regression guards #41 itself added for + #22 (dashboard rename keeps the local filename, same-name clobber) and #23 + (stale baseline must not block a push). They had never passed. +- `push-stale-baseline-noop` would not have exercised its case even with a + migrated fixture: empty `credentials` makes `maybeBootstrapState` + (`src/push.ts:509-525`) treat state as uninitialized, and the bootstrap pull + rewrites the stale baseline before the drift check runs. +- The hash store resolves beside `src/` (`src/hash-store.ts:26-31`), not + under a temp dir, so in-process tests that push write baselines into the + developer's real store — `tool-assistant-cycle.test.ts` left one under + `.vapi-state-hash/test-fixture-org/` on every run. + +### Risk + +Any engine regression could merge green. The rename, clobber and +phantom-drift fixes had no working coverage. + +### Current mitigation + +None needed once the fix below lands; CI now fails the PR. + +### Possible fix (landed) + +- `.github/workflows/ci.yml` runs `npm run build` and `npm test` on every PR + and on pushes to `main`, on Node 20 and 22 (the `engines` range). +- Fixtures moved to the hash store: spawn-based tests seed + `/.vapi-state-hash//` (the engine runs from the copied + `src/`), `drift.test.ts` seeds through `writeBaseline` under a throwaway + org slug and removes it, and `audit.ts` gained a `baselineReader` DI seam + (`src/audit.ts:86`) beside `stateLoader` / `listLocalIds`. +- `push-stale-baseline-noop` seeds a credential so no bootstrap pull runs, and + asserts that. Removing the agree-gate now fails it, as it does the + converged-edge `classifyDrift` test. +- `tool-assistant-cycle.test.ts` deletes the baseline it writes. + +### Status + +**RESOLVED 2026-09-30.** Tests are still outside `tsconfig.json`'s `include`, +so `npm run build` never typechecks them and the compiler could not have +caught these stale fixture shapes. Including them surfaces 37 existing type +errors today; widening `include` is a separate change. + +--- + ## Out of scope (intentionally not improvements) - **State file is identity-only and not git-ignored.** It's intentionally diff --git a/src/audit.ts b/src/audit.ts index cbe3f98..9ca5a73 100644 --- a/src/audit.ts +++ b/src/audit.ts @@ -79,6 +79,11 @@ export interface AuditOptions { // frontmatter parsing of the assistant file on disk. Sync OR async return // is accepted so tests can keep their fixtures plain-object. readAssistantTools?: (resourceId: string) => unknown[] | Promise; + // DI seam: swap the drift-baseline lookup. Defaults to the per-developer + // hash store (.vapi-state-hash//), which resolves beside src/ + // rather than under any temp dir — so in-process tests must inject this + // instead of seeding baseline files. + baselineReader?: (uuid: string) => string | undefined; } // ───────────────────────────────────────────────────────────────────────────── @@ -243,10 +248,11 @@ function checkStateUuidCollisions( function checkContentIdentical( type: ResourceType, state: StateFile, + baselineReader: (uuid: string) => string | undefined, ): { findings: AuditFinding[]; identicalSlugs: Set } { const byHash = new Map(); for (const [resourceId, entry] of Object.entries(state[type])) { - const hash = readBaseline(VAPI_ENV, entry.uuid); + const hash = baselineReader(entry.uuid); if (!hash) continue; const slugs = byHash.get(hash) ?? []; slugs.push(resourceId); @@ -368,6 +374,7 @@ function checkContentDrift( state: StateFile, remote: VapiResource[], localIds: string[], + baselineReader: (uuid: string) => string | undefined, ): AuditFinding[] { const remoteByUuid = new Map(remote.map((r) => [r.id, r])); const credReverse = credentialReverseMap(state); @@ -390,7 +397,7 @@ function checkContentDrift( ); const direction = classifyDrift({ localHash, - lastPulledHash: readBaseline(VAPI_ENV, entry.uuid), + lastPulledHash: baselineReader(entry.uuid), platformHash, }); if (direction === "clean") continue; @@ -466,6 +473,8 @@ export async function runAudit( const remoteFetcher = opts.remoteFetcher ?? fetchAllResources; const readAssistantTools = opts.readAssistantTools ?? defaultReadAssistantTools; + const baselineReader = + opts.baselineReader ?? ((uuid: string) => readBaseline(VAPI_ENV, uuid)); const state = stateLoader(); @@ -519,13 +528,15 @@ export async function runAudit( const remoteUuids = new Set(remote.map((r) => r.id)); findings.push(...checkStateGhosts(type, state, remoteUuids)); findings.push(...checkDashboardOrphans(type, state, remote)); - findings.push(...checkContentDrift(type, state, remote, localIds)); + findings.push( + ...checkContentDrift(type, state, remote, localIds, baselineReader), + ); } findings.push(...checkStateUuidCollisions(type, state)); const { findings: identicalFindings, identicalSlugs } = - checkContentIdentical(type, state); + checkContentIdentical(type, state, baselineReader); findings.push(...identicalFindings); findings.push(...checkSiblingBaseSlug(type, state, identicalSlugs)); diff --git a/tests/audit.test.ts b/tests/audit.test.ts index d8c1f00..d348bd2 100644 --- a/tests/audit.test.ts +++ b/tests/audit.test.ts @@ -24,8 +24,20 @@ import type { ResourceState, ResourceType, StateFile } from "../src/types.ts"; // Helpers — keep fixtures DI-friendly and avoid filesystem / network. // ───────────────────────────────────────────────────────────────────────────── +// Drift baselines live in the per-developer hash store, not the state file, so +// fixtures record them here and `baseOpts` serves them via `baselineReader`. +// A hash-less entry clears its uuid so a baseline can't leak between tests +// that reuse the same fixture uuid. +const baselines = new Map(); + function makeStateEntry(uuid: string, hash?: string): ResourceState { - return hash ? { uuid, lastPulledHash: hash } : { uuid }; + if (hash) baselines.set(uuid, hash); + else baselines.delete(uuid); + return { uuid }; +} + +function readFixtureBaseline(uuid: string): string | undefined { + return baselines.get(uuid); } // All sections start empty so callers only populate the type(s) under test. @@ -54,6 +66,7 @@ function baseOpts(state: StateFile) { stateLoader: () => state, listLocalIds: (_t: ResourceType) => [] as string[], readAssistantTools: (_id: string) => [] as unknown[], + baselineReader: readFixtureBaseline, }; } @@ -440,6 +453,7 @@ test("inline-tools: assistant with empty model.tools array → 0 findings", asyn const findings = await runAudit({ ...baseOpts(state), readAssistantTools: () => [], + baselineReader: readFixtureBaseline, }); const inline = findings.filter((f) => f.rule === "inline-tools"); assert.equal(inline.length, 0); @@ -455,6 +469,7 @@ test("inline-tools: readAssistantTools returns non-array (treated as no inline t const findings = await runAudit({ ...baseOpts(state), readAssistantTools: () => [], + baselineReader: readFixtureBaseline, }); const inline = findings.filter((f) => f.rule === "inline-tools"); assert.equal(inline.length, 0); @@ -505,6 +520,7 @@ test("integration: orphan-yaml + collision + content-identical(4) + sibling-base // 1 orphan-yaml: a local file with no state entry. listLocalIds: (t) => (t === "assistants" ? ["stray-local"] : []), readAssistantTools: () => [], + baselineReader: readFixtureBaseline, }); // Total: 1 orphan-yaml + 1 collision + 2 content-identical + 1 sibling diff --git a/tests/drift.test.ts b/tests/drift.test.ts index 4c6e48f..73fbf61 100644 --- a/tests/drift.test.ts +++ b/tests/drift.test.ts @@ -6,6 +6,14 @@ import { upsertState, } from "../src/state-serialize.ts"; import type { ResourceState } from "../src/types.ts"; +import { + deleteBaseline, + hashStoreDir, + readBaseline, + writeBaseline, +} from "../src/hash-store.ts"; +import { rmSync } from "node:fs"; +import { after } from "node:test"; // Stack G — drift unit tests. // `checkDriftForUpdate` itself fires GET against the Vapi platform; a unit @@ -20,8 +28,8 @@ import type { ResourceState } from "../src/types.ts"; // // In-scope for this test file: // 1. `classifyDrift` truth table — every cell of the 3-hash decision matrix, -// including the both-diverged edge where local == platform but both -// diverge from the baseline. +// including the converged edge where local == platform but both diverge +// from the baseline (pinned as clean: live agreement is never drift). // 2. `formatDriftLabel` — non-empty + operator-actionable phrasing per // direction. The exact wording is the implementer's choice; the contract // is that the operator can read the label and know which command to run. @@ -67,10 +75,8 @@ const { classifyDrift, formatDriftLabel, checkDriftForUpdate } = resourceLabel: string; resourceType: string; resourceId: string; - state: Record< - string, - Record - >; + state: Record>; + env: string; overwrite: boolean; }) => Promise<{ ok: boolean; @@ -84,10 +90,8 @@ const { classifyDrift, formatDriftLabel, checkDriftForUpdate } = // every resource-type section via buildReverseMap and `credentialReverseMap` // reads `.credentials`, so a partial object throws. `inject` seeds one section. function driftState( - inject: Partial< - Record> - > = {}, -): Record> { + inject: Partial>> = {}, +): Record> { return { tools: {}, structuredOutputs: {}, @@ -100,12 +104,17 @@ function driftState( evals: {}, credentials: {}, ...inject, - } as Record< - string, - Record - >; + } as Record>; } +// Drift baselines live in the hash store (.vapi-state-hash//), which +// resolves beside src/ rather than under a temp dir. Seed them under an org slug +// no real checkout uses, and remove the whole folder when the file finishes. +const HASH_STORE_TEST_ENV = `drift-test-${process.pid}`; +after(() => { + rmSync(hashStoreDir(HASH_STORE_TEST_ENV), { recursive: true, force: true }); +}); + test("checkPronunciationDictDrop: warns when prior had ID and new lost it", () => { const prior = { voice: { provider: "cartesia", pronunciationDictId: "pdict_X" }, @@ -312,19 +321,20 @@ test("classifyDrift: both-diverged — local, platform, lastPulled all differ", assert.equal(direction, "both-diverged"); }); -test("classifyDrift: both-diverged edge — local == platform but both diverged from baseline", () => { - // Real edge: two independent edits happen to converge on the same content - // (e.g. both sides corrected the same typo). The classifier must still - // report both-diverged — the baseline is what the operator pulled, and - // BOTH sides have moved past it. Treating this as `clean` would lose the - // signal that the operator's state pointer is stale. +test("classifyDrift: converged edge — local == platform but both diverged from baseline is clean", () => { + // Two independent edits converged on the same content (e.g. both sides + // fixed the same typo), or a push made the dashboard match local while the + // baseline still held the previous pull's hash. Live agreement is never + // drift: callers treat `clean` as "refresh the baseline", which self-heals + // the stale pointer. Reporting both-diverged here manufactured a phantom + // conflict on every untouched resource after a hash-basis change. assert.ok(classifyDrift, "classifyDrift export missing from src/drift.ts"); const direction = classifyDrift!({ localHash: H_CONVERGED, lastPulledHash: H_BASE, platformHash: H_CONVERGED, }); - assert.equal(direction, "both-diverged"); + assert.equal(direction, "clean"); }); test("classifyDrift: no-baseline — lastPulledHash is undefined", () => { @@ -506,7 +516,7 @@ test("checkDriftForUpdate: drift-blocked message includes a bracketed direction // dashboard-ahead (local clean, platform moved). const remote = { name: "intake-bot", systemPrompt: "hello" }; // A baseline that cannot equal the canonicalized platform hash → drift. - const lastPulledHash = "stale-baseline-hash-deadbeef"; + await writeBaseline(HASH_STORE_TEST_ENV, "test-uuid", "stale-baseline-hash-deadbeef"); const originalFetch = globalThis.fetch; globalThis.fetch = makeFetchStub(remote) as typeof globalThis.fetch; @@ -517,8 +527,9 @@ test("checkDriftForUpdate: drift-blocked message includes a bracketed direction resourceType: "assistants", resourceId: "intake-bot", state: driftState({ - assistants: { "intake-bot": { uuid: "test-uuid", lastPulledHash } }, + assistants: { "intake-bot": { uuid: "test-uuid" } }, }), + env: HASH_STORE_TEST_ENV, overwrite: false, }); assert.equal(result.ok, false, "drift should be blocked"); @@ -531,6 +542,7 @@ test("checkDriftForUpdate: drift-blocked message includes a bracketed direction ); } finally { globalThis.fetch = originalFetch; + await deleteBaseline(HASH_STORE_TEST_ENV, "test-uuid"); } }); @@ -569,7 +581,7 @@ test("checkDriftForUpdate: canonicalizes the platform payload (tool UUID → res name: "intake-bot", model: { provider: "openai", toolIds: ["my-tool-abc12345"] }, }; - const baseline = hashPayload(canonicalForm); + await writeBaseline(HASH_STORE_TEST_ENV, "assistant-uuid", hashPayload(canonicalForm)); const originalFetch = globalThis.fetch; globalThis.fetch = makeFetchStub(remoteRaw) as typeof globalThis.fetch; @@ -581,10 +593,11 @@ test("checkDriftForUpdate: canonicalizes the platform payload (tool UUID → res resourceId: "intake-bot", state: driftState({ assistants: { - "intake-bot": { uuid: "assistant-uuid", lastPulledHash: baseline }, + "intake-bot": { uuid: "assistant-uuid" }, }, tools: { "my-tool-abc12345": { uuid: "tool-uuid-xyz" } }, }), + env: HASH_STORE_TEST_ENV, overwrite: false, }); assert.equal( @@ -595,12 +608,13 @@ test("checkDriftForUpdate: canonicalizes the platform payload (tool UUID → res assert.equal(result.ok, true); } finally { globalThis.fetch = originalFetch; + await deleteBaseline(HASH_STORE_TEST_ENV, "assistant-uuid"); } }); test("checkDriftForUpdate: no-baseline path returns early without fetching", async () => { - // Regression guard — when there's no lastPulledHash, the function returns - // early without fetching. + // Regression guard — when the hash store holds no baseline for the + // resource, the function returns early without fetching. assert.ok( checkDriftForUpdate, "checkDriftForUpdate export missing from src/drift.ts", @@ -619,8 +633,9 @@ test("checkDriftForUpdate: no-baseline path returns early without fetching", asy resourceType: "assistants", resourceId: "new-bot", state: driftState({ - assistants: { "new-bot": { uuid: "test-uuid" } }, // no lastPulledHash + assistants: { "new-bot": { uuid: "never-seeded-uuid" } }, }), + env: HASH_STORE_TEST_ENV, overwrite: false, }); assert.equal(result.ok, true); @@ -767,96 +782,35 @@ test("canonicalizeForHash: strips server-managed fields (id, orgId, createdAt, u }); // ───────────────────────────────────────────────────────────────────────────── -// Section J (added 2026-05-20): classifier short-circuit state preservation. +// Section J (added 2026-05-20, reworked for the hash store): classifier +// short-circuit baseline preservation. // // Regression coverage for a bug introduced by the drift-direction-classifier // PR (#38) and caught by the E2E both-diverged smoke test on mudflap-iform-test: +// pull rebuilds each state section from EMPTY, and the classifier short-circuit +// branches wrote back a bare `{ uuid }` — dropping the baseline, so the next +// pull classified the resource as `no-baseline` and could never detect drift +// on it again. // -// pull.ts `newStateSection` starts EMPTY for a full pull (line 769). The -// classifier short-circuit branches (`dashboard-ahead`, `local-ahead`, -// `both-diverged`) previously called `upsertState(newStateSection, id, { uuid })` -// against this empty section — dropping `lastPulledHash` and `lastPulledAt` -// from state. The `both-diverged` branch was worse: it called no upsert at all, -// so the entry vanished entirely. -// -// After the per-type loop, `state[type] = newStateSection` (line 1040) -// persists the loss. The operator's NEXT pull sees no baseline → `no-baseline` -// classification → the classifier can never detect drift on this resource -// again until something writes a fresh baseline. -// -// These tests pin the upsertState patch SHAPE that the fix uses, plus the -// bare-{ uuid }-only failure mode so a future contributor can't silently -// revert without breaking a test. +// Baselines now live in the hash store, keyed by uuid, so rebuilding a state +// section cannot touch them. This pins that separation: if a future change +// moves the baseline back into the state entry, or makes upsertState clear the +// store, the short-circuit regression returns and this test fails. // ───────────────────────────────────────────────────────────────────────────── -test("classifier short-circuit: full patch (uuid + lastPulledHash + lastPulledAt) survives empty newStateSection", () => { - const newStateSection: Record = {}; - upsertState(newStateSection, "r1", { - uuid: "u1", - lastPulledHash: "h-baseline", - lastPulledAt: "2026-01-01T00:00:00.000Z", - }); - assert.equal(newStateSection.r1?.uuid, "u1"); - assert.equal( - newStateSection.r1?.lastPulledHash, - "h-baseline", - "dashboard-ahead / local-ahead branches MUST pass lastPulledHash through; otherwise next pull sees no-baseline", - ); - assert.equal(newStateSection.r1?.lastPulledAt, "2026-01-01T00:00:00.000Z"); -}); - -test("classifier short-circuit: bare { uuid }-only patch DROPS lastPulledHash on empty section (regression hazard)", () => { - // Pins the failure mode — if a future contributor reverts to passing only - // { uuid: resource.id } to upsertState (the shape the original PR shipped - // with), this test catches it. The fix is in the CALLER (pull.ts classifier - // branches); upsertState's merge semantics are correct as-is. - const newStateSection: Record = {}; - upsertState(newStateSection, "r1", { uuid: "u1" }); - assert.equal(newStateSection.r1?.uuid, "u1"); - assert.equal( - newStateSection.r1?.lastPulledHash, - undefined, - "bare patch must NOT magically materialize lastPulledHash — fix lives in the caller's patch", - ); -}); - -test("classifier both-diverged: direct assignment from existing state preserves all fields verbatim", () => { - // both-diverged path does NOT call upsertState (resolveBothDivergedResources - // takes over post-loop, with per-resolve-mode state mutation). Without the - // preservation assignment in the branch, the entry vanishes from - // newStateSection. With it, the operator can re-run pull with --resolve and - // still have the baseline to compare against. - const existingState: Record = { - r1: { - uuid: "u1", - lastPulledHash: "h-baseline", - lastPulledAt: "2026-01-01T00:00:00.000Z", - }, - }; - const newStateSection: Record = {}; - const existing = existingState.r1; - if (existing) { - newStateSection.r1 = existing; +test("classifier short-circuit: rebuilding a state section from empty keeps the resource's baseline", async () => { + const uuid = "short-circuit-uuid"; + await writeBaseline(HASH_STORE_TEST_ENV, uuid, "h-baseline"); + try { + const newStateSection: Record = {}; + upsertState(newStateSection, "r1", { uuid }); + assert.deepEqual(newStateSection.r1, { uuid }); + assert.equal( + readBaseline(HASH_STORE_TEST_ENV, uuid), + "h-baseline", + "a bare { uuid } state write must not drop the baseline; otherwise the next pull sees no-baseline", + ); + } finally { + await deleteBaseline(HASH_STORE_TEST_ENV, uuid); } - assert.deepEqual( - newStateSection.r1, - existing, - "both-diverged branch MUST preserve the existing entry verbatim; saveState writes newStateSection over state[type] at end of loop", - ); -}); - -test("upsertState merge: pre-existing entry + new patch produces union, NOT replacement", () => { - // Sanity-check that upsertState's documented merge semantic still holds. - // If a future refactor switches to replacement semantics, the classifier - // short-circuits would lose lastPulledHash on subsequent calls. - const section: Record = { - r1: { - uuid: "u1", - lastPulledHash: "h-old", - lastPulledAt: "2026-01-01T00:00:00.000Z", - }, - }; - upsertState(section, "r1", { uuid: "u1", lastPulledAt: "2026-02-01T00:00:00.000Z" }); - assert.equal(section.r1?.lastPulledHash, "h-old", "upsertState must preserve fields not in the patch"); - assert.equal(section.r1?.lastPulledAt, "2026-02-01T00:00:00.000Z", "upsertState must overwrite fields in the patch"); }); diff --git a/tests/pull-rename-preserves-filename.test.ts b/tests/pull-rename-preserves-filename.test.ts index 460d487..7405380 100644 --- a/tests/pull-rename-preserves-filename.test.ts +++ b/tests/pull-rename-preserves-filename.test.ts @@ -134,13 +134,19 @@ test("pull: dashboard rename of tracked resource preserves the local filename (n mkdirSync(assistantsDir, { recursive: true }); writeFileSync(join(assistantsDir, `${TRACKED_SLUG}.md`), PREEXISTING_MD); + // The drift baseline lives in the hash store, not the state file. The + // engine runs from the copied src/, so its store resolves under `dir`. + const hashStore = join(dir, ".vapi-state-hash", ENV); + mkdirSync(hashStore, { recursive: true }); + writeFileSync(join(hashStore, UUID_X), "stale-hash-X\n"); + writeFileSync( join(dir, `.vapi-state.${ENV}.json`), JSON.stringify( { credentials: {}, assistants: { - [TRACKED_SLUG]: { uuid: UUID_X, lastPulledHash: "stale-hash-X" }, + [TRACKED_SLUG]: { uuid: UUID_X }, }, structuredOutputs: {}, tools: {}, diff --git a/tests/pull-same-name-clobber.test.ts b/tests/pull-same-name-clobber.test.ts index 2be3e17..de1f852 100644 --- a/tests/pull-same-name-clobber.test.ts +++ b/tests/pull-same-name-clobber.test.ts @@ -152,13 +152,19 @@ async function runClobberScenario( writeFileSync(join(assistantsDir, "riley.md"), PREEXISTING_RILEY_MD); // Seed state: slug `riley` already maps to UUID A. + // The drift baseline lives in the hash store, not the state file. The + // engine runs from the copied src/, so its store resolves under `dir`. + const hashStore = join(dir, ".vapi-state-hash", ENV); + mkdirSync(hashStore, { recursive: true }); + writeFileSync(join(hashStore, UUID_A), "stale-hash-A\n"); + writeFileSync( join(dir, `.vapi-state.${ENV}.json`), JSON.stringify( { credentials: {}, assistants: { - riley: { uuid: UUID_A, lastPulledHash: "stale-hash-A" }, + riley: { uuid: UUID_A }, }, structuredOutputs: {}, tools: {}, diff --git a/tests/push-stale-baseline-noop.test.ts b/tests/push-stale-baseline-noop.test.ts index 5ee1953..882ce3d 100644 --- a/tests/push-stale-baseline-noop.test.ts +++ b/tests/push-stale-baseline-noop.test.ts @@ -125,15 +125,24 @@ test("push: stale lastPulledHash does not block when local and dashboard agree", mkdirSync(assistantsDir, { recursive: true }); writeFileSync(join(assistantsDir, `${SLUG}.md`), LOCAL_MD); + // The drift baseline lives in the hash store, not the state file. The + // engine runs from the copied src/, so its store resolves under `dir`. + // It is deliberately stale: it equals neither the local file's hash nor + // the canonicalized dashboard hash. + const hashStore = join(dir, ".vapi-state-hash", ENV); + mkdirSync(hashStore, { recursive: true }); + writeFileSync(join(hashStore, UUID), "stale-older-basis-hash\n"); + writeFileSync( join(dir, `.vapi-state.${ENV}.json`), JSON.stringify( { - credentials: {}, - // The baseline is deliberately stale: it equals neither the local - // file's hash nor the canonicalized dashboard hash. + // A non-empty credentials section keeps push from treating state as + // uninitialized; otherwise its bootstrap pull rewrites the baseline + // before the drift check and the stale case is never exercised. + credentials: { "unused-credential": { uuid: "cred-uuid-unused" } }, assistants: { - [SLUG]: { uuid: UUID, lastPulledHash: "stale-older-basis-hash" }, + [SLUG]: { uuid: UUID }, }, structuredOutputs: {}, tools: {}, @@ -191,6 +200,11 @@ test("push: stale lastPulledHash does not block when local and dashboard agree", 0, `push must exit 0 (no phantom block)\n${out}`, ); + assert.doesNotMatch( + out, + /Bootstrap state sync required/, + `push must reach the drift check with the stale baseline intact, not re-pull first\n${out}`, + ); assert.doesNotMatch( out, /drift detected|both-diverged/, diff --git a/tests/reconcile-state-key.test.ts b/tests/reconcile-state-key.test.ts index a4b4b0e..aefca7a 100644 --- a/tests/reconcile-state-key.test.ts +++ b/tests/reconcile-state-key.test.ts @@ -169,10 +169,10 @@ for (const rtype of RTYPES) { harness: h, }); - // Canonical key now points at the adopted UUID (with lastPushedHash - // populated after applyFn success). - assert.equal(h.state[rtype]["my-resource"]?.uuid, sharedUuid); - assert.ok(h.state[rtype]["my-resource"]?.lastPushedHash); + // Canonical key now points at the adopted UUID. The entry is a bare + // { uuid }: the drift baseline is recorded by applyFn's own push path in + // the hash store, never in state. + assert.deepEqual(h.state[rtype]["my-resource"], { uuid: sharedUuid }); // Alias was deleted. assert.equal(h.state[rtype]["my-resource-12345678"], undefined); // Both keys marked touched: the deletion AND the new canonical entry. @@ -304,11 +304,9 @@ for (const rtype of RTYPES) { harness: h, }); - assert.equal( - h.state[rtype]["brand-new"]?.uuid, - "33333333-3333-3333-3333-333333333aaa", - ); - assert.ok(h.state[rtype]["brand-new"]?.lastPushedHash); + assert.deepEqual(h.state[rtype]["brand-new"], { + uuid: "33333333-3333-3333-3333-333333333aaa", + }); assert.equal(h.applied[rtype], 1); assert.equal(h.autoAppliedList.length, 1); assert.ok(h.autoApplied.has(`${rtype}:brand-new`)); diff --git a/tests/state-migration.test.ts b/tests/state-migration.test.ts index f5f0ac3..069bcef 100644 --- a/tests/state-migration.test.ts +++ b/tests/state-migration.test.ts @@ -10,24 +10,30 @@ import type { ResourceState } from "../src/types.ts"; // Stack F — state schema migration coverage. // -// The architectural pivot wraps each state value as a ResourceState. Legacy -// state files (Record) must keep loading cleanly so the -// rollout is a no-op for customers until their first pull populates the -// hash fields. These specs pin the behavior of the public helpers without -// importing the full state.ts module (which loads config.ts and exits). +// Each state value is a ResourceState — a pure `{ uuid }`. Drift baselines +// moved to the per-developer hash store (.vapi-state-hash/), so the helpers +// must strip any legacy hash/timestamp field rather than carry it forward: +// saveState must never re-emit one. These specs pin the behavior of the public +// helpers without importing the full state.ts module (which loads config.ts +// and exits). test("asResourceState: wraps a bare string UUID as { uuid }", () => { const result = asResourceState("uuid-abc-123"); assert.deepEqual(result, { uuid: "uuid-abc-123" }); }); -test("asResourceState: passes through a ResourceState object", () => { - const input: ResourceState = { +test("asResourceState: keeps only the uuid of an object entry", () => { + assert.deepEqual(asResourceState({ uuid: "u" }), { uuid: "u" }); +}); + +test("asResourceState: strips legacy hash/timestamp fields", () => { + const legacy = { uuid: "u", lastPulledHash: "h", lastPulledAt: "2026-04-30T12:00:00Z", + lastPushedHash: "p", }; - assert.equal(asResourceState(input), input); + assert.deepEqual(asResourceState(legacy), { uuid: "u" }); }); test("asResourceState: rejects non-string-non-object values", () => { @@ -44,24 +50,13 @@ test("upsertState: creates a new entry when none exists", () => { assert.deepEqual(section["agent-a"], { uuid: "u1" }); }); -test("upsertState: preserves prior fields not being patched", () => { - const section: Record = { - "agent-a": { - uuid: "u1", - lastPulledHash: "old-hash", - lastPulledAt: "2026-04-29T00:00:00Z", - }, - }; - upsertState(section, "agent-a", { - uuid: "u1", - lastPushedHash: "new-push-hash", - }); - assert.deepEqual(section["agent-a"], { - uuid: "u1", - lastPulledHash: "old-hash", - lastPulledAt: "2026-04-29T00:00:00Z", - lastPushedHash: "new-push-hash", - }); +test("upsertState: writes only the uuid, even when the patch carries legacy fields", () => { + // A caller still holding an old-shaped object must not smuggle a hash back + // into the state file; baselines belong to the hash store. + const section: Record = {}; + const legacyPatch = { uuid: "u1", lastPushedHash: "new-push-hash" }; + upsertState(section, "agent-a", legacyPatch); + assert.deepEqual(section["agent-a"], { uuid: "u1" }); }); test("upsertState: overwrites uuid if it changes", () => { diff --git a/tests/tool-assistant-cycle.test.ts b/tests/tool-assistant-cycle.test.ts index c4fe8c2..c0c5ee4 100644 --- a/tests/tool-assistant-cycle.test.ts +++ b/tests/tool-assistant-cycle.test.ts @@ -1,5 +1,5 @@ import assert from "node:assert/strict"; -import test from "node:test"; +import test, { after } from "node:test"; // ───────────────────────────────────────────────────────────────────────────── // Tools are applied before assistants because assistants reference tools. But a @@ -26,10 +26,16 @@ const { updateToolAssistantRefs, } = await import("../src/push.ts"); +import { deleteBaseline } from "../src/hash-store.ts"; import type { ResourceFile, StateFile } from "../src/types.ts"; const UUID = "8f14e45f-ceea-467a-9f1b-1a1b2c3d4e5f"; +// updateToolAssistantRefs records a drift baseline for every PATCH it sends, and +// the hash store resolves beside src/ — not under a temp dir — so remove the +// one these tests write rather than leave it in the developer's real store. +after(() => deleteBaseline("test-fixture-org", UUID)); + test("an unresolved assistant destination drops the whole destinations key", async () => { // Unresolved = resolution left the slug exactly as written, because the // assistant is not in state yet.