fix(pull): preserve lastPulledHash on classifier short-circuits (hotfix for #38)

The drift-direction-classifier PR (#38) introduced a regression: classifier
short-circuit branches in `pullResourceType` silently dropped `lastPulledHash`
from state, breaking drift detection for any resource that ever returned a
non-clean direction.

## Root cause

`pull.ts:769` initializes `newStateSection` as an empty object for a full pull:

  const newStateSection: Record<string, ResourceState> = resourceIds?.length
    ? { ...state[resourceType] }
    : {};   // ← empty for full pulls

The three classifier short-circuit branches then did one of:

- `both-diverged` (`:818`) \u2014 NO upsertState call at all. Entry vanishes from
  newStateSection. At end-of-loop, `state[type] = newStateSection` removes it
  from state entirely.
- `dashboard-ahead` (`:870`) \u2014 `upsertState(newStateSection, id, { uuid })`.
  Merge against undefined produces `{ uuid }`-only, dropping lastPulledHash
  and lastPulledAt.
- `local-ahead` (`:882`) \u2014 same shape as dashboard-ahead.

Symptom on the next pull: classifier sees no baseline \u2192 `no-baseline` \u2192
content-drift detection silently disabled for that resource until something
writes a fresh baseline. The `--resolve=ours` and `--resolve=theirs` paths
masked the bug locally (they explicitly write full state entries), but bare
plain pulls and `--resolve=fail` did not.

Caught by the E2E both-diverged smoke test on mudflap-iform-test:
- TEST 1: plain pull on both-diverged \u2192 exit 1 (gate fires) \u2192 state for that
  resource becomes `{ uuid }` only (lastPulledHash gone)
- TEST 2: --resolve=fail \u2192 exit 1 \u2192 same state loss

## Fix

Pass the existing baseline through at each short-circuit:

- both-diverged: assign the existing state entry verbatim into newStateSection
  before `continue`. `resolveBothDivergedResources` will overwrite it later
  if the operator passes --resolve.
- dashboard-ahead, local-ahead: include lastPulledHash + lastPulledAt in the
  upsertState patch so the merge against an empty section produces the full
  shape.

## Tests

4 new unit tests in tests/drift.test.ts \u00a7 Section J pin:
- Full patch survives empty newStateSection (the fix shape)
- Bare { uuid }-only patch DROPS lastPulledHash (the regression hazard \u2014
  fails if someone reverts the caller)
- both-diverged direct assignment preserves all fields verbatim
- upsertState merge semantics still hold (union, not replacement)

Empirically verified end-to-end against mudflap-iform-test:
- Create scenario \u2192 baseline hash X recorded \u2192 manufacture both-diverged \u2192
  plain pull \u2192 exit 1 \u2192 state still has lastPulledHash == X \u2705

## Why this wasn't caught in review

Two rounds of code-reviewer subagent passes on PR #38 verified contract
correctness of `classifyDrift`'s output (which direction maps to which return
value) but did NOT trace what each branch did to `newStateSection`. State
mutation per classifier branch should be an explicit review surface; will
add to future review briefs.

## Defer to follow-up

A full integration test using the pull-same-name-clobber.test.ts pattern
(spawn worker thread with HTTP stub, run pull-cmd.ts via spawnSync, assert
post-pull state) would lock this contract more rigorously. The unit tests
here pin the shape; the smoke + manual repro confirm the integration.

270/270 tests pass, tsc --noEmit clean.

Author: dhruva-vapi

src/pull.ts

@@ -852,6 +852,16 @@ export async function pullResourceType(
           });
           if (driftCounts) driftCounts[direction]++;
 
+          // STATE-PRESERVATION INVARIANT: classifier short-circuit branches
+          // MUST carry `lastPulledHash` (and `lastPulledAt`) forward into
+          // `newStateSection`. `newStateSection` starts EMPTY for a full pull
+          // (pull.ts:769); a bare `upsertState(..., { uuid })` merges against
+          // `undefined`, producing `{ uuid }`-only — dropping the baseline.
+          // Then `state[type] = newStateSection` at end-of-loop persists the
+          // loss. Symptom: next pull sees no baseline → `no-baseline`, the
+          // classifier can never detect drift on this resource again until
+          // something else writes a fresh baseline. The both-diverged branch
+          // skipped `upsertState` entirely, dropping the entry outright.
           if (direction === "both-diverged") {
             bothDiverged?.push({
               resourceType,
@@ -861,6 +871,14 @@ export async function pullResourceType(
               platformHash,
               lastPulledHash,
             });
+            // Preserve the existing entry verbatim. `resolveBothDivergedResources`
+            // will overwrite it with the resolve-mode-appropriate hash after the
+            // per-type loop completes; if the operator doesn't pass --resolve, we
+            // want the baseline still intact so the next pull can see drift.
+            const existing = state[resourceType][resourceId];
+            if (existing) {
+              newStateSection[resourceId] = existing;
+            }
             skipped++;
             continue;
           }
@@ -869,7 +887,11 @@ export async function pullResourceType(
             console.log(
               `   ✏️  ${resourceId} (locally modified, preserving) ${formatDriftLabel(direction)}`,
             );
-            upsertState(newStateSection, resourceId, { uuid: resource.id });
+            upsertState(newStateSection, resourceId, {
+              uuid: resource.id,
+              lastPulledHash,
+              lastPulledAt: stateEntry?.lastPulledAt,
+            });
             skipped++;
             continue;
           }
@@ -881,7 +903,11 @@ export async function pullResourceType(
             console.log(
               `   ⬆️  ${resourceId} (local ahead of dashboard) ${formatDriftLabel(direction)}`,
             );
-            upsertState(newStateSection, resourceId, { uuid: resource.id });
+            upsertState(newStateSection, resourceId, {
+              uuid: resource.id,
+              lastPulledHash,
+              lastPulledAt: stateEntry?.lastPulledAt,
+            });
             skipped++;
             continue;
           }

tests/drift.test.ts

@@ -3,7 +3,9 @@ import test from "node:test";
 import {
   checkPronunciationDictDrop,
   hashPayload,
+  upsertState,
 } from "../src/state-serialize.ts";
+import type { ResourceState } from "../src/types.ts";
 
 // Stack G — drift unit tests.
 // `checkDriftForUpdate` itself fires GET against the Vapi platform; a unit
@@ -659,3 +661,98 @@ test("canonicalizeForHash: strips server-managed fields (id, orgId, createdAt, u
   assert.equal(result.updatedAt, undefined);
   assert.equal(result.name, "Foo");
 });
+
+// ─────────────────────────────────────────────────────────────────────────────
+// Section J (added 2026-05-20): classifier short-circuit state 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.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.
+// ─────────────────────────────────────────────────────────────────────────────
+
+test("classifier short-circuit: full patch (uuid + lastPulledHash + lastPulledAt) survives empty newStateSection", () => {
+  const newStateSection: Record<string, ResourceState> = {};
+  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<string, ResourceState> = {};
+  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<string, ResourceState> = {
+    r1: {
+      uuid: "u1",
+      lastPulledHash: "h-baseline",
+      lastPulledAt: "2026-01-01T00:00:00.000Z",
+    },
+  };
+  const newStateSection: Record<string, ResourceState> = {};
+  const existing = existingState.r1;
+  if (existing) {
+    newStateSection.r1 = existing;
+  }
+  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<string, ResourceState> = {
+    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");
+});