fix(cli): fail loudly on render job path collisions (#65)#68
Merged
Conversation
buildRenderJobs now throws a clear error when two jobs resolve to the same work dir / output — i.e. a tutorial id that coincides with another tutorial's id+language suffix (e.g. `setup.es` source vs `setup` in es). Pre-existing edge (a benign sequential overwrite before); the new --render-concurrency makes it simultaneous corruption, so detect and reject it up front rather than ship a latent corruption path with the concurrency feature. Tests cover the collision + the legitimate case. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Code review on PR #68: the cross-tutorial id+suffix collision can't occur through the CLI — validateTutorial's SLUG_RE forbids dots in ids and discoverTutorials rejects duplicate ids, so the real protection is the slug rule, not this guard. - Reframe the guard's comment + CHANGELOG: it's defense-in-depth (guards a future relaxed slug rule / a caller that skips validation), not a live corruption fix. - Add a spec test pinning that a dotted id (`setup.es`) is rejected at validation — the actual first line of defense for #65. - Note in the guard's own test that its literals deliberately bypass validation to exercise the backstop directly. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
1 participant
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.
Closes #65. Defense-in-depth before cutting 0.13.0.
Correction from code review
The collision #65 describes cannot occur through the CLI today:
validateTutorialenforcesSLUG_RE(no dots in ids) anddiscoverTutorialsrejects duplicate ids, so a tutorial namedsetup.esis rejected at discovery long beforebuildRenderJobs. The real protection is the slug rule.So this PR is not a live corruption fix — it's a backstop plus a test that pins the actual protection.
What
buildRenderJobsthrows a clear error if two jobs would resolve to the same work dir / output (jobPathKeymirrors the real.forge/<id><suffix>+<id><suffix>.mp4derivation). A backstop for a future relaxed slug rule or a caller that skips validation — where the collision would be simultaneous corruption under--render-concurrency, not a benign overwrite.setup.es) is rejected byvalidateTutorial/SLUG_RE.Tests
spec.test.ts: dotted-id rejection (the production protection).render.test.ts: the backstop throws on validation-bypassing literals; legitimate combos don't. core 183 + cli 10 pass.🤖 Generated with Claude Code