Skip to content

Commit 622596f

Browse files
committed
feat(push): enhance safety and clarity in push and apply operations
This commit introduces several improvements to the push and apply processes, focusing on safety and user experience. Key changes include: - Updated the `apply` command to incorporate a validation step before executing the pull and push operations, ensuring that schema errors are caught early. - Enhanced logging in the `push` command to provide clearer context during scoped runs, suppressing unnecessary output while still informing users of the resources being applied. - Introduced a `quiet` option in the resource loading logic to minimize clutter in the console output during scoped operations. These changes aim to reduce the risk of errors during deployment and improve the overall clarity of the command outputs.
1 parent e84566d commit 622596f

4 files changed

Lines changed: 127 additions & 5 deletions

File tree

improvements.md

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,10 @@ you which stack PR closes the row.**
7373
| 19 | No `maxTokens` floor warning for tool-using assistants | `maxTokens: 1` bricks the assistant silently | None | RESOLVED 2026-04-30 (Stack D) |
7474
| 20 | Prompt vocabulary leaks into TTS | `Reason.` becomes verbal contaminant | None | Partial — Stack D heuristic |
7575
| 21 | `.vapi-ignore` was pull-only (push could silently delete) | `--force` push DELETEd dashboard-only opt-outs | None | RESOLVED 2026-05-11 (#TBD) |
76+
| 22 | Dashboard rename recreated the local file (duplicate per UUID) | Duplicate files after dashboard renames | None | RESOLVED (see entry) |
77+
| 23 | Phantom `both-diverged` (canonicalization + agree-gate) | Untouched resources blocked on first push | None | RESOLVED 2026-06-03 (#TBD) |
78+
| 24 | Bare `push` is too easy to use as the deploy path | Raw push skips apply's validate+pull safety | None | Open — mitigated by per-resource drift gate |
79+
| 25 | Interactive flows lack automated coverage | Picker/conflict-prompt regressions ship silently | None | Open — scheduled for the test-update iteration |
7680

7781
---
7882

@@ -1156,6 +1160,72 @@ RESOLVED 2026-06-03 (#TBD — PR number updates when opened).
11561160

11571161
---
11581162

1163+
## 24. Bare `push` is too easy to use as the deploy path
1164+
1165+
**Problem.** PR #41 review (dhruva-reddy): operators shouldn't have to memorize
1166+
"`validate` && `apply` && avoid `push`". Raw `push` skips apply's
1167+
validate-then-pull safety yet reads like the natural deploy verb, so it keeps
1168+
getting used as one.
1169+
1170+
**Current behavior.** `npm run push -- <org>` (`src/push.ts`) runs the raw
1171+
push directly. Apply now embeds validation (`src/apply.ts` runs
1172+
`validate-cmd` before its pull stage) and defaults conflict handling to the
1173+
per-resource prompt, but bare push remains a single flag-less command.
1174+
1175+
**Risk.** Lower than when the review was written — push now has the
1176+
per-resource drift gate (`src/push.ts`, `checkDriftForUpdate` +
1177+
`promptDriftResolution`), so out-of-band dashboard edits prompt or block
1178+
instead of being clobbered. Remaining gap: push skips schema validation and
1179+
the pull/merge pass, so it can still 400 mid-run or PATCH from a stale local
1180+
view (one-sided, detectable, but avoidable).
1181+
1182+
**Current mitigation.** Docs hierarchy (`AGENTS.md` "Choosing a sync
1183+
command"), the orphan-YAML gate, and the drift gate/prompt.
1184+
1185+
**Possible fix.** Reviewer's suggested shape: make raw local→dashboard push
1186+
explicit/advanced via `npm run push -- <org> --direct`; bare `push` either
1187+
delegates to safe apply or prints guidance telling the operator to use
1188+
`apply` unless `--direct` is intended. Backwards-incompatible for automation
1189+
that calls bare `push` — needs a deprecation window.
1190+
1191+
**Status.** Open (mitigated 2026-06-04 by the per-resource drift gate and
1192+
apply's built-in validate).
1193+
1194+
---
1195+
1196+
## 25. Interactive flows lack automated coverage
1197+
1198+
**Problem.** PR #41 review (dhruva-reddy): the interactive picker
1199+
(`src/interactive.ts` — Back/Cancel/empty-selection states) and the
1200+
local-wins-apply-stays-clean invariant have no automated tests, and these are
1201+
exactly the paths that regress while unit tests for path parsing still pass.
1202+
1203+
**Current behavior.** No tests cover: (a) the picker's step machine
1204+
(org → scope → pick → confirm → execute, including Back/Cancel/empty), (b)
1205+
the push conflict prompt's three choices, (c) the regression "apply the same
1206+
resource twice → second run is clean, not `both-diverged`" (root cause fixed
1207+
by response-hash baselines in `src/push.ts` `writeBaselineFromResponse` and
1208+
the `classifyDrift` live-agreement invariant in `src/drift.ts`, but
1209+
unguarded by tests).
1210+
1211+
**Risk.** Silent regressions in the most operator-visible surfaces; the
1212+
existing suite also predates the hash-store refactor and will fail until
1213+
reconciled.
1214+
1215+
**Current mitigation.** Manual run-throughs per stage (see
1216+
`docs/superpowers/specs/2026-06-03-state-hash-store-design.md`).
1217+
1218+
**Possible fix.** The planned test-update iteration for the hash-store
1219+
refactor must include, by name: picker smoke test (Back/Cancel/empty),
1220+
conflict-prompt choice handling (mock TTY + select), and the
1221+
double-apply-stays-clean regression (`localHash=B, platformHash=B,
1222+
baseline=A → clean` plus baseline refresh after push).
1223+
1224+
**Status.** Open — scheduled as part of the test-update iteration that
1225+
reconciles the suite with the slim-state/hash-store engine.
1226+
1227+
---
1228+
11591229
## Out of scope (intentionally not improvements)
11601230

11611231
- **State file is identity-only and not git-ignored.** It's intentionally

src/apply.ts

Lines changed: 14 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -106,14 +106,27 @@ export async function runApply(): Promise<void> {
106106
"═══════════════════════════════════════════════════════════════",
107107
);
108108
console.log(`🔄 Vapi GitOps Apply - Environment: ${env}`);
109-
console.log(" Pull → Merge → Push");
109+
console.log(" Validate → Pull → Merge → Push");
110110
if (hasForce) {
111111
console.log(" ⚠️ Deletions enabled (--force)");
112112
}
113113
console.log(
114114
"═══════════════════════════════════════════════════════════════\n",
115115
);
116116

117+
// Apply is the one-shot safe deploy verb: run the local schema validators
118+
// first so a shape error never burns a pull/push cycle. No network call —
119+
// this is the same check as `npm run validate`, just built in. It stays
120+
// available standalone for CI / pre-flight use.
121+
const validateCmd = `npx tsx src/validate-cmd.ts ${env}`;
122+
const validateExit = runPassthrough(validateCmd);
123+
if (validateExit !== 0) {
124+
console.error(
125+
"\n❌ Validation failed — fix the schema errors above before applying.",
126+
);
127+
process.exit(1);
128+
}
129+
117130
const pullCmd = `npx tsx src/pull.ts ${env} ${pullArgs}`.trim();
118131
const pullExit = runPassthrough(pullCmd);
119132
if (pullExit !== 0) {

src/push.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1391,9 +1391,18 @@ async function main(): Promise<void> {
13911391
// a `--force` push can't silently delete a dashboard resource the repo
13921392
// has explicitly opted out of managing. Bootstrap-pull paths read their
13931393
// own patterns directly and are unaffected by this constant.
1394-
console.log("\n📂 Loading resources...\n");
1394+
if (partial) {
1395+
console.log(
1396+
"\n📂 Loading resources (scoped run — full set loaded quietly for reference resolution; only the selection below is applied)...\n",
1397+
);
1398+
} else {
1399+
console.log("\n📂 Loading resources...\n");
1400+
}
13951401
const ignorePatterns = FORCE_DELETE ? [] : loadIgnorePatterns();
1396-
const loadOpts = { ignorePatterns };
1402+
// In a scoped run, suppress the per-file "Loaded" chatter — printing every
1403+
// local resource makes the blast radius look larger than it is. The scoped
1404+
// selection is printed explicitly after filtering instead.
1405+
const loadOpts = { ignorePatterns, quiet: partial };
13971406
const allToolsRaw = await loadResources<Record<string, unknown>>(
13981407
"tools",
13991408
loadOpts,
@@ -1621,6 +1630,31 @@ async function main(): Promise<void> {
16211630
? filterResourcesByPaths(allEvals, "evals")
16221631
: [];
16231632

1633+
// Scoped run: print exactly what's in scope (the load above was quiet).
1634+
if (partial) {
1635+
const scoped: Array<[string, ResourceFile<Record<string, unknown>>[]]> = [
1636+
["tools", tools],
1637+
["structuredOutputs", structuredOutputs],
1638+
["assistants", assistants],
1639+
["squads", squads],
1640+
["personalities", personalities],
1641+
["scenarios", scenarios],
1642+
["simulations", simulations],
1643+
["simulationSuites", simulationSuites],
1644+
["evals", evals],
1645+
];
1646+
let any = false;
1647+
for (const [label, list] of scoped) {
1648+
for (const r of list) {
1649+
console.log(` 📦 In scope: ${label}/${r.resourceId}`);
1650+
any = true;
1651+
}
1652+
}
1653+
if (!any) {
1654+
console.log(" ⚠️ Nothing matched the requested scope.");
1655+
}
1656+
}
1657+
16241658
// Auto-dependency resolution context
16251659
const autoApplied = new Set<string>();
16261660
const autoAppliedTools: ResourceFile<Record<string, unknown>>[] = [];

src/resources.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@ import type { ResourceFile, ResourceType, StateFile } from "./types.ts";
1515
// preserve the pre-change behavior.
1616
export interface LoadOptions {
1717
ignorePatterns?: string[];
18+
// Suppress per-file "📦 Loaded" / "📁 No <type> directory" chatter. Used by
19+
// scoped (single-file / --type) pushes: the FULL set is still loaded for
20+
// reference resolution, but printing every file makes the blast radius look
21+
// larger than it is — the caller prints the scoped selection instead.
22+
quiet?: boolean;
1823
}
1924

2025
// Map resource types to their folder paths (relative to resources/)
@@ -208,7 +213,7 @@ export async function loadResources<T>(
208213
const ignorePatterns = options.ignorePatterns ?? [];
209214

210215
if (!existsSync(resourceDir)) {
211-
console.log(`📁 No ${type} directory found, skipping...`);
216+
if (!options.quiet) console.log(`📁 No ${type} directory found, skipping...`);
212217
return [];
213218
}
214219

@@ -309,7 +314,7 @@ export async function loadResources<T>(
309314
}
310315

311316
resources.push({ resourceId, filePath, data });
312-
console.log(` 📦 Loaded ${resourceId}`);
317+
if (!options.quiet) console.log(` 📦 Loaded ${resourceId}`);
313318
}
314319

315320
return resources;

0 commit comments

Comments
 (0)