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"); +});