feat/slug-budget-24

Stale '17-char cap' log message after budget raised to 24

.github/workflows/preview-provision.yml:762

Line 762 echoes 'Skipping CDK: slug exceeds the 17-char cap.' — the guard condition correctly tests > 24, but this message will confuse anyone reading CI output. One-liner fix.

Prefix-stripping of 'test-' and 'ci-' can produce surprisingly short slugs

infra/cdk/src/lib/naming.ts

BRANCH_TYPE_PREFIX includes 'test' and 'ci', so 'test-utils' → 'utils' (5 chars) and 'ci-cd-pipeline' → 'cd-pipeline'. A developer who names a branch 'test-something' intending it as a literal name (not a type prefix) gets a very short, potentially colliding slug. This is documented in CLAUDE.md, but the 'test' prefix is especially risky since it's common in ad-hoc branch names. No test guards this edge case.

BranchNameLengthAspect property access via camelCase field cast is fragile

infra/cdk/src/aspects/branch-name-length.ts

(node as unknown as Record<string, unknown>)[propertyFieldFor(prop)] reads the CDK L1 TypeScript property. This works for all 10 current entries but is not the canonical CDK API — cfnProperties (keyed by CFN prop name) would be more robust and avoid reliance on the undocumented field-naming convention. Low risk for stable resource types, but worth a caveat comment.

4-hex hash gives only 65536 collision values for long-slug fallback

infra/cdk/src/lib/naming.ts

sha256(slug).hex.slice(0,4) = 16 bits. Two long-named branches with the same 18-char prefix have a non-zero collision chance. A collision causes the second CDK deploy to fail ('resource already exists') — not silent, but disruptive. 6 hex chars (24 bits, 16M values) would fit within the 23-char budget (18+1+6=25 — actually too long; 5 hex chars = 24 chars total, still at budget). 4 is fine for the expected branch volume but worth noting.

github.ref_name interpolated directly into run: shell scripts

.github/workflows/preview-provision.yml

Multiple lines assign BRANCH_NAME="${{ github.ref_name }}" inside run: blocks. GHA substitutes before shell execution, so a branch name containing $(...) or backticks would execute with workflow permissions. Risk is bounded: workflow triggers only on push to feat/**/fix/**/etc. (requires repo write access, no pull_request trigger). Hardening: pass via env: map instead of inline interpolation. Not introduced by this PR but worth flagging.

'Mirrors' comment on NAME_LIMITS is inaccurate — it's an intentional subset

infra/cdk/src/aspects/branch-name-length.ts:21

Comment reads 'Mirrors scripts/audit-resource-name-lengths.mjs — keep the two in sync' but the aspect has 10 entries while the script has 13 (script adds LogGroup/CW-Alarm/SecretsManager, all with limits ≥255 that can never threaten the 24-char budget). The omission is intentional and correct, but 'mirrors' sends future maintainers hunting for a phantom sync bug. Fix: 'Subset of scripts/audit-resource-name-lengths.mjs; LogGroup/CW-Alarm/Secret omitted (limits ≥255). Keep limits of the shared 10 in sync.'

'feature/' prefix triggers the workflow but is absent from the prefix-strip list

.github/workflows/preview-provision.yml:9

The workflow triggers on branches/filter 'feature/**' but the sed strip on line 736 only covers feat|fix|hotfix|...|ci (no 'feature'). naming.ts has the same list. Behavior is internally consistent (neither strips 'feature/'), but the workflow comment says 'keep the prefix list in sync with BRANCH_TYPE_PREFIX in naming.ts' — which makes 'feature' look like an oversight. Either add 'feature' to both strip lists, or add a comment that 'feature/**' is intentionally un-stripped (legacy alias, full name kept).

cdk.out*/ gitignore glob is correct — two older per-path entries now redundant

.gitignore

