Ponytail cleanup: preview-env helper consolidation, duplicated name and issue strings

[simpros]

#280

Why the change

Nine small duplications found by the weekly ponytail audit now live in exactly one home module each, so the next reader meets one duration grammar, one issue-message table, and one name builder.

Special things to note

• health.interval/health.timeout now accept the shared duration grammar (2s, 30m, 2h, 7d) instead of seconds-only; the CLI error text and the adopting-a-repo table say so.
• PreviewVolumeIssue leaves the @sprout/preview-env barrel (its test imports it from ./volumes.ts); nothing else referenced it.
• E2E provisioning tests still skip without SPROUT_E2E_MANAGED=1, so provisioning was verified by suite-green plus harness compile, not a live deploy.

Change outline

Single home for each helper; callers import instead of re-implementing.

  packages/preview-env/src/
  ├── governance.ts      # owns parseDurationMs (sole grammar)
  ├── health.ts          # - local parseDurationMs, + healthIssueMessage
+ ├── env-keys.ts        # + previewEnvIssueMessage (all 5 codes)
  ├── naming.ts          # + previewContainerName
- └── index.ts           # - PreviewVolumeIssue barrel export
  packages/preview-db/src/
+ ├── preview-names.ts   # + previewDbName (sole template)
  └── restricted-role.ts # + companionRoleLimitMessage

Callers now resolve through the shared surface:

  apps/cli/src/yaml.ts
  - local healthIssueMessage + 5-arm env switch
  + healthIssueMessage + previewEnvIssueMessage("preview.env", issue)
  apps/server/src/http/deploy.ts
  - inline `preview.env.${key} requires db.provider ${home}`
  + previewEnvIssueMessage("preview.env", issue)
  apps/server/src/preview/naming.ts
  - local previewContainerName
  + re-export from @sprout/preview-env
  apps/server/src/preview-db/names.ts
  - local previewDbName + twin SERVICE_NAME_RE checks
  + re-export from @sprout/preview-db; one validateName helper
  e2e/harness/docker.ts
  - local previewAppContainerName builder
  + alias of previewContainerName
  e2e/lifecycle.test.ts
  - inline `sprout_${slug}_pr${prId}` × 2
  + previewDbName(slug, prId) × 2

Verification: bun test 1283 pass / 0 fail; bun run typecheck (turbo build + docs:check + docs:typecheck + scripts:typecheck) green, including the dead-exports guard and the assembled-site link gate.

[simpros]

🧊 Thermo-nuclear review (round 1) — APPROVE · 5ebbaff0

• Residual risks
• Unverifiable from here

Full report (click to expand)

VERDICT: APPROVE

Reviewed git diff origin/main...HEAD at HEAD 5ebbaff075b75099e850181677fcf5e8e1fd6535 (matches the brief; nothing moved). This is a genuine consolidation — message builders and name builders move to their canonical packages, duplication at call sites (apps/server/src/http/deploy.ts, apps/server/src/app-deployment/pg-env.ts, apps/cli/src/yaml.ts) is deleted rather than rearranged, and no changed file crosses the 1k threshold. The findings below are follow-ups, not regressions; none meets the presumptive-blocker bar on its own.

Findings (priority order):

1. e2e/harness/docker.ts:8-10 — the harness now exposes two exported names for one function: it re-exports previewContainerName from @sprout/preview-env and also defines export const previewAppContainerName = previewContainerName;. That is a pure identity alias and it contradicts the MR's own de-duplication goal. Remedy: delete line 10 and update the six previewAppContainerName(...) call sites (e2e/mail.test.ts:88, e2e/lifecycle.test.ts:90,143, e2e/sqlite.test.ts:57, e2e/none.test.ts:53, e2e/data-volumes.test.ts:145) to the canonical previewContainerName. Keep one name, not two.

2. apps/server/src/preview/naming.ts:2-5 and apps/server/src/preview-db/names.ts:1-4 — both server modules re-export the package function (export { previewContainerName } / export { previewDbName }) while server call sites keep importing through the local module. This preserves ~15 import sites but creates two "canonical" paths for the same symbol and a standing risk that a future contributor re-implements next to the re-export. If the local module is deliberately the server seam, that is defensible; state it in the comment, otherwise import the package directly at the call sites and drop the re-export. This is the intermediate step of a migration, so not a blocker — but finish it or comment it.

3. apps/cli/src/yaml.ts:609 (with import at :28) — const issue: HealthIssue = resolved.issue; return { ok: false, error: healthIssueMessage(issue) }; is churn: the local variable exists only to keep the HealthIssue type import in use after the message builder moved to the package. Inline it (healthIssueMessage(resolved.issue)) and drop the now-unused HealthIssue import. The annotation adds nothing the narrowed union doesn't already carry.

4. packages/preview-env/src/health.ts:1,50,54 — the consolidation silently widens the health.interval/health.timeout grammar from seconds-only to the full s|m|h|d grammar by swapping the local parser for governance.parseDurationMs (which also newly rejects unsafe-integer magnitudes). It is tested and documented, so this is a deliberate behavior change, not a bug — but it is a semantic change riding inside a "cleanup" MR. Recommend calling it out explicitly in the PR body; 7d polling intervals are legal now and that should be a conscious contract, not a side effect of de-duplication.

Residual risks

• Health-grammar widening: a manifest that previously failed fast on health.interval: 30m now parses. Probe: boot with health.interval: 30m and log the resolved intervalMs the poller uses.
• Deploy error detail text now flows through previewEnvIssueMessage; exact-string assertions or clients keying on the old inline format could drift. Probe: POST /v1/deploy with a provider-mismatched preview.env and diff the detail body.
• parseDurationMs safe-integer clamp now applies to health fields; an extreme value that previously passed silently now errors. Probe: health.timeout: 999999999999999d and confirm a clean invalid_health_timeout, not a throw.
• Re-export shims could mask a future second definition of previewContainerName / previewDbName. Probe: rg "function (previewContainerName|previewDbName)" returns exactly one of each.
• Removal of type PreviewVolumeIssue from the @sprout/preview-env barrel: no in-repo consumer references it from the barrel today, but a downstream deep import would break. Probe: tsc any external workspace importing it.

Unverifiable from here

I could not run bun test or bun run typecheck (no Bun binary in this checkout), so compile/test-green and the runtime effects above are unconfirmed by execution; findings are from static reading of the diff and surrounding code.

---

Automated agent review (opencode) · worktree /home/sim/.herdr/worktrees/sprout/gh-280-ponytail-cleanup-preview-env-hel · subsequent rounds will reply in this thread.