feat/slug-budget-24
blockedviewing older commit47e763e · fullpre-PRreviewed 2026-07-16 19:47 UTC1H · 2M · 4L · 2I- Purpose
- Raise the CDK preview branch-slug character budget from 17 to 24 chars so more descriptively-named branches can get full preview infra (Supabase + Vercel + CDK stacks). The 17-char limit was a hard constraint derived from the longest CFE resource name; this branch makes room by introducing a branch-only abbreviation pass so the fixed parts shrink without renaming deployed (non-preview) resources.
- Goal
- Raise MAX_BRANCH_NAME_LEN to 24 with structural enforcement (BranchNameLengthAspect at synth time, updated GHA check), so the budget is future-proof and self-documenting.
- Sub-goals
- SG-1: Branch-only abbreviation pass (BRANCH_TOKEN_ABBREV + BRANCH_EXACT_ABBREV) in naming.ts applied only when branch is set — un-branched envs untouched
- SG-2: BranchNameLengthAspect CDK Aspect fails synth if any literal branched resource name would exceed the 24-char slug budget
- SG-3: Update GHA preview-provision.yml slug-length check from > 17 to > 24
- SG-4: Add audit script audit-resource-name-lengths.mjs for manual budget verification
- SG-5: Update CLAUDE.md and infra/cdk/CLAUDE.md docs to reflect new budget and enforcement mechanism
- What
- 8 files: preview-provision.yml (threshold 17→24), CLAUDE.md + infra/cdk/CLAUDE.md (docs), eslint.config.js (ignore scripts/*.mjs), new audit-resource-name-lengths.mjs, main.ts (adds BranchNameLengthAspect), new branch-name-length.ts Aspect, naming.ts (abbreviation maps + updated boundedBranchSlug threshold).
- Why
- The 17-char budget was derived from a now-outdated analysis of the longest fixed resource part. By abbreviating token-by-token for preview names only, the fixed part stays ≤40 chars (audited via the new script), leaving 24 chars for the slug — a 41% increase that covers virtually all descriptive branch names.
- Areas
- .github/workflows+8−8CLAUDE.md+1−1infra/cdk/CLAUDE.md+5−2infra/cdk/eslint.config.js+1−1infra/cdk/scripts+69−0infra/cdk/src/app+10−1infra/cdk/src/aspects+70−0infra/cdk/src/lib+47−10
- Blast
- 8 files, +211/−23. CDK-only except for docs. No domain logic, no DB schema, no API surface, no frontend. Impact is scoped to preview stack provisioning.
Findings · 11
correctness3
Stale boundedBranchSlug copy in metrics-explore-core.ts uses old ≤17 threshold
apps/platform/src/api/utils/metrics-explore-core.ts:54
This file has its own copy of boundedBranchSlug (explicitly commented 'MUST mirror infra/cdk/src/lib/naming.ts boundedBranchSlug') that still uses `if (slug.length <= 17)` and `slug.slice(0, 11)`. The CDK naming.ts was updated to ≤24 / slice(0,18) by this branch. For any branch slug 18–24 chars long (e.g. the current branch 'feat-slug-budget-24' = 20 chars), CDK deploys the exploration SFN with the verbatim slug, but the runtime ARN resolver still hashes it to a `head11-XXXX` form — the ARN points to a non-existent state machine and every metrics exploration call on that branch returns 503. The accompanying test (metrics-explore-core.test.ts) still asserts ≤17 / 11-char truncation, so CI passes even though the mirror is broken.
Stale log message still says '17-char cap' after threshold raised to 24
.github/workflows/preview-provision.yml
The condition was correctly updated to `> 24` but the warning echo inside the skip branch still reads '...exceeds the 17-char cap'. Misleading in CI logs for anyone debugging a slug skip.
BranchNameLengthAspect slug detection misses names ending with slug (no trailing hyphen)
infra/cdk/src/aspects/branch-name-length.ts:52
Detection guard `name.includes('-{slug}-')` requires hyphens on both sides. A name ending with the slug would be silently skipped. In practice all Batu resource names follow `batu-{slug}-{env}-...` so the slug is always interior — but this is a latent gap if any future naming function emits a slug-terminated name.
conventions4
NAME_LIMITS in BranchNameLengthAspect diverges from audit script NAME_PROPS
infra/cdk/src/aspects/branch-name-length.ts:22
The Aspect comment says 'keep the two in sync' with audit-resource-name-lengths.mjs, but the Aspect omits AWS::Logs::LogGroup (limit 512), AWS::CloudWatch::Alarm (limit 255), and AWS::SecretsManager::Secret (limit 512) — all three present in the audit script. The omission is benign for current names (generous limits) but the claimed sync guarantee is false. Either add the three types or remove the sync claim.
Inconsistent quote style between naming.ts (single) and branch-name-length.ts (double)
infra/cdk/src/aspects/branch-name-length.ts:1
naming.ts and other CDK files use single-quoted strings; branch-name-length.ts uses double-quoted strings throughout. Cosmetic but deviates from the established pattern in the package.
Inline arithmetic comment on boundedBranchSlug restates the obvious
infra/cdk/src/lib/naming.ts:62
Comment '// 18 + 1 + 4 = 23 chars, deterministic' restates arithmetic visible from the template literal. Project convention: add comments only when WHY is non-obvious.
BRANCH_EXACT_ABBREV JSDoc split across two lines inconsistently
infra/cdk/src/lib/naming.ts:42
The JSDoc block uses irregular indentation. Minor formatting inconsistency.
tests2
abbreviateForBranch whole-token invariant has no automated test
infra/cdk/src/lib/naming.ts
The key safety property of abbreviateForBranch is whole-hyphen-bounded token matching ('scheduler' must NOT be abbreviated by 'schedule'). This invariant is documented in comments but has no test. A future typo in BRANCH_TOKEN_ABBREV would silently fail to abbreviate and erode the slug budget without any signal. Suggested cases: partial-token non-match, multi-token abbreviation, exact-rename after token pass.
boundedBranchSlug boundary at 24 chars has no test (old tests assert ≤17)
apps/platform/src/api/utils/__tests__/metrics-explore-core.test.ts
The test suite for metrics-explore-core still asserts the old boundary (≤17 verbatim, 11-char truncation) and must be updated to ≤24 / 18-char truncation as part of fixing the critical mirror bug. Additionally, CDK's own naming.ts has no test suite at all — add at minimum boundary tests for the new threshold.
improvement2
Filter logic asymmetry: Aspect uses .includes(), audit script uses .startsWith()
infra/cdk/scripts/audit-resource-name-lengths.mjs:40
The audit script filters with startsWith('batu-{SLUG}-') while the Aspect uses .includes('-{slug}-'). Both miss no real resources (all Batu names start with 'batu-'), but the two tools audit different populations. Align to one predicate.
BRANCH_EXACT_ABBREV ordering dependency (post-token form required) is implicit
infra/cdk/src/lib/naming.ts:44
Keys in BRANCH_EXACT_ABBREV must match the string AFTER BRANCH_TOKEN_ABBREV has been applied ('metrics-helio-materialize-persist', not 'metrics-helioscope-materialize-persist'). A future maintainer using the pre-substitution form gets a silently no-op entry. The comment mentions 'after token abbreviation' but doesn't illustrate with an example.