[codex] fix downstream Bazel package consumption#89
Conversation
Greptile SummaryThis PR bumps
Confidence Score: 3/5Merge blocked by a P1 in the critical new wrapper path; fix is a one-line change before merging. One P1 finding (mkdirSync return-value used as workDir, which is undefined when the dir pre-exists) sits on the single new code path this PR introduces. Two P2 concerns (hardcoded internal CLI path, temp-dir leak on exceptions) lower confidence further. All other files (version bumps, generated docs) are consistent and mechanical. scripts/svelte-package-bazel-wrapper.mjs — the prepareWritableProject function and resolvePackageCli. Important Files Changed
Sequence DiagramsequenceDiagram
participant Bazel as Bazel (js_run_binary)
participant Wrapper as svelte-package-bazel-wrapper.mjs
participant FS as Filesystem (os.tmpdir)
participant CLI as @sveltejs/package CLI
Bazel->>Wrapper: spawn with args [-i scheduling_kit_src, -o dist]
Wrapper->>Wrapper: resolveExistingPath(scheduling_kit_src)
Wrapper->>FS: mkdtempSync → tempRoot
Wrapper->>FS: mkdirSync(tempRoot/work) → workDir
Wrapper->>FS: symlinkSync(scheduling_kit_src → workDir/src)
Wrapper->>FS: symlinkSync(package.json, svelte.config.js, tsconfig.json)
Wrapper->>FS: cpSync(.svelte-kit → workDir/.svelte-kit)
Wrapper->>FS: makeWritable(workDir/.svelte-kit)
Wrapper->>FS: symlinkSync(node_modules → workDir/node_modules)
Wrapper->>CLI: spawnSync(node cli.js -i src -o /abs/dist, cwd=workDir)
CLI-->>Wrapper: exit status
Wrapper->>FS: rmSync(tempRoot, recursive)
Wrapper-->>Bazel: exit(status)
Reviews (1): Last reviewed commit: "fix: make kit Bazel package consumer-saf..." | Re-trigger Greptile |
| const tempRoot = mkdtempSync( | ||
| path.join(os.tmpdir(), "scheduling-kit-package-"), | ||
| ); | ||
| const workDir = mkdirSync(path.join(tempRoot, "work"), { recursive: true }); |
There was a problem hiding this comment.
mkdirSync with { recursive: true } returns string | undefined — it returns undefined when the target directory already exists. Assigning its return value to workDir means that on any retry or unusual condition where tempRoot/work somehow pre-exists, path.join(workDir, "src") on the next line throws TypeError: The "path" argument must be of type string. Received undefined. The conventional fix is to build the path first and pass it to mkdirSync as a side-effect only.
| const workDir = mkdirSync(path.join(tempRoot, "work"), { recursive: true }); | |
| const workDir = path.join(tempRoot, "work"); | |
| mkdirSync(workDir, { recursive: true }); |
| path.join( | ||
| packageRoot, | ||
| "node_modules", | ||
| "@sveltejs", | ||
| "package", | ||
| "src", | ||
| "cli.js", | ||
| ), | ||
| path.join( | ||
| originalCwd, | ||
| "node_modules", | ||
| "@sveltejs", | ||
| "package", | ||
| "src", | ||
| "cli.js", | ||
| ), | ||
| process.env.JS_BINARY__EXECROOT | ||
| ? path.join( | ||
| process.env.JS_BINARY__EXECROOT, | ||
| "node_modules", | ||
| "@sveltejs", | ||
| "package", | ||
| "src", | ||
| "cli.js", | ||
| ) | ||
| : undefined, |
There was a problem hiding this comment.
The internal path
src/cli.js inside @sveltejs/package is not a public API contract. Future releases of that package could restructure to dist/cli.js or similar, silently breaking this lookup with a confusing "Unable to locate" error. Using the package's bin symlink through node_modules/.bin/svelte-package would be more resilient.
| path.join( | |
| packageRoot, | |
| "node_modules", | |
| "@sveltejs", | |
| "package", | |
| "src", | |
| "cli.js", | |
| ), | |
| path.join( | |
| originalCwd, | |
| "node_modules", | |
| "@sveltejs", | |
| "package", | |
| "src", | |
| "cli.js", | |
| ), | |
| process.env.JS_BINARY__EXECROOT | |
| ? path.join( | |
| process.env.JS_BINARY__EXECROOT, | |
| "node_modules", | |
| "@sveltejs", | |
| "package", | |
| "src", | |
| "cli.js", | |
| ) | |
| : undefined, | |
| path.join(packageRoot, "node_modules", ".bin", "svelte-package"), | |
| path.join(originalCwd, "node_modules", ".bin", "svelte-package"), | |
| process.env.JS_BINARY__EXECROOT | |
| ? path.join( | |
| process.env.JS_BINARY__EXECROOT, | |
| "node_modules", | |
| ".bin", | |
| "svelte-package", | |
| ) | |
| : undefined, |
| function prepareWritableProject(inputDir) { | ||
| const tempRoot = mkdtempSync( | ||
| path.join(os.tmpdir(), "scheduling-kit-package-"), | ||
| ); | ||
| const workDir = mkdirSync(path.join(tempRoot, "work"), { recursive: true }); | ||
|
|
||
| linkOrCopy(inputDir, path.join(workDir, "src")); | ||
| for (const file of ["package.json", "svelte.config.js", "tsconfig.json"]) { | ||
| linkOrCopy(resolveExistingPath(file), path.join(workDir, file)); | ||
| } | ||
|
|
||
| const svelteKitDir = resolveExistingPath(".svelte-kit"); | ||
| if (existsSync(svelteKitDir)) { | ||
| linkOrCopy(svelteKitDir, path.join(workDir, ".svelte-kit"), true); | ||
| makeWritable(path.join(workDir, ".svelte-kit")); | ||
| } | ||
|
|
||
| const nodeModulesDir = resolveExistingPath("node_modules"); | ||
| if (existsSync(nodeModulesDir)) { | ||
| linkOrCopy(nodeModulesDir, path.join(workDir, "node_modules")); | ||
| } | ||
|
|
||
| return { tempRoot, workDir }; | ||
| } |
There was a problem hiding this comment.
Temp directory leaked on setup exceptions
If prepareWritableProject throws (e.g., symlinkSync fails because a target path already exists, or resolveExistingPath returns a bad path for package.json), tempRoot is never passed back to the caller and rmSync(tempRoot, ...) on line 174 is never reached. Each failed invocation silently accumulates a scheduling-kit-package-* directory under os.tmpdir(). Wrapping the setup and spawn in a try/finally would close this gap.
Summary
@tummycrypt/scheduling-kitmetadata to0.7.6//:pkgSvelte package action safe when kit is consumed as an external Bzlmod modulesrcRoot Cause
bazel build @tummycrypt_scheduling_kit//:pkgfrom a downstream registry consumer failed because the Svelte package action assumedsrcexisted relative to the main workspace/action cwd. After fixing that, the action also needed a writable.svelte-kitmirror because external module inputs are read-only inside the sandbox.Validation
pnpm docs:generatepnpm check:release-metadatapnpm exec prettier --check AGENTS.md scripts/svelte-package-bazel-wrapper.mjsgit diff --checknpm exec --yes @bazel/bazelisk -- build //:pkg //:typecheck@tummycrypt_scheduling_kit//:pkgand@tummycrypt_scheduling_bridge//:pkgvia the local tinyland registry with--override_module=tummycrypt_scheduling_kit=/private/tmp/scheduling-kit-consumer-bazel