refactor: scoped state writes preserve untouched entries - #22
Merged
Merged
Conversation
This was referenced May 1, 2026
Contributor
Author
dhruva-vapi
force-pushed
the
dhruva-reddy/feat/snapshot-rollback
branch
from
May 1, 2026 22:56
fdf7bfb to
c916a21
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/refactor/scoped-state-writes
branch
from
May 1, 2026 22:56
6ebaebb to
90d7cd0
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/feat/snapshot-rollback
branch
from
May 2, 2026 01:24
c916a21 to
34ae2cb
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/refactor/scoped-state-writes
branch
from
May 2, 2026 01:24
90d7cd0 to
1b79bc3
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/feat/snapshot-rollback
branch
from
May 2, 2026 01:29
34ae2cb to
9667011
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/refactor/scoped-state-writes
branch
from
May 2, 2026 01:29
1b79bc3 to
0aa2c59
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/feat/snapshot-rollback
branch
from
May 2, 2026 01:33
9667011 to
866b910
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/refactor/scoped-state-writes
branch
from
May 2, 2026 01:34
0aa2c59 to
259746d
Compare
Contributor
Author
Merge activity
|
dhruva-vapi
force-pushed
the
dhruva-reddy/refactor/scoped-state-writes
branch
from
May 5, 2026 02:14
259746d to
9d8933a
Compare
dhruva-vapi
force-pushed
the
dhruva-reddy/feat/snapshot-rollback
branch
from
May 5, 2026 02:14
866b910 to
83242f3
Compare
dhruva-vapi
changed the base branch from
dhruva-reddy/feat/snapshot-rollback
to
graphite-base/22
May 5, 2026 02:20
## ELI5 **Problem.** Even when you ran a *scoped* push — say `npm run push -- <env> assistants/foo.md` to update one assistant — the engine rewrote the **entire** state file. Any pre-existing drift in unrelated state entries (UUIDs from earlier sessions, untracked local files, etc.) swept into the focused commit. Reviewers couldn't tell from the state-file diff "what did this push actually change?" and the state file became a pile of side effects accumulated across sessions instead of a precise record of intent. **What this fix does.** During a push, the engine tracks which `resourceId`s it actually mutated (a per-section `Set<string>`). At end-of-run, for **scoped pushes only**, it loads the on-disk state fresh, replaces only the touched entries with the in-memory version, and leaves everything else alone. Full pushes (no scope) still write wholesale (existing behavior). Credentials are always replaced because bootstrap pull populates them every push regardless. This depends on Stack F's `ResourceState` because we need per-entry metadata to distinguish "stale" from "just-not-touched." **Outcome you'll notice.** A one-file `npm run push` produces a one-file diff in the state file — same scope as the resource change. Reviewers can read the state diff and tell "this push updated assistant `foo`, here's its new hash" cleanly. Pre-existing drift elsewhere in state stays where it is until you explicitly address it. --- When push is scoped to specific paths, only update state entries for the resources actually touched. A surgical push of two files used to rewrite the entire state file, sweeping in pre-existing drift from earlier pushes (improvements.md #15) and producing noisy diffs that hide the actual scope of the change. Files: - src/state-merge.ts (NEW): mergeScoped(disk, inMemory, touched). For each section, replace only touched.X resourceIds with the in-memory version; leave the rest of disk's section as-is. Credentials are always replaced wholesale (bootstrap pull populates them on every push). Pure data, no I/O — safe to test directly. - src/push.ts: TouchedSets tracker. Each upsertState call site records the resourceId. End-of-run, partial pushes call mergeScoped(loadState(), state, touched) before saveState; full pushes save wholesale (existing behavior). - tests/state-merge.test.ts: replace-only-touched, leave-untouched, drift in untouched stays, credentials always replaced. Closes improvements.md #15. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
dhruva-vapi
force-pushed
the
dhruva-reddy/refactor/scoped-state-writes
branch
from
May 5, 2026 02:22
9d8933a to
9696d10
Compare
scott-lowe-vapi
added a commit
that referenced
this pull request
Oct 1, 2026
…CI (#56) ## Problem `npm test` isn't wired into any workflow (`.github/workflows/` only held `promotion.yml`), so nothing blocks a merge that breaks tests. Since the hash-store migration in #41, **20 of 357 tests have failed on `main`**, unnoticed. Bisecting first-parent `main`: 0 failures at #42, 20 failures from #41's merge onward, and no new failures since then (#55 included). ## Diagnosis: stale tests, not engine bugs #41 moved drift baselines out of the state file into `.vapi-state-hash/<org>/<uuid>`, made `upsertState` / `asResourceState` strip legacy fields, and made pull/push/apply refuse legacy-shaped state. The tests were never moved over. | Tests | Failures | Cause | |---|---|---| | `audit.test.ts` | 4 | Fixtures put `lastPulledHash` in state; `audit.ts` correctly reads the hash store now | | `state-migration.test.ts`, 2 in `drift.test.ts` | 4 | Asserted legacy fields survive state writes, which is exactly what the migration removed | | `reconcile-state-key.test.ts` | 4 | Asserted `lastPushedHash` lands in state; the shared push path records the baseline in the hash store instead | | `drift.test.ts` (`checkDriftForUpdate`) | 3 | Missing the new required `env` argument, so the hash-store path was `undefined` | | `drift.test.ts` (converged edge) | 1 | Pinned `both-diverged` for local == platform ≠ baseline, which `classifyDrift` now deliberately returns as `clean` (the documented invariant behind the phantom-drift fix, improvements.md #23) | | Pull/push spawn tests | 4 | Legacy-format state fixtures, refused by the migration gate | The last row matters most. Three of those were the regression guards #41 added for #22 (rename keeps the local filename; same-name clobber) and #23 (a stale baseline must not block a push). **They have never passed**, so those fixes had no working coverage. ## Changes - **CI**: `.github/workflows/ci.yml` runs `npm run build` + `npm test` on every PR and on pushes to `main`, on Node 20 and 22 (the `engines` range). It sets a job timeout and read-only permissions. - **Fixtures moved to the hash store**: - The spawn tests copy `src/` into a temp dir, so the engine's store resolves there; they seed `<tmp>/.vapi-state-hash/<env>/<uuid>`. - `drift.test.ts` seeds through `writeBaseline` under a throwaway `drift-test-<pid>` org and removes it afterwards. - `audit.ts` gains an optional `baselineReader` DI seam next to `stateLoader` / `listLocalIds`. That is the only production-code change, and the default is the existing `readBaseline(VAPI_ENV, uuid)`. - **`push-stale-baseline-noop` now tests what it says.** With empty `credentials`, `maybeBootstrapState` treats state as uninitialized and its bootstrap pull rewrote the stale baseline before the drift check ever ran. Migrating the fixture alone would have produced a green test that exercises nothing. It now seeds a dummy credential and asserts no bootstrap ran. - **Converged-edge test** rewritten to pin `clean`, with the rationale from `drift.ts`. - **Section J** (classifier short-circuit) rewritten around the hash store. The original bug (a rebuilt state section dropped the baseline) can't happen by construction now. The new test pins that separation, so moving the baseline back into the state entry fails it. - **`tool-assistant-cycle.test.ts`** deleted nothing it wrote: `updateToolAssistantRefs` records a baseline into the developer's *real* `.vapi-state-hash/test-fixture-org/` on every run. It now removes it. - `improvements.md` #32. ## Evidence - `npm test`: **355 / 355 pass** (was 337 / 357; two fewer tests because Section J's four tests became one and one `state-migration` test was split in two). `npm run build` is clean. - **Mutation check:** removing the agree-gate (`localHash === platformHash` in both `classifyDrift` and `checkDriftForUpdate`) fails `push-stale-baseline-noop` and the converged-edge test. Restoring it passes both. - **Isolation check:** after a full run, the real `.vapi-state-hash/` holds no test-written baseline. The `drift-test-<pid>` folder is removed, and the `tool-assistant-cycle` baseline file is deleted, leaving only an empty gitignored `test-fixture-org/` folder. ## Not in this PR `tsconfig.json` still includes only `src/**/*`, so the typecheck never sees `tests/`. That's why stale fixture shapes compiled silently. Including `tests/` surfaces **37 existing type errors**; that's its own change. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

ELI5
Problem. Even when you ran a scoped push — say
npm run push -- <env> assistants/foo.mdto update one assistant —the engine rewrote the entire state file. Any pre-existing drift
in unrelated state entries (UUIDs from earlier sessions, untracked
local files, etc.) swept into the focused commit. Reviewers couldn't
tell from the state-file diff "what did this push actually change?"
and the state file became a pile of side effects accumulated across
sessions instead of a precise record of intent.
What this fix does. During a push, the engine tracks which
resourceIds it actually mutated (a per-sectionSet<string>). Atend-of-run, for scoped pushes only, it loads the on-disk state
fresh, replaces only the touched entries with the in-memory version,
and leaves everything else alone. Full pushes (no scope) still write
wholesale (existing behavior). Credentials are always replaced
because bootstrap pull populates them every push regardless.
This depends on Stack F's
ResourceStatebecause we need per-entrymetadata to distinguish "stale" from "just-not-touched."
Outcome you'll notice. A one-file
npm run pushproduces aone-file diff in the state file — same scope as the resource change.
Reviewers can read the state diff and tell "this push updated
assistant
foo, here's its new hash" cleanly. Pre-existing driftelsewhere in state stays where it is until you explicitly address it.
When push is scoped to specific paths, only update state entries for
the resources actually touched. A surgical push of two files used to
rewrite the entire state file, sweeping in pre-existing drift from
earlier pushes (improvements.md #15) and producing noisy diffs that
hide the actual scope of the change.
Files:
For each section, replace only touched.X resourceIds with the in-memory
version; leave the rest of disk's section as-is. Credentials are
always replaced wholesale (bootstrap pull populates them on every
push). Pure data, no I/O — safe to test directly.
records the resourceId. End-of-run, partial pushes call
mergeScoped(loadState(), state, touched) before saveState; full
pushes save wholesale (existing behavior).
drift in untouched stays, credentials always replaced.
Closes improvements.md #15.
🤖 Generated with Claude Code