Skip to content

fix: fail loudly if generated format files can't be formatted - #17

Merged
dogmar merged 1 commit into
mainfrom
fix/generate-formats
Jun 14, 2026
Merged

fix: fail loudly if generated format files can't be formatted#17
dogmar merged 1 commit into
mainfrom
fix/generate-formats

Conversation

@dogmar

@dogmar dogmar commented Jun 14, 2026

Copy link
Copy Markdown
Collaborator

Audit fix D (4 of 5).

What the audit found vs. current state

The audit reported generate-formats.ts shelling out to npx oxfmt/npx oxlint (IDE-only no-op stubs) → unformatted files committed, plus a sync-generated.yml missing a types.ts trigger. Both were already resolved by #9 (merged): the script now uses npx vp check --fix, and the workflow runs on every PR (no paths filter). I verified the headline bug no longer reproduces — regenerating off main produces a clean diff.

The remaining, real hazard

The post-#9 format step is still wrapped in a try { … } catch {} that silently swallows any failure:

try {
  execSync(`npx vp check --fix ${FORMATS_DIR}`, { cwd: ROOT, stdio: "pipe" });
} catch {
  // vp check --fix may exit non-zero for unfixable issues; that's OK
}

The raw generator output has unwrapped imports (formatting changes ~860 lines across the 7 files — verified by skipping the step), so the format step is load-bearing. And sync-generated.yml commits whatever the script writes. So if npx vp check --fix ever fails, unformatted files land on main with no signal — the same end state the audit was worried about, just via the new code path.

Fix

Replace it with a targeted, fail-loud format:

execSync(`npx vp fmt ${FORMATS_DIR}`, { cwd: ROOT, stdio: "inherit" });
  • No catch — a formatting failure now aborts generation (and the CI step), so unformatted output can't be silently committed.
  • vp fmt instead of vp check --fix — verified vp fmt alone reproduces the committed output byte-for-byte; the heavier check also ran lint-fix + typecheck (whose failures were being swallowed), and lint/typecheck correctness is already enforced by the ready gate.

Verification

  • Regenerating with the new step → clean diff (correct formatting).
  • Skipping the step → ~860-line unformatted diff (confirms it's load-bearing).
  • vp run ready: lint+typecheck 0/0, all suites green.

🤖 Generated with Claude Code

The format step in generate-formats.ts ran `npx vp check --fix` inside a
try/catch that swallowed every error. The raw generator output has
unwrapped imports, so formatting is load-bearing — and the sync-generated
CI workflow commits whatever the script writes. A swallowed failure would
therefore land unformatted files on main with no signal (the original
audit's hazard, via the post-#9 code path).

Replace it with a targeted `npx vp fmt` and no catch, so a formatting
failure aborts generation (and the CI step). `vp fmt` alone reproduces the
committed output exactly; lint/typecheck correctness is already enforced by
the `ready` gate, so the heavier `vp check --fix` was unnecessary here.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@dogmar
dogmar force-pushed the fix/generate-formats branch from 37f4c59 to c4bbec8 Compare June 14, 2026 19:51
@dogmar
dogmar merged commit c12ecbc into main Jun 14, 2026
6 checks passed
@dogmar
dogmar deleted the fix/generate-formats branch June 14, 2026 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant