From 259746da8d369f7e5300994293c7b4a9c163289d Mon Sep 17 00:00:00 2001 From: Dhruva Reddy Date: Fri, 1 May 2026 12:50:10 -0700 Subject: [PATCH] refactor: scoped state writes preserve untouched entries MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## ELI5 **Problem.** Even when you ran a *scoped* push ā€” say `npm run push -- 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`). 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) --- improvements.md | 2 +- src/push.ts | 57 ++++++++++++++++- src/state-merge.ts | 86 +++++++++++++++++++++++++ tests/state-merge.test.ts | 130 ++++++++++++++++++++++++++++++++++++++ 4 files changed, 273 insertions(+), 2 deletions(-) create mode 100644 src/state-merge.ts create mode 100644 tests/state-merge.test.ts diff --git a/improvements.md b/improvements.md index 6d9590f..c9f078e 100644 --- a/improvements.md +++ b/improvements.md @@ -66,7 +66,7 @@ you which stack PR closes the row.** | 12 | State file accumulates UUIDs without source files | Silent gitops drift | None | Partial | | 13 | `.agent/` and `.claude/handoffs/` not gitignored | `git add -A` sweeps PII handoff scratch | None | RESOLVED 2026-04-30 (Stack A) | | 14 | Multi-file push undocumented | Discoverability | None | RESOLVED 2026-04-30 (Stack A) | -| 15 | Scoped push rewrites entire state file | Pre-existing drift sweeps into focused commits | #4 | Open (Stack J planned) | +| 15 | Scoped push rewrites entire state file | Pre-existing drift sweeps into focused commits | #4 | RESOLVED 2026-04-30 (Stack J) | | 16 | No CLI runner for simulation suites | Engine pushes them, can't run them | None | RESOLVED 2026-04-30 (Stack E) | | 17 | State file key-order churn produces noisy diffs | Reorderings hide real changes | None | RESOLVED 2026-04-30 (Stack B) | | 18 | Structured-output `name` capped at 40 chars (no warning) | Push fails partway after partial application | None | RESOLVED 2026-04-30 (Stack D) | diff --git a/src/push.ts b/src/push.ts index e086a30..d98ba26 100644 --- a/src/push.ts +++ b/src/push.ts @@ -15,6 +15,7 @@ import { import { summarizeFindings, validateResources } from "./validate.ts"; import { checkDriftForUpdate } from "./drift.ts"; import { writeSnapshot } from "./snapshot.ts"; +import { mergeScoped } from "./state-merge.ts"; // Map a resource label to its state-file key. Used for snapshotting (Stack H) // ā€” snapshot directories are keyed by the same names the state file uses. @@ -777,6 +778,40 @@ function filterResourcesByPaths( return resources.filter((r) => matchingIds.has(r.resourceId)); } +// Stack J ā€” track which resourceIds were actually written during this apply. +// On scoped push, the end-of-run save merges only these entries back into +// the on-disk state, leaving untouched entries alone. Without this, a scoped +// push (`npm run push -- assistants/foo.md`) sweeps in any pre-existing +// drift across the entire state file (improvements.md #15). +interface TouchedSets { + tools: Set; + structuredOutputs: Set; + assistants: Set; + squads: Set; + personalities: Set; + scenarios: Set; + simulations: Set; + simulationSuites: Set; + evals: Set; + // refreshed on every push (bootstrap pull populates them) + credentials: Set; +} + +function emptyTouchedSets(): TouchedSets { + return { + tools: new Set(), + structuredOutputs: new Set(), + assistants: new Set(), + squads: new Set(), + personalities: new Set(), + scenarios: new Set(), + simulations: new Set(), + simulationSuites: new Set(), + evals: new Set(), + credentials: new Set(), + }; +} + // ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ā”€ // Auto-Dependency Resolution // When pushing a resource with missing dependencies, auto-apply them first @@ -966,6 +1001,10 @@ async function main(): Promise { // Load current state (needed for reference resolution even in partial apply) let state = loadState(); + // Stack J ā€” track which resourceIds we actually mutate so the end-of-run + // save can merge into existing on-disk state instead of rewriting wholesale. + const touched: TouchedSets = emptyTouchedSets(); + // Track what was applied for summary const applied: Record = { tools: 0, @@ -1203,6 +1242,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(tool.data), }); + touched.tools.add(tool.resourceId); applied.tools++; } catch (error) { console.error(formatApiError(tool.resourceId, error)); @@ -1221,6 +1261,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(output.data), }); + touched.structuredOutputs.add(output.resourceId); applied.structuredOutputs++; } catch (error) { console.error(formatApiError(output.resourceId, error)); @@ -1252,6 +1293,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(assistant.data), }); + touched.assistants.add(assistant.resourceId); applied.assistants++; } catch (error) { console.error(formatApiError(assistant.resourceId, error)); @@ -1277,6 +1319,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(squad.data), }); + touched.squads.add(squad.resourceId); applied.squads++; } catch (error) { console.error(formatApiError(squad.resourceId, error)); @@ -1295,6 +1338,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(personality.data), }); + touched.personalities.add(personality.resourceId); applied.personalities++; } catch (error) { console.error(formatApiError(personality.resourceId, error)); @@ -1313,6 +1357,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(scenario.data), }); + touched.scenarios.add(scenario.resourceId); applied.scenarios++; } catch (error) { console.error(formatApiError(scenario.resourceId, error)); @@ -1331,6 +1376,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(simulation.data), }); + touched.simulations.add(simulation.resourceId); applied.simulations++; } catch (error) { console.error(formatApiError(simulation.resourceId, error)); @@ -1349,6 +1395,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(suite.data), }); + touched.simulationSuites.add(suite.resourceId); applied.simulationSuites++; } catch (error) { console.error(formatApiError(suite.resourceId, error)); @@ -1366,6 +1413,7 @@ async function main(): Promise { uuid, lastPushedHash: hashPayload(evalResource.data), }); + touched.evals.add(evalResource.resourceId); applied.evals++; } catch (error) { console.error(formatApiError(evalResource.resourceId, error)); @@ -1455,7 +1503,14 @@ async function main(): Promise { ); } else { try { - await saveState(state); + // Stack J ā€” for scoped pushes, only persist entries we actually + // mutated. Re-load disk state and merge our touched entries on top + // so unrelated drift in untouched entries is left alone. A bare + // (non-partial) push falls through to the wholesale save. + const stateToWrite = partial + ? mergeScoped(loadState(), state, touched) + : state; + await saveState(stateToWrite); } catch (saveError) { console.error( "\nāš ļø Failed to persist state file after apply:", diff --git a/src/state-merge.ts b/src/state-merge.ts new file mode 100644 index 0000000..a88153d --- /dev/null +++ b/src/state-merge.ts @@ -0,0 +1,86 @@ +// Stack J ā€” scoped state writes. +// +// On a scoped push (`npm run push -- assistants/foo.md`), the engine +// previously rewrote the entire state file even when only one assistant +// was applied. Pre-existing dashboard drift in unrelated state entries +// would silently sweep into the commit-able diff. +// +// `mergeScoped` produces a new state object where: +// - Every entry NOT in `touched` is copied from `onDisk` (untouched ā€” +// leaves pre-existing drift alone). +// - Every entry IN `touched` is taken from `inMemory` (the live state +// after the push run). +// +// Untouched-on-platform entries that no longer have a local file are +// preserved AS-IS ā€” they're outside the scope of this push and will be +// reconciled by a subsequent full push or pull. +// +// Credentials always refresh from `inMemory` because bootstrap pull +// rewrites them whether or not a partial filter targeted them. + +import type { ResourceState, StateFile } from "./types.ts"; + +export interface TouchedSets { + tools: Set; + structuredOutputs: Set; + assistants: Set; + squads: Set; + personalities: Set; + scenarios: Set; + simulations: Set; + simulationSuites: Set; + evals: Set; + credentials: Set; +} + +const SECTIONS: Array = [ + "tools", + "structuredOutputs", + "assistants", + "squads", + "personalities", + "scenarios", + "simulations", + "simulationSuites", + "evals", +]; + +export function mergeScoped( + onDisk: StateFile, + inMemory: StateFile, + touched: TouchedSets, +): StateFile { + const merged: StateFile = { + credentials: { ...inMemory.credentials }, // always refresh + tools: {}, + structuredOutputs: {}, + assistants: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; + + for (const section of SECTIONS) { + const touchedIds = touched[section]; + const out: Record = {}; + + // Copy all on-disk entries that weren't touched (leave them alone). + for (const [id, entry] of Object.entries(onDisk[section])) { + if (!touchedIds.has(id)) { + out[id] = entry; + } + } + // Overlay in-memory entries that WERE touched. + for (const id of touchedIds) { + const entry = inMemory[section][id]; + if (entry) out[id] = entry; + } + + merged[section] = out; + } + + return merged; +} diff --git a/tests/state-merge.test.ts b/tests/state-merge.test.ts new file mode 100644 index 0000000..3e6dec8 --- /dev/null +++ b/tests/state-merge.test.ts @@ -0,0 +1,130 @@ +import test from "node:test"; +import assert from "node:assert/strict"; +import { mergeScoped, type TouchedSets } from "../src/state-merge.ts"; +import type { StateFile } from "../src/types.ts"; + +// Stack J ā€” scoped state-merge coverage. The plan's #15 fix: a scoped push +// must NOT sweep pre-existing drift (state entries unrelated to the touched +// resources) into the commit-able state diff. + +function emptyState(): StateFile { + return { + credentials: {}, + assistants: {}, + structuredOutputs: {}, + tools: {}, + squads: {}, + personalities: {}, + scenarios: {}, + simulations: {}, + simulationSuites: {}, + evals: {}, + }; +} + +function emptyTouched(): TouchedSets { + return { + tools: new Set(), + structuredOutputs: new Set(), + assistants: new Set(), + squads: new Set(), + personalities: new Set(), + scenarios: new Set(), + simulations: new Set(), + simulationSuites: new Set(), + evals: new Set(), + credentials: new Set(), + }; +} + +test("mergeScoped: untouched entries copied from on-disk state", () => { + const onDisk = emptyState(); + onDisk.assistants["unrelated-1"] = { uuid: "u-1", lastPulledHash: "h-1" }; + onDisk.assistants["unrelated-2"] = { uuid: "u-2", lastPulledHash: "h-2" }; + + const inMemory = emptyState(); + // In-memory state has unrelated-1 with a different hash (drift) and a + // newly-touched assistant. mergeScoped should copy unrelated-1 from disk + // (untouched), and only take touched-agent from in-memory. + inMemory.assistants["unrelated-1"] = { uuid: "u-1", lastPulledHash: "h-X" }; + inMemory.assistants["touched-agent"] = { + uuid: "u-3", + lastPushedHash: "fresh", + }; + + const touched = emptyTouched(); + touched.assistants.add("touched-agent"); + + const merged = mergeScoped(onDisk, inMemory, touched); + assert.equal(merged.assistants["unrelated-1"]!.lastPulledHash, "h-1"); + assert.equal(merged.assistants["unrelated-2"]!.lastPulledHash, "h-2"); + assert.equal(merged.assistants["touched-agent"]!.lastPushedHash, "fresh"); +}); + +test("mergeScoped: touched entries take in-memory version", () => { + const onDisk = emptyState(); + onDisk.assistants["agent-a"] = { uuid: "u-1", lastPulledHash: "old" }; + + const inMemory = emptyState(); + inMemory.assistants["agent-a"] = { + uuid: "u-1", + lastPulledHash: "old", + lastPushedHash: "new", + }; + + const touched = emptyTouched(); + touched.assistants.add("agent-a"); + + const merged = mergeScoped(onDisk, inMemory, touched); + assert.equal(merged.assistants["agent-a"]!.lastPushedHash, "new"); +}); + +test("mergeScoped: credentials always refreshed from in-memory", () => { + const onDisk = emptyState(); + onDisk.credentials["openai"] = { uuid: "old-cred-uuid" }; + + const inMemory = emptyState(); + inMemory.credentials["openai"] = { uuid: "new-cred-uuid" }; + // Bootstrap pull also added a new credential + inMemory.credentials["langfuse"] = { uuid: "lang-cred-uuid" }; + + const touched = emptyTouched(); // credentials are NOT explicitly touched + + const merged = mergeScoped(onDisk, inMemory, touched); + assert.equal(merged.credentials["openai"]!.uuid, "new-cred-uuid"); + assert.equal(merged.credentials["langfuse"]!.uuid, "lang-cred-uuid"); +}); + +test("mergeScoped: empty touched preserves all on-disk state", () => { + const onDisk = emptyState(); + onDisk.assistants["a"] = { uuid: "u-a" }; + onDisk.tools["t"] = { uuid: "u-t" }; + + const inMemory = emptyState(); // empty (e.g., scoped to a missing path) + + const touched = emptyTouched(); + + const merged = mergeScoped(onDisk, inMemory, touched); + assert.deepEqual(merged.assistants, { a: { uuid: "u-a" } }); + assert.deepEqual(merged.tools, { t: { uuid: "u-t" } }); +}); + +test("mergeScoped: cross-section isolation (touched assistants do NOT affect tools section)", () => { + const onDisk = emptyState(); + onDisk.tools["unrelated-tool"] = { uuid: "u-tool", lastPulledHash: "tool-hash" }; + onDisk.assistants["agent-a"] = { uuid: "u-old" }; + + const inMemory = emptyState(); + inMemory.assistants["agent-a"] = { uuid: "u-old", lastPushedHash: "fresh" }; + // In-memory has an unrelated drift in tools section that should NOT bleed in + inMemory.tools["unrelated-tool"] = { uuid: "u-tool", lastPulledHash: "drifted" }; + + const touched = emptyTouched(); + touched.assistants.add("agent-a"); // ONLY assistants touched + + const merged = mergeScoped(onDisk, inMemory, touched); + // tools section preserved from disk + assert.equal(merged.tools["unrelated-tool"]!.lastPulledHash, "tool-hash"); + // assistants section: touched entry takes in-memory + assert.equal(merged.assistants["agent-a"]!.lastPushedHash, "fresh"); +});