Commit 301900b
committed
refactor(push): extract reconcileStateKeyForResource — fold two ensure-fns into one generic helper (#34) [skip ci]
* refactor(engine): extract shared slug + folder helpers into slug-utils (#35)
Consolidates 4+ duplicated helpers that had accumulated across the
gitops engine as symptom-fixes piled up. Pure factoring, zero
behavior change at every reachable call site.
Duplications collapsed:
- `slugify` — 4 byte-identical copies (pull.ts, dep-dedup.ts,
audit.ts, setup.ts) → 1 in src/slug-utils.ts
- `extractBaseSlug` — 2 byte-identical copies → 1
- `FOLDER_MAP` — 2 byte-identical copies (pull.ts, resources.ts) → 1
- `UUID_SUFFIX_RE` — open-coded in 3 places → 1 constant
- recanonicalize's inlined precondition-2 check (UUID prefix match)
→ extracted as `isEngineSuffixedSlug`
src/slug-utils.ts is config-free by design (no `./config.ts` import,
no side effects at load) so it's safely importable from any test
without priming process.argv / VAPI_TOKEN. This is the testability
property the prior dep-dedup.ts comment claimed for its local
duplicates but didn't actually enforce.
Regex tightening: shared `UUID_SUFFIX_RE` uses `^(.+)-([0-9a-f]{8})$`
(non-empty base) where the prior pull/dep-dedup copies used
`^(.*)-...` (allowed empty base). Strict improvement — engine-
generated keys always have a non-empty base, and the only input
class affected is the synthetic `-<8hex>` shape which is never
produced by `generateResourceId`. Pinned by a regression test in
tests/slug-utils.test.ts.
Back-compat: dep-dedup.ts re-exports slugify/extractBaseSlug so
existing tests importing them via that path keep working.
Tests: 228/228 pass (208 prior + 20 new slug-utils cases covering
slugify behavior, UUID_SUFFIX_RE boundaries, extractBaseSlug loose
form, isEngineSuffixedSlug strict form).
* refactor(push): extract reconcileStateKeyForResource — fold two 94-line ensure-fns into one generic helper
`ensureToolExists` and `ensureStructuredOutputExists` were structurally
identical 94-line functions differing only in: resource type label,
apply function, state section, remote-list cache, and the per-type
bookkeeping array. Both implemented the same dedup-then-apply flow:
look up existing dashboard/state match by canonical name, adopt with
orphan-deletion guard, mark `touched` for `mergeScoped`, call apply.
Behavioral contract preserved exactly:
- Log strings byte-identical (verified path-by-path against
pre-refactor)
- `autoApplied.add` BEFORE the `if (!uuid) return` early-exit
- `applied[type]++`, `pushToAutoAppliedList`, `touched.add` AFTER
the null check — preserves dry-run / drift-halt semantics
- Orphan-deletion guard scope unchanged: deletes state keys
pointing at the adopted UUID, leaves `duplicateUuids` alone for
`npm run cleanup` to handle
- try/catch boundary identical
- All 228 prior tests pass unchanged, including the integration
test in tests/push-dry-run.test.ts
push.ts shrunk -129 net LOC (two 94-line functions collapsed to
~14-line wrappers). Helper is `tools | structuredOutputs` narrow
today; adding a future type requires a deliberate union widening
+ LABELS map entry, not a config flag.
`vapiEnv` and `formatError` parameters are required (not optional
with placeholder defaults) so a future caller can't accidentally
emit a degraded warning or error message.
Tests: 244/244 pass (228 prior + 16 new — 8 scenarios × 2 resource
types covering happy path, ambiguous match, null applyFn ordering
contract, orphan-deletion guard scope, run-scoped idempotency,
state-hit/dashboard-hit/no-match branches).
Closes the symptom-fix pattern documented in improvements.md #10
(now handled by the generic helper instead of the two hardcoded
functions).1 parent 41c47c8 commit 301900b
6 files changed
Lines changed: 53 additions & 1 deletion
File tree
- .claude
- .cursor/rules
- .gemini
- .github
- .vscode
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
17 | 17 | | |
18 | 18 | | |
19 | 19 | | |
20 | | - | |
| 20 | + | |
21 | 21 | | |
22 | 22 | | |
23 | 23 | | |
| |||
0 commit comments