diff --git a/docs/guides/how-it-works.md b/docs/guides/how-it-works.md index 0d08adf..293ae59 100644 --- a/docs/guides/how-it-works.md +++ b/docs/guides/how-it-works.md @@ -75,6 +75,11 @@ Push adds two more: `⏭️` (conflict prompt → kept dashboard, push skipped) Mental model: `⬆️` flows UP (push), `⬇️` flows DOWN (pull), `📝` is the engine doing routine file I/O. +The pull summary's updated count includes an existing resource only when +the serialized file content differs from what is on disk. The normal pull +path still writes the file when its content is identical; the file-write +icon describes that write, while the count describes content changes. + ## Listing completeness Vapi list endpoints cap a response at 100 items and expose no page cursor — only `createdAt` comparison filters. The engine pages backwards through `createdAt` until it gets a short page, so pull, push's invalid-mapping detection, `delete`'s orphan sweep, `audit`, and the credential reverse-map all see the whole type instead of the first hundred. When completeness cannot be proven — an endpoint that ignores the cursor params, a payload with no `createdAt`, or the page-count backstop — the engine says so on stderr. Treat that warning as "do not infer deletion from absence for this type". diff --git a/improvements.md b/improvements.md index 43be71a..9dccd92 100644 --- a/improvements.md +++ b/improvements.md @@ -2061,6 +2061,53 @@ dependency check, as `check-payload.ts` already does. --- +## 40. Pull summary reports dashboard-ahead resources as preserved + +**Discovered:** 2026-10-10, while reviewing pull/apply output after a dashboard edit. + +### Problem + +The pull engine syncs a dashboard-ahead resource down when the local file is +unchanged, but the final drift summary says the resource was "preserved +locally". The summary also counts every existing resource as updated, even +when the serialized file content is identical to disk. + +### Current behavior + +Each resource line correctly shows a down arrow and says that the unchanged +local file is being synced down. The final summary uses stale preserve-local +wording. Pull and apply also report unchanged existing resources as updated +because the counter follows the resource's existence in state rather than +whether the generated file content changed. + +The stale summary wording dates to commit 4136fde, which added drift labels +for preserved resources. Commit 965a057 later changed dashboard-ahead +handling to sync the dashboard version down, but did not update that summary +text. Apply uses the same pull engine and reporting path. + +### Risk + +The summary contradicts the action shown on the resource line and can make a +clean pull look like it changed every tracked resource. That obscures actual +changes during review. + +### Current mitigation + +Read each resource line for its action and compare files directly to identify +which resources changed. + +### Possible fix + +Label dashboard-ahead summary entries as synced down. Count an existing +resource as updated only when the generated file content differs from disk; +keep the normal file write behavior for identical content. + +### Status + +**Open.** + +--- + ## Out of scope (intentionally not improvements) - **State file is identity-only and not git-ignored.** It's intentionally diff --git a/src/pull.ts b/src/pull.ts index 2eca30a..80fff46 100644 --- a/src/pull.ts +++ b/src/pull.ts @@ -1,6 +1,6 @@ import { execSync } from "child_process"; import { existsSync, readdirSync, statSync } from "fs"; -import { mkdir, writeFile } from "fs/promises"; +import { mkdir, readFile, writeFile } from "fs/promises"; import { dirname, extname, join, relative, resolve } from "path"; import { fileURLToPath } from "url"; import { stringify } from "yaml"; @@ -530,7 +530,7 @@ async function writeResourceFile( resourceType: ResourceType, resourceId: string, data: Record, -): Promise { +): Promise<{ filePath: string; changed: boolean }> { const folderPath = FOLDER_MAP[resourceType]; const dir = join(RESOURCES_DIR, folderPath); @@ -544,11 +544,12 @@ async function writeResourceFile( await mkdir(dirname(filePath), { recursive: true }); const yamlContent = stringify(cleanedData, YAML_OPTIONS); + const content = `---\n${yamlContent}---\n\n${systemPrompt}\n`; + const changed = + !existsSync(filePath) || (await readFile(filePath, "utf-8")) !== content; + await writeFile(filePath, content); - const mdContent = `---\n${yamlContent}---\n\n${systemPrompt}\n`; - await writeFile(filePath, mdContent); - - return filePath; + return { filePath, changed }; } } @@ -556,11 +557,12 @@ async function writeResourceFile( const filePath = join(dir, `${resourceId}.yml`); await mkdir(dirname(filePath), { recursive: true }); - const yamlContent = stringify(data, YAML_OPTIONS); - - await writeFile(filePath, yamlContent); + const content = stringify(data, YAML_OPTIONS); + const changed = + !existsSync(filePath) || (await readFile(filePath, "utf-8")) !== content; + await writeFile(filePath, content); - return filePath; + return { filePath, changed }; } // Write the dashboard version of a resource as a sibling @@ -588,11 +590,12 @@ export async function writeDashboardBackup( .toISOString() .replace(/\.\d{3}Z$/, "") .replace(/:/g, "-"); - return writeResourceFile( + const backup = await writeResourceFile( resourceType, `${resourceId}.${timestamp}.bkp`, withCredNames, ); + return backup.filePath; } // ───────────────────────────────────────────────────────────────────────────── @@ -1065,6 +1068,7 @@ export async function pullResourceType( // fallback. Encodes the 3-step pipeline (cleanResource → resolve refs → // replace credential UUIDs) plus the _platformDefault marker. const withCredNames = canonicalizeForHash(resource, state, credReverse); + let fileChanged = false; if (bootstrap) { const icon = isPlatformDefault ? "🔒" : isNew ? "✨" : "📝"; @@ -1073,11 +1077,13 @@ export async function pullResourceType( ); } else { // Write to file - const filePath = await writeResourceFile( + const written = await writeResourceFile( resourceType, resourceId, withCredNames, ); + fileChanged = written.changed; + const filePath = written.filePath; const icon = isPlatformDefault ? "🔒" : isNew ? "✨" : "📝"; const relPath = relative(BASE_DIR, filePath); console.log( @@ -1086,7 +1092,7 @@ export async function pullResourceType( } if (isNew) created++; - else updated++; + else if (fileChanged) updated++; // Record the `{ uuid }` mapping in state and write the drift baseline into // the hash store. @@ -1250,7 +1256,7 @@ function printDriftSummary(counts: DriftDirectionCounts): void { const lines: string[] = []; if (counts["dashboard-ahead"] > 0) { lines.push( - ` ✏️ dashboard-ahead : ${counts["dashboard-ahead"]} (these resources have UI edits since last pull; preserved locally)`, + ` ⬇️ dashboard-ahead : ${counts["dashboard-ahead"]} (dashboard changes synced down to local)`, ); } if (counts["local-ahead"] > 0) { diff --git a/tests/pull-summary-output.test.ts b/tests/pull-summary-output.test.ts new file mode 100644 index 0000000..649dc47 --- /dev/null +++ b/tests/pull-summary-output.test.ts @@ -0,0 +1,111 @@ +import assert from "node:assert/strict"; +import { cpSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, symlinkSync } from "node:fs"; +import { spawnSync } from "node:child_process"; +import { tmpdir } from "node:os"; +import { join } from "node:path"; +import test from "node:test"; +import { Worker } from "node:worker_threads"; +import { fileURLToPath } from "node:url"; + +const REPO_ROOT = fileURLToPath(new URL("..", import.meta.url)); +const ENV = "test-pull-summary"; +const UUID = "aaaaaaaa-1111-1111-1111-111111111111"; + +function assistant(marker: string) { + return { + id: UUID, + orgId: "org-test", + name: "Summary Test Assistant", + model: { provider: "openai", model: "gpt-4o", messages: [{ role: "system", content: "marker:" + marker }] }, + voice: { provider: "11labs", voiceId: "burt" }, + }; +} + +async function startStub(initialBody: unknown) { + const source = [ + "const http = require('node:http');", + "const { parentPort, workerData } = require('node:worker_threads');", + "let body = workerData.body;", + "const server = http.createServer((req, res) => {", + " res.statusCode = 200;", + " res.setHeader('Content-Type', 'application/json');", + " res.end(JSON.stringify((req.url || '').startsWith('/assistant') ? [body] : []));", + "});", + "server.listen(0, '127.0.0.1', () => parentPort.postMessage({ type: 'listening', port: server.address().port }));", + "parentPort.on('message', (msg) => {", + " if (msg && msg.type === 'set-body') { body = msg.body; parentPort.postMessage({ type: 'body-updated' }); }", + " if (msg && msg.type === 'shutdown') server.close(() => process.exit(0));", + "});", + ].join("\n"); + const worker = new Worker(source, { eval: true, workerData: { body: initialBody } }); + const port = await new Promise((resolve, reject) => { + worker.once("error", reject); + worker.on("message", (msg: { type: string; port?: number }) => { + if (msg.type === "listening" && typeof msg.port === "number") resolve(msg.port); + }); + }); + return { worker, port }; +} + +function setAssistant(worker: Worker, body: unknown): Promise { + return new Promise((resolve) => { + const listener = (msg: { type: string }) => { + if (msg.type !== "body-updated") return; + worker.off("message", listener); + resolve(); + }; + worker.on("message", listener); + worker.postMessage({ type: "set-body", body }); + }); +} + +function runPull(dir: string, port: number) { + return spawnSync("node", ["--import", "tsx", "src/pull.ts", ENV], { + cwd: dir, + env: { ...process.env, VAPI_TOKEN: "fake-token", VAPI_BASE_URL: "http://127.0.0.1:" + port }, + encoding: "utf-8", + timeout: 30_000, + }); +} + +test("pull: dashboard-ahead reports sync-down and identical writes are not counted as updates", async () => { + const dir = mkdtempSync(join(tmpdir(), "vapi-pull-summary-")); + cpSync(join(REPO_ROOT, "src"), join(dir, "src"), { recursive: true }); + cpSync(join(REPO_ROOT, "package.json"), join(dir, "package.json")); + symlinkSync(join(REPO_ROOT, "node_modules"), join(dir, "node_modules"), "dir"); + const { worker, port } = await startStub(assistant("v1")); + + try { + const first = runPull(dir, port); + assert.equal(first.status, 0, "first pull failed: " + first.stderr); + const assistantDir = join(dir, "resources", ENV, "assistants"); + const file = readdirSync(assistantDir).find((name) => name.endsWith(".md")); + assert.ok(file, "first pull should write an assistant markdown file"); + const filePath = join(assistantDir, file); + assert.match(readFileSync(filePath, "utf-8"), /marker:v1/); + + await setAssistant(worker, assistant("v2")); + const dashboardAhead = runPull(dir, port); + assert.equal(dashboardAhead.status, 0, "dashboard-ahead pull failed: " + dashboardAhead.stderr); + assert.match(dashboardAhead.stdout, /⬇️\s+summary-test-assistant-[a-f0-9]{8} \(dashboard ahead, local unchanged — syncing down\)/); + assert.match(dashboardAhead.stdout, /⬇️\s+dashboard-ahead\s*:\s*1\s+\(dashboard changes synced down to local\)/); + assert.match(dashboardAhead.stdout, /assistants: 0 new, 1 updated/); + assert.doesNotMatch(dashboardAhead.stdout, /dashboard-ahead.*preserved locally/); + assert.match(readFileSync(filePath, "utf-8"), /marker:v2/); + + const before = statSync(filePath).mtimeMs; + await new Promise((resolve) => setTimeout(resolve, 20)); + const identical = runPull(dir, port); + assert.equal(identical.status, 0, "identical pull failed: " + identical.stderr); + assert.match(identical.stdout, /assistants: 0 new, 0 updated/); + assert.match(identical.stdout, /📝\s+summary-test-assistant-[a-f0-9]{8} -> .*summary-test-assistant-[a-f0-9]{8}\.md/); + assert.ok(statSync(filePath).mtimeMs > before, "pull should still write identical serialized content"); + } finally { + worker.postMessage({ type: "shutdown" }); + await new Promise((resolve) => { + worker.once("exit", () => resolve()); + setTimeout(() => worker.terminate().then(() => resolve()).catch(() => resolve()), 1000); + }); + rmSync(dir, { recursive: true, force: true }); + } +});