fix: prevent deno.lock creation during extension bundling#491
Merged
Conversation
Add --no-lock to the deno bundle subprocess to prevent Deno from auto-discovering and creating/modifying a lockfile in the user's CWD. Without this flag, every time swamp bundles an extension model at startup, Deno's default lockfile behavior creates or updates a deno.lock in whatever directory the user runs swamp from — polluting their repo with an unexpected file. The --no-lock flag disables lockfile auto-discovery entirely, which is the correct behavior for swamp's use case: extensions are bundled as a build step, not as a user-facing dependency install. npm resolution still works normally — packages are fetched from the registry and inlined into the bundle. The only tradeoff is that without a lockfile, version resolution is not pinned across runs, which is addressed by updating the extension model skill docs to require explicit version pinning on npm imports (e.g., npm:[email protected] instead of npm:lodash-es). Changes: - Add --no-lock flag to deno bundle args in bundle.ts - Add test verifying npm imports resolve without creating deno.lock - Add version pinning guidance to extension model skill docs - Update example imports to use pinned versions Co-authored-by: Blake Irvin <[email protected]> Co-Authored-By: Claude Opus 4.6 <[email protected]>
725a27d to
45e20eb
Compare
There was a problem hiding this comment.
Review: Approved ✅
This PR cleanly fixes the deno.lock pollution issue (#490) with a minimal, well-targeted change.
What I Reviewed
Code Quality
- ✅ TypeScript strict mode compliant - no
anytypes - ✅ Named exports used per CLAUDE.md guidelines
- ✅ AGPLv3 copyright headers present
- ✅ Code passes
deno check,deno lint, anddeno fmt
Test Coverage
- ✅ New test in
bundle_test.tsverifies both bundle success AND lockfile absence - ✅ Proper cleanup with
finallyblock - ✅ Test file location follows convention (next to source)
Domain-Driven Design
- ✅
bundleExtensionremains a clean stateless service - ✅ No DDD concerns - appropriate infrastructure code
Security
- ✅
--no-lockflag is safe - ✅ No injection risks introduced
Documentation
- ✅ SKILL.md updated with version pinning guidance
- ✅ Examples updated to use pinned versions
- ✅ Clear explanation of why
npm:zod@4is excepted (externalized)
No Blocking Issues
The implementation is clean and follows project conventions. The PR description thoroughly explains why --no-lock is the right approach over alternatives like temp directories or lockfile cleanup.
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.
Summary
Fixes #490.
PR #452 introduced
deno bundlefor extension model transpilation. The subprocess inherits the user's CWD and Deno's default lockfile behavior creates/updates adeno.lockin the user's project root — polluting their repo with an unexpected file.Why
--no-lockis the right fixThe
--no-lockflag tells Deno to skip lockfile auto-discovery entirely. This is correct for swamp's bundling use case because:Bundling is a build step, not a dependency install. Swamp bundles extensions at startup as an internal transpilation step. The user didn't ask Deno to manage their dependencies — swamp is doing it behind the scenes. Creating a
deno.lockin their project is a side effect they never opted into.npm resolution still works without a lockfile. Packages are fetched from the registry and fully inlined into the bundle. The lockfile only pins versions across runs — it doesn't enable resolution.
Version pinning is handled at the source level instead. Since there's no lockfile, users should pin explicit versions in their import specifiers (e.g.,
npm:[email protected]). The skill docs and examples are updated to reflect this guidance. This is actually more explicit and portable than relying on a lockfile that lives outside the extension source.Alternative approaches are worse:
cwdon the subprocess to a temp dir would break relative imports in extensionsdeno.lockafter bundling is fragile and races with other processes--lock=<temp-path>still creates a lockfile (just elsewhere) for no benefitChanges
src/domain/models/bundle.ts— Add--no-locktodeno bundleargssrc/domain/models/bundle_test.ts— Add test: bundles npm imports successfully and verifies nodeno.lockis created in the source directory.claude/skills/swamp-extension-model/SKILL.md— Add version pinning rule to Key Rules section.claude/skills/swamp-extension-model/references/examples.md— Pin versions in import examples table and text analyzer exampleTest plan
deno run test src/domain/models/bundle_test.ts(4/4 pass)deno check— type checking passesdeno lint— linting passesdeno fmt— formatting passesdeno run compile— binary compiles successfully🤖 Generated with Claude Code