cdk.out*/ at the root correctly captures all cdk.out-prefixed output dirs. Lines 122 and 124 in .gitignore still have /infra/cdk/cdk.out/* and infra/cdk/cdk.out, which are now redundant but harmless.

No test infrastructure exists in infra/cdk — claimed 'Aspect negative test' is not in the diff

infra/cdk/src/aspects/branch-name-length.ts

PR description claims 'Aspect negative test: flags only an over-budget name, spares compliant ones' but the test file is not in either commit and infra/cdk has no vitest config or test runner. The verification was a manual cdk-synth run. The pure functions (boundedBranchSlug, abbreviateForBranch, BranchNameLengthAspect.visit) have zero automated coverage — any future refactor of the abbreviation maps or prefix-strip regex has no regression guard.

abbreviateForBranch token map has no unit tests — whole-token semantics untested

infra/cdk/src/lib/naming.ts

BRANCH_TOKEN_ABBREV uses split('-').map().join('-') so 'scheduler' is NOT abbreviated (no whole 'schedule' token with an 'r' after), but 'payment-schedule' IS (standalone token). This is subtle and critical for budget correctness. No test documents the expected transformation for key inputs like 'cfe-bills-payment-subscriber-schedule' → 'cfe-bills-payment-sub-sched' or the exact-override 'metrics-helio-materialize-persist' → 'metrics-helio-materialize'.

NAME_LIMITS (aspect) and NAME_PROPS (audit script) maintained separately with no structural sharing

infra/cdk/src/aspects/branch-name-length.ts

Both tables cover the same AWS resource-name constraints but already differ by 3 entries and are maintained independently. A shared JSON or exported TS constant would prevent silent drift. Blocked by the TS vs .mjs boundary, but a JSON file importable by both would work. Low priority given the limit difference is intentional and the generous-limit resources are unlikely to hit the 24-char budget.

BRANCH_EXACT_ABBREV escape hatch has no doc comment explaining when to use it vs token map

infra/cdk/src/lib/naming.ts

Two abbreviation tables exist (BRANCH_TOKEN_ABBREV: per-token; BRANCH_EXACT_ABBREV: post-token full-string override). The exact table is necessary because the 'persist' token has no safe abbreviation and token substitution alone left 'metrics-helio-materialize-persist' over budget. Without a comment, the next engineer won't know whether to add a token entry or an exact entry when a new resource breaches budget. Add: 'Use this when token substitution alone can't get the result under the 40-char fixed-part target and no safe single-token abbreviation exists.'

boundedBranchSlug hash now uses post-strip slug as input, not raw branch — undocumented

infra/cdk/src/lib/naming.ts

Previously the hash was computed from the full branch name (pre-strip); now it uses slug (post-strip). For over-budget branches previously deployed under the old hash the physical name changes (new hash suffix), treating the stack as NEW in CloudFormation. Since over-length branches historically skipped CDK preview, the blast radius is narrow, but a comment on the hash line noting 'input is post-prefix-strip slug' clarifies the intentional change.

propertyFieldFor could use node.cfnProperties instead of camelCase cast

infra/cdk/src/aspects/branch-name-length.ts

CfnResource.cfnProperties returns a {[propName: string]: any} map keyed by CloudFormation property names (e.g. 'FunctionName'), avoiding the need to derive camelCase field names. Switching to cfnProperties[prop] would be more idiomatic CDK and more robust against potential internal renames.

Workflow separator alignment with naming.ts is incidental — should be documented

.github/workflows/preview-provision.yml:736

naming.ts strips BRANCH_TYPE_PREFIX matching [-/] (both dash and slash); workflow uses sed with dash-only because SLUG was already slugified (/ → -). The slash variant in the regex is dead code for the workflow path. A comment noting 'workflow pre-slugifies; slash variant handles direct callers' would make this alignment explicit rather than incidental.

abbreviateForBranch pattern repeated in resourceName and s3BucketName — minor DRY opportunity

infra/cdk/src/lib/naming.ts

Both functions compute `${parts.service}-${suffix}` and call abbreviateForBranch on it conditionally. Extracting to a helper branchBody(parts, suffix) would make adding a third name-builder (e.g. CloudFront distribution) a one-liner and keep the abbreviation conditional in one place.