refactor(engine): extract shared slug + folder helpers into slug-utils#33
Closed
dhruva-reddy wants to merge 1 commit into
Closed
refactor(engine): extract shared slug + folder helpers into slug-utils#33dhruva-reddy wants to merge 1 commit into
dhruva-reddy wants to merge 1 commit into
Conversation
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).
4 tasks
This was referenced May 15, 2026
Contributor
Author
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pure factoring PR. Consolidates duplicated helpers that accumulated across the gitops engine as symptom-fixes piled up. Zero behavior change at every reachable call site. Tier 1 of an engine consolidation series — surfaced by the post-mortem on the recanonicalize PR (#32, base of this stack).
What gets deduplicated
Why a separate module instead of "just import from pull.ts"
`src/slug-utils.ts` is config-free by design — no `./config.ts` import, no side effects at load. `config.ts` asserts `argv[2]` / `VAPI_TOKEN` at module load and `process.exit(1)`s under unit tests. Any test that imports a slug helper transitively via `pull.ts` would otherwise have to prime `process.argv` and `process.env.VAPI_TOKEN` (see `tests/recanonicalize.test.ts:7-8`).
The prior `dep-dedup.ts` comment claimed its local duplicates were "intentionally copied for testability" but `new-file-gate.ts` imported the helper from `pull.ts` anyway — so the testability claim didn't actually hold. The new `slug-utils.ts` makes it real: zero imports, importable from any test without ceremony.
Two-form contract: loose vs. strict
The helpers split into two semantically distinct forms that callers were previously mixing:
Regex tightening (intentional)
Shared `UUID_SUFFIX_RE` uses `^(.+)-([0-9a-f]{8})$` (non-empty base). Prior pull/dep-dedup copies used `^(.*)-...` (allowed empty base). Strict improvement:
Files changed
Net: +275 / -85 → +190. New file weight is mostly the test suite (170 lines) — source-code net is +105 / -85 = +20.
Test plan
Code review
In-branch review by code-reviewer subagent. No blocking findings. Four L-level cleanups addressed in-branch:
Stack
This is PR 1 of 2 in the engine-consolidation series. PR 2 stacks on this one and folds `ensureToolExists` / `ensureStructuredOutputExists` (both ~94 lines, structurally identical) into a generic `reconcileStateKeyForResource` helper. Reviewable alone — PR 2 is sequencing, not technical dependency.
Related