Migrate all executors from @container to @riwi#986
Open
philiplehmann wants to merge 8 commits intomainfrom
Open
Migrate all executors from @container to @riwi#986philiplehmann wants to merge 8 commits intomainfrom
philiplehmann wants to merge 8 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughRename package scope from Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip CodeRabbit can generate a title for your PR based on the changes.Add |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
libs/node/src/executors/test/executor.ts (1)
55-62:⚠️ Potential issue | 🟡 MinorPre-existing bug: tsconfig fallback logic is inverted.
The condition on line 55 checks if the specified
tsconfigexists, but then proceeds to look for alternatives and potentially overwrite it. This should check if it does not exist.Additionally, the loop iterates over object keys (
for (const config in possibleTsConfigs)) instead of array values (for (const config of possibleTsConfigs)), which would yield indices ("0", "1", ...) rather than filenames.🐛 Proposed fix
- if (existsSync(resolve(root, tsconfig))) { - for (const config in possibleTsConfigs) { + if (!existsSync(resolve(root, tsconfig))) { + for (const config of possibleTsConfigs) { if (existsSync(resolve(root, config))) { tsconfig = config; break; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/node/src/executors/test/executor.ts` around lines 55 - 62, The tsconfig fallback logic is inverted and uses the wrong loop type: change the initial condition to check if the requested tsconfig does NOT exist (use !existsSync(resolve(root, tsconfig))) and then iterate over possibleTsConfigs with a value loop (for (const config of possibleTsConfigs)) instead of a key loop; inside that loop check existsSync(resolve(root, config)), assign tsconfig = config when found, and break.libs/tsc/src/executors/typecheck/executor.ts (1)
26-32:⚠️ Potential issue | 🟡 MinorPre-existing bugs in tsconfig resolution logic.
Two issues in this code block (not introduced by this PR):
Line 26: Condition is inverted — the fallback search should trigger when the default tsconfig does not exist, not when it does.
Line 27:
for (const config in possibleTsConfigs)iterates over array indices (0, 1, 2...) rather than values. Should useofinstead ofin.🐛 Proposed fix
- if (existsSync(resolve(root, tsconfig))) { - for (const config in possibleTsConfigs) { + if (!existsSync(resolve(root, tsconfig))) { + for (const config of possibleTsConfigs) { if (existsSync(resolve(root, config))) { tsconfig = config; break; } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/tsc/src/executors/typecheck/executor.ts` around lines 26 - 32, The tsconfig fallback logic is inverted and iterates incorrectly: change the initial condition so the fallback search runs when the default tsconfig does NOT exist (use !existsSync(resolve(root, tsconfig))), and iterate over the possibleTsConfigs array with "for (const config of possibleTsConfigs)" so you get values not indices; when a matching file is found set tsconfig = config and break (use the existing existsSync(resolve(root, config)) check).
🧹 Nitpick comments (1)
libs/binary/libreoffice/src/lib/libreoffice.ts (1)
66-66: Consider cleaning up the UserInstallation temp directory.A temporary directory is created for
UserInstallationbut unlikeinFileandoutDir, it's not cleaned up in thefinallyblock. This could lead to accumulated temp directories over time.♻️ Suggested fix
const inFile = `${tmpdir()}/${randomUUID()}`; const outDir = `${tmpdir()}/${randomUUID()}`; + const userInstallDir = `${tmpdir()}/${randomUUID()}`; try { await streamInputToWriteable(input, createWriteStream(inFile), { end: true }); ... - `-env:UserInstallation=file://${tmpdir()}/${randomUUID()}`, + `-env:UserInstallation=file://${userInstallDir}`, ... } finally { await cleanup(inFile, 'file'); await cleanup(outDir, 'dir'); + await cleanup(userInstallDir, 'dir'); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/binary/libreoffice/src/lib/libreoffice.ts` at line 66, The temporary LibreOffice UserInstallation path created via `UserInstallation=file://${tmpdir()}/${randomUUID()}` is never removed; assign that path to a variable (e.g., userInstallationDir) when building the args and in the existing `finally` block remove that directory (matching how `inFile` and `outDir` are cleaned) using a safe fs removal (fs.rm / rmSync with recursive:true, force:true or equivalent) to avoid leaving temp folders behind. Ensure you reference the same variable when deleting and handle errors quietly so cleanup doesn't mask primary errors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/bun/package.json`:
- Around line 2-18: Add missing runtime dependencies to the package manifest so
executors can be loaded: update `@riwi/bun`'s package.json to include the required
packages in "dependencies" — at minimum add "@nx/devkit", "@nx/js",
"@riwi/docker", and "@riwi/nx" — ensuring the package.json that currently
defines "executors": "./executors.json" and exports (main/typings) lists these
modules under "dependencies" (not just devDependencies) so the executors
referenced by executors.json can resolve at runtime in consumer workspaces.
In `@tsconfig.base.json`:
- Around line 32-57: The tsconfig paths include three aliases that point to
non-existent entry files: "@riwi/node" -> libs/node/src/index.ts, "@riwi/tsc" ->
libs/tsc/src/index.ts, and "@riwi/http/multipart-form-data" ->
libs/http/multipart-form-data (missing). Fix by either adding the missing entry
modules (create libs/node/src/index.ts and libs/tsc/src/index.ts that re-export
the intended executors, and create libs/http/multipart-form-data/src/index.ts
with the proper exports) or remove/update the three path aliases in the "paths"
map so they point to actual files/directories; reference the alias keys
"@riwi/node", "@riwi/tsc", and "@riwi/http/multipart-form-data" when making the
change.
---
Outside diff comments:
In `@libs/node/src/executors/test/executor.ts`:
- Around line 55-62: The tsconfig fallback logic is inverted and uses the wrong
loop type: change the initial condition to check if the requested tsconfig does
NOT exist (use !existsSync(resolve(root, tsconfig))) and then iterate over
possibleTsConfigs with a value loop (for (const config of possibleTsConfigs))
instead of a key loop; inside that loop check existsSync(resolve(root, config)),
assign tsconfig = config when found, and break.
In `@libs/tsc/src/executors/typecheck/executor.ts`:
- Around line 26-32: The tsconfig fallback logic is inverted and iterates
incorrectly: change the initial condition so the fallback search runs when the
default tsconfig does NOT exist (use !existsSync(resolve(root, tsconfig))), and
iterate over the possibleTsConfigs array with "for (const config of
possibleTsConfigs)" so you get values not indices; when a matching file is found
set tsconfig = config and break (use the existing existsSync(resolve(root,
config)) check).
---
Nitpick comments:
In `@libs/binary/libreoffice/src/lib/libreoffice.ts`:
- Line 66: The temporary LibreOffice UserInstallation path created via
`UserInstallation=file://${tmpdir()}/${randomUUID()}` is never removed; assign
that path to a variable (e.g., userInstallationDir) when building the args and
in the existing `finally` block remove that directory (matching how `inFile` and
`outDir` are cleaned) using a safe fs removal (fs.rm / rmSync with
recursive:true, force:true or equivalent) to avoid leaving temp folders behind.
Ensure you reference the same variable when deleting and handle errors quietly
so cleanup doesn't mask primary errors.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: d16d35a5-c16f-4778-bfd0-361781580fb2
📒 Files selected for processing (117)
apps/mailcatcher/project.jsonapps/mailcatcher/src/test/mailcatcher.spec.tsapps/maildev/project.jsonapps/maildev/src/test/maildev.spec.tsapps/mailhog/project.jsonapps/mailhog/src/test/mailhog.spec.tsapps/nx-cache-server/project.jsonapps/nx-cache-server/src/test/nx-cache-server.spec.tsapps/pdftk/project.jsonapps/pdftk/src/main.tsapps/pdftk/src/test/pdftk.spec.tsapps/poppler/project.jsonapps/poppler/src/main.tsapps/poppler/src/test/poppler.spec.tsapps/puppeteer/project.jsonapps/puppeteer/src/e2e/snapshot.spec.tsapps/puppeteer/src/main.tsapps/puppeteer/src/test/puppeteer.spec.tsapps/tesseract/project.jsonapps/tesseract/src/main.tsapps/tesseract/src/test/tesseract.spec.tsapps/unoserver/project.jsonapps/unoserver/src/main.tsapps/unoserver/src/test/unoserver.spec.tslibs/binary/libreoffice/project.jsonlibs/binary/libreoffice/src/lib/libreoffice.tslibs/binary/pdftk/project.jsonlibs/binary/pdftk/src/lib/compress.tslibs/binary/pdftk/src/lib/data-dump.tslibs/binary/pdftk/src/lib/data-fdf.tslibs/binary/pdftk/src/lib/data-fields.tslibs/binary/pdftk/src/lib/decrypt.tslibs/binary/pdftk/src/lib/encrypt.tslibs/binary/pdftk/src/lib/uncompress.tslibs/binary/poppler/project.jsonlibs/binary/poppler/src/lib/pdf-to.tslibs/binary/puppeteer/project.jsonlibs/binary/tesseract/project.jsonlibs/binary/tesseract/src/lib/image-to-text.tslibs/binary/unoserver/project.jsonlibs/binary/unoserver/src/lib/unoconvert.tslibs/biomejs/package.jsonlibs/biomejs/project.jsonlibs/biomejs/src/executors/lint/executor.tslibs/biomejs/src/index.tslibs/biomejs/tsconfig.lib.jsonlibs/bun/package.jsonlibs/bun/project.jsonlibs/bun/src/executors/build/executor.tslibs/bun/src/executors/serve/executor.tslibs/bun/src/executors/test/executor.tslibs/bun/src/index.tslibs/bun/tsconfig.lib.jsonlibs/docker/package.jsonlibs/docker/project.jsonlibs/helper/package.jsonlibs/helper/project.jsonlibs/helper/src/executors/e2e-install/executor.tslibs/http/body/project.jsonlibs/http/body/src/lib/http-body.spec.tslibs/http/body/src/lib/request-to-buffer.tslibs/http/body/src/lib/request-to-json.tslibs/http/body/src/lib/request-to-multipart-form-data/get-boundary.tslibs/http/body/src/lib/request-to-text.tslibs/http/body/src/lib/validate-request-headers.tslibs/http/error/project.jsonlibs/http/route/project.jsonlibs/http/route/src/lib/route.tslibs/http/route/src/lib/test/object-config-return.del.spec.tslibs/http/route/src/lib/test/object-config-return.get.spec.tslibs/http/route/src/lib/test/object-config-return.patch.spec.tslibs/http/route/src/lib/test/object-config-return.post.spec.tslibs/http/route/src/lib/test/object-config-return.put.spec.tslibs/http/route/src/lib/test/object-config.del.spec.tslibs/http/route/src/lib/test/object-config.get.spec.tslibs/http/route/src/lib/test/object-config.patch.spec.tslibs/http/route/src/lib/test/object-config.post.spec.tslibs/http/route/src/lib/test/object-config.put.spec.tslibs/http/route/src/lib/test/string-config-return.del.spec.tslibs/http/route/src/lib/test/string-config-return.get.spec.tslibs/http/route/src/lib/test/string-config-return.patch.spec.tslibs/http/route/src/lib/test/string-config-return.post.spec.tslibs/http/route/src/lib/test/string-config-return.put.spec.tslibs/http/route/src/lib/test/string-config.del.spec.tslibs/http/route/src/lib/test/string-config.get.spec.tslibs/http/route/src/lib/test/string-config.patch.spec.tslibs/http/route/src/lib/test/string-config.post.spec.tslibs/http/route/src/lib/test/string-config.put.spec.tslibs/http/route/src/lib/test/tree.spec.tslibs/http/server/project.jsonlibs/http/validate/project.jsonlibs/http/validate/src/lib/body.tslibs/http/validate/src/lib/query.tslibs/http/validate/src/lib/validate-combo.spec.tslibs/http/validate/src/lib/validate-single.spec.tslibs/node/package.jsonlibs/node/project.jsonlibs/node/src/executors/test/executor.tslibs/nx/project.jsonlibs/stream/project.jsonlibs/test/bun/project.jsonlibs/test/bun/src/lib/hook.tslibs/test/bun/src/lib/use-test-container.tslibs/test/node/project.jsonlibs/test/node/src/lib/hook.tslibs/test/node/src/lib/use-test-container.tslibs/test/request/project.jsonlibs/test/request/src/lib/test-request.tslibs/test/server/package.jsonlibs/test/server/project.jsonlibs/test/server/src/lib/TestServer.tslibs/tsc/package.jsonlibs/tsc/project.jsonlibs/tsc/src/executors/typecheck/executor.tsnx.jsonpackage.jsontsconfig.base.json
Comment on lines
+2
to
+18
| "name": "@riwi/bun", | ||
| "version": "0.0.1", | ||
|
|
||
| "type": "module", | ||
| "main": "./src/index.js", | ||
| "typings": "./src/index.d.ts", | ||
| "executors": "./executors.json" | ||
| "exports": { | ||
| ".": { | ||
| "types": "./src/index.d.ts", | ||
| "default": "./src/index.js" | ||
| } | ||
| }, | ||
| "executors": "./executors.json", | ||
| "files": [ | ||
| "executors.json", | ||
| "src/**/*", | ||
| "README.md" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
import re
from pathlib import Path
pkg_path = Path("libs/bun/package.json")
pkg = json.loads(pkg_path.read_text())
declared = set()
for field in ("dependencies", "peerDependencies", "optionalDependencies"):
declared.update((pkg.get(field) or {}).keys())
imports = set()
for src in Path("libs/bun/src").rglob("*.ts"):
text = src.read_text()
for match in re.finditer(r"""from\s+['"]([^'"]+)['"]""", text):
spec = match.group(1)
if spec.startswith((".", "/", "node:")):
continue
imports.add("/".join(spec.split("/")[:2]) if spec.startswith("@") else spec.split("/")[0])
print("declared runtime deps:", sorted(declared))
print("external imports:", sorted(imports))
print("missing from runtime deps:", sorted(imports - declared))
PYRepository: philiplehmann/container
Length of output: 267
Declare the executor's runtime dependencies before publishing.
@riwi/bun has npm-facing metadata but executor sources import external packages while the manifest declares no runtime dependencies. The following packages must be declared: @nx/devkit, @nx/js, @riwi/docker, and @riwi/nx. If this package is published unbundled, Nx will fail to load the executors in consumer workspaces due to missing modules.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/bun/package.json` around lines 2 - 18, Add missing runtime dependencies
to the package manifest so executors can be loaded: update `@riwi/bun`'s
package.json to include the required packages in "dependencies" — at minimum add
"@nx/devkit", "@nx/js", "@riwi/docker", and "@riwi/nx" — ensuring the
package.json that currently defines "executors": "./executors.json" and exports
(main/typings) lists these modules under "dependencies" (not just
devDependencies) so the executors referenced by executors.json can resolve at
runtime in consumer workspaces.
Comment on lines
32
to
57
| "paths": { | ||
| "@container/binary/pdftk": ["libs/binary/pdftk/src/index.ts"], | ||
| "@container/binary/poppler": ["libs/binary/poppler/src/index.ts"], | ||
| "@container/binary/puppeteer": ["libs/binary/puppeteer/src/index.ts"], | ||
| "@container/binary/tesseract": ["libs/binary/tesseract/src/index.ts"], | ||
| "@container/binary/unoserver": ["libs/binary/unoserver/src/index.ts"], | ||
| "@container/binary/libreoffice": ["libs/binary/libreoffice/src/index.ts"], | ||
| "@container/biomejs": ["libs/biomejs/src/index.ts"], | ||
| "@container/bun": ["libs/bun/src/index.ts"], | ||
| "@container/docker": ["libs/docker/src/index.ts"], | ||
| "@container/node": ["libs/node/src/index.ts"], | ||
| "@container/nx": ["libs/nx/src/index.ts"], | ||
| "@container/helper": ["libs/helper/src/index.ts"], | ||
| "@container/http/body": ["libs/http/body/src/index.ts"], | ||
| "@container/http/error": ["libs/http/error/src/index.ts"], | ||
| "@container/http/multipart-form-data": ["libs/http/multipart-form-data/src/index.ts"], | ||
| "@container/http/route": ["libs/http/route/src/index.ts"], | ||
| "@container/http/server": ["libs/http/server/src/index.ts"], | ||
| "@container/http/validate": ["libs/http/validate/src/index.ts"], | ||
| "@container/stream": ["libs/stream/src/index.ts"], | ||
| "@container/test/bun": ["libs/test/bun/src/index.ts"], | ||
| "@container/test/node": ["libs/test/node/src/index.ts"], | ||
| "@container/test/request": ["libs/test/request/src/index.ts"], | ||
| "@container/test/server": ["libs/test/server/src/index.ts"], | ||
| "@container/tsc": ["libs/tsc/src/index.ts"] | ||
| "@riwi/binary/pdftk": ["libs/binary/pdftk/src/index.ts"], | ||
| "@riwi/binary/poppler": ["libs/binary/poppler/src/index.ts"], | ||
| "@riwi/binary/puppeteer": ["libs/binary/puppeteer/src/index.ts"], | ||
| "@riwi/binary/tesseract": ["libs/binary/tesseract/src/index.ts"], | ||
| "@riwi/binary/unoserver": ["libs/binary/unoserver/src/index.ts"], | ||
| "@riwi/binary/libreoffice": ["libs/binary/libreoffice/src/index.ts"], | ||
| "@riwi/biomejs": ["libs/biomejs/src/index.ts"], | ||
| "@riwi/bun": ["libs/bun/src/index.ts"], | ||
| "@riwi/docker": ["libs/docker/src/index.ts"], | ||
| "@riwi/node": ["libs/node/src/index.ts"], | ||
| "@riwi/nx": ["libs/nx/src/index.ts"], | ||
| "@riwi/helper": ["libs/helper/src/index.ts"], | ||
| "@riwi/http/body": ["libs/http/body/src/index.ts"], | ||
| "@riwi/http/error": ["libs/http/error/src/index.ts"], | ||
| "@riwi/http/multipart-form-data": ["libs/http/multipart-form-data/src/index.ts"], | ||
| "@riwi/http/route": ["libs/http/route/src/index.ts"], | ||
| "@riwi/http/server": ["libs/http/server/src/index.ts"], | ||
| "@riwi/http/validate": ["libs/http/validate/src/index.ts"], | ||
| "@riwi/stream": ["libs/stream/src/index.ts"], | ||
| "@riwi/test/bun": ["libs/test/bun/src/index.ts"], | ||
| "@riwi/test/node": ["libs/test/node/src/index.ts"], | ||
| "@riwi/test/request": ["libs/test/request/src/index.ts"], | ||
| "@riwi/test/server": ["libs/test/server/src/index.ts"], | ||
| "@riwi/tsc": ["libs/tsc/src/index.ts"] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Verify all path alias targets in tsconfig.base.json exist
paths=(
"libs/binary/pdftk/src/index.ts"
"libs/binary/poppler/src/index.ts"
"libs/binary/puppeteer/src/index.ts"
"libs/binary/tesseract/src/index.ts"
"libs/binary/unoserver/src/index.ts"
"libs/binary/libreoffice/src/index.ts"
"libs/biomejs/src/index.ts"
"libs/bun/src/index.ts"
"libs/docker/src/index.ts"
"libs/node/src/index.ts"
"libs/nx/src/index.ts"
"libs/helper/src/index.ts"
"libs/http/body/src/index.ts"
"libs/http/error/src/index.ts"
"libs/http/multipart-form-data/src/index.ts"
"libs/http/route/src/index.ts"
"libs/http/server/src/index.ts"
"libs/http/validate/src/index.ts"
"libs/stream/src/index.ts"
"libs/test/bun/src/index.ts"
"libs/test/node/src/index.ts"
"libs/test/request/src/index.ts"
"libs/test/server/src/index.ts"
"libs/tsc/src/index.ts"
)
missing=0
for path in "${paths[@]}"; do
if [ ! -f "$path" ]; then
echo "Missing: $path"
missing=$((missing + 1))
fi
done
if [ $missing -eq 0 ]; then
echo "All ${`#paths`[@]} path alias targets exist."
else
echo "$missing path alias target(s) missing."
fiRepository: philiplehmann/container
Length of output: 213
🏁 Script executed:
# Check which directories exist for the missing path aliases
ls -la libs/node/ 2>&1 | head -20
echo "---"
ls -la libs/http/multipart-form-data/ 2>&1 | head -20
echo "---"
ls -la libs/tsc/ 2>&1 | head -20Repository: philiplehmann/container
Length of output: 1492
🏁 Script executed:
# Also check the overall structure to understand the pattern
find libs -maxdepth 2 -type d | grep -E "(node|tsc|multipart)" | sortRepository: philiplehmann/container
Length of output: 127
🏁 Script executed:
# Check if the missing directories even exist
test -d "libs/node" && echo "libs/node exists" || echo "libs/node does NOT exist"
test -d "libs/http/multipart-form-data" && echo "libs/http/multipart-form-data exists" || echo "libs/http/multipart-form-data does NOT exist"
test -d "libs/tsc" && echo "libs/tsc exists" || echo "libs/tsc does NOT exist"Repository: philiplehmann/container
Length of output: 144
🏁 Script executed:
# Check if the index.ts files exist in the src directories that do exist
test -f "libs/node/src/index.ts" && echo "libs/node/src/index.ts EXISTS" || echo "libs/node/src/index.ts MISSING"
test -f "libs/tsc/src/index.ts" && echo "libs/tsc/src/index.ts EXISTS" || echo "libs/tsc/src/index.ts MISSING"
# List what's actually in those src directories
echo "=== libs/node/src/ ==="
ls -la libs/node/src/ 2>&1
echo "=== libs/tsc/src/ ==="
ls -la libs/tsc/src/ 2>&1
echo "=== libs/http/ ==="
ls -la libs/http/ 2>&1Repository: philiplehmann/container
Length of output: 914
🏁 Script executed:
# Check all http submodules to see what exists
find libs/http -maxdepth 2 -type f -name "index.ts" | sortRepository: philiplehmann/container
Length of output: 49
Path alias configuration contains 3 broken references that will cause import failures.
Three path aliases in tsconfig.base.json reference non-existent files or directories:
@riwi/node:libs/node/src/index.tsdoes not exist (libs/node/src/ only contains anexecutors/subdirectory)@riwi/tsc:libs/tsc/src/index.tsdoes not exist (libs/tsc/src/ only contains anexecutors/subdirectory)@riwi/http/multipart-form-data:libs/http/multipart-form-data/does not exist (libs/http/ contains only: body, error, route, server, validate)
Either these missing entry point files need to be created with proper exports, or the path aliases should be removed from the configuration to prevent import resolution failures at runtime.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@tsconfig.base.json` around lines 32 - 57, The tsconfig paths include three
aliases that point to non-existent entry files: "@riwi/node" ->
libs/node/src/index.ts, "@riwi/tsc" -> libs/tsc/src/index.ts, and
"@riwi/http/multipart-form-data" -> libs/http/multipart-form-data (missing). Fix
by either adding the missing entry modules (create libs/node/src/index.ts and
libs/tsc/src/index.ts that re-export the intended executors, and create
libs/http/multipart-form-data/src/index.ts with the proper exports) or
remove/update the three path aliases in the "paths" map so they point to actual
files/directories; reference the alias keys "@riwi/node", "@riwi/tsc", and
"@riwi/http/multipart-form-data" when making the change.
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.npmrc:
- Around line 1-2: The CI is missing the NPM_TOKEN environment variable
referenced by .npmrc, causing authenticated installs of `@riwi` packages to fail;
update the workflow that runs the bun install steps to inject NPM_TOKEN from
repo secrets (e.g., use secrets.NPM_TOKEN) at the job or step level so that
.npmrc can resolve ${NPM_TOKEN} during installs, ensuring the bun install steps
that pull `@riwi/`* packages run with authentication.
In `@libs/tsc/src/executors/typecheck/executor.ts`:
- Around line 26-27: The current check using existsSync(resolve(root, tsconfig))
silently falls back to possibleTsConfigs even when the caller passed an explicit
--tsconfig, causing wrong-project typechecks; change the branch in executor.ts
so that if the resolved tsconfig path does not exist and the tsconfig was
explicitly provided (i.e., not the default value set earlier), the executor
throws/returns a clear error instead of iterating possibleTsConfigs, otherwise
(when no explicit tsconfig was provided) continue to auto-discover by looping
over possibleTsConfigs; reference the variables/functions: existsSync, resolve,
root, tsconfig, and possibleTsConfigs to locate and implement this logic.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a7d7e6ce-83a5-400f-97af-3d320cb70362
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (4)
.npmrclibs/tsc/src/executors/typecheck/executor.tsnx.jsonpackage.json
🚧 Files skipped from review as they are similar to previous changes (1)
- nx.json
Comment on lines
+1
to
+2
| @riwi:registry=https://registry.npmjs.org/ | ||
| //registry.npmjs.org/:_authToken=${NPM_TOKEN} |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "Checking workflow files for bun/npm install steps and NPM_TOKEN wiring..."
fd -p '.github/workflows' -t f | while read -r f; do
echo
echo "=== $f ==="
rg -n -C2 'bun install|npm install|pnpm install|NPM_TOKEN|NODE_AUTH_TOKEN|secrets\.' "$f" || true
doneRepository: philiplehmann/container
Length of output: 140
🏁 Script executed:
find .github/workflows -type f 2>/dev/null | head -20Repository: philiplehmann/container
Length of output: 136
🏁 Script executed:
if [ -f .github/workflows/ci.yaml ]; then
echo "=== ci.yaml found ==="
cat -n .github/workflows/ci.yaml
elif [ -f .github/workflows/ci.yml ]; then
echo "=== ci.yml found ==="
cat -n .github/workflows/ci.yml
else
echo "=== Checking what exists in .github/workflows ==="
ls -la .github/workflows/ 2>/dev/null || echo "Directory does not exist"
fiRepository: philiplehmann/container
Length of output: 9115
🏁 Script executed:
# Search for NPM_TOKEN or NODE_AUTH_TOKEN across the entire codebase
rg -i 'NPM_TOKEN|NODE_AUTH_TOKEN' --type yaml --type ymlRepository: philiplehmann/container
Length of output: 94
🏁 Script executed:
# Check package.json for `@riwi` dependencies
rg '@riwi' package.jsonRepository: philiplehmann/container
Length of output: 92
🏁 Script executed:
# Also check if there are any workspaces or monorepo structures that might use `@riwi`
find . -name "package.json" -type f | head -10 | xargs rg -l '@riwi' 2>/dev/null || echo "No `@riwi` dependencies found"Repository: philiplehmann/container
Length of output: 268
🏁 Script executed:
# Check what `@riwi` dependencies are in the workspace
rg '"@riwi/' --type json $(find . -name "package.json" -type f | head -20)Repository: philiplehmann/container
Length of output: 474
🏁 Script executed:
# Check if .npmrc already existed in the repository before this change
git show HEAD:.npmrc 2>/dev/null || echo ".npmrc did not exist before this change"Repository: philiplehmann/container
Length of output: 155
Wire NPM_TOKEN into CI if @riwi packages will be installed from npm.
The .npmrc file configures authentication via ${NPM_TOKEN} for the @riwi registry, but this environment variable is never set in the CI workflow. All bun install steps (lines 50, 120, 164, 210 in .github/workflows/ci.yaml) run without NPM_TOKEN in their environment. For a monorepo, internal workspace packages are typically resolved locally, so this may not cause immediate failures. However, if any @riwi/* packages are published to npm and installed as external dependencies, authentication will fail during install. To ensure robustness, add NPM_TOKEN from secrets at the job or step level that runs installs.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.npmrc around lines 1 - 2, The CI is missing the NPM_TOKEN environment
variable referenced by .npmrc, causing authenticated installs of `@riwi` packages
to fail; update the workflow that runs the bun install steps to inject NPM_TOKEN
from repo secrets (e.g., use secrets.NPM_TOKEN) at the job or step level so that
.npmrc can resolve ${NPM_TOKEN} during installs, ensuring the bun install steps
that pull `@riwi/`* packages run with authentication.
Comment on lines
+26
to
+27
| if (!existsSync(resolve(root, tsconfig))) { | ||
| for (const config of possibleTsConfigs) { |
There was a problem hiding this comment.
Don't silently substitute a different tsconfig for an invalid explicit path.
Because Line 21 already defaults tsconfig, this branch can't tell whether the missing file is the implicit default or a caller typo. Passing something like --tsconfig=tsconfig.lbi.json will now fall back to another config and can make the executor succeed against the wrong project. Please only auto-discover when no tsconfig was provided; otherwise fail fast.
Suggested fix
-const tscTypecheckExecutor: Executor<TscTypecheckExecutorSchema> = async (
- { tsconfig = 'tsconfig.spec.json' },
- context,
-) => {
+const tscTypecheckExecutor: Executor<TscTypecheckExecutorSchema> = async (
+ options,
+ context,
+) => {
+ let tsconfig = options.tsconfig ?? 'tsconfig.spec.json';
const root = projectRoot(context);
if (!existsSync(resolve(root, tsconfig))) {
+ if (options.tsconfig) {
+ console.error(`Cannot find tsconfig: ${resolve(root, tsconfig)}`);
+ return { success: false };
+ }
for (const config of possibleTsConfigs) {
if (existsSync(resolve(root, config))) {
tsconfig = config;
break;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/tsc/src/executors/typecheck/executor.ts` around lines 26 - 27, The
current check using existsSync(resolve(root, tsconfig)) silently falls back to
possibleTsConfigs even when the caller passed an explicit --tsconfig, causing
wrong-project typechecks; change the branch in executor.ts so that if the
resolved tsconfig path does not exist and the tsconfig was explicitly provided
(i.e., not the default value set earlier), the executor throws/returns a clear
error instead of iterating possibleTsConfigs, otherwise (when no explicit
tsconfig was provided) continue to auto-discover by looping over
possibleTsConfigs; reference the variables/functions: existsSync, resolve, root,
tsconfig, and possibleTsConfigs to locate and implement this logic.
There was a problem hiding this comment.
Actionable comments posted: 6
♻️ Duplicate comments (3)
libs/bun/package.json (1)
14-19:⚠️ Potential issue | 🟠 MajorRuntime dependency declarations for executors are still missing.
This looks like the same unresolved issue from the earlier review: published executors can fail to load if required modules are not declared in
dependencies.#!/bin/bash set -euo pipefail python - <<'PY' import json, re from pathlib import Path pkg = json.loads(Path("libs/bun/package.json").read_text()) declared = set() for field in ("dependencies", "peerDependencies", "optionalDependencies"): declared.update((pkg.get(field) or {}).keys()) imports = set() for src in Path("libs/bun/src").rglob("*.ts"): text = src.read_text() for m in re.finditer(r"""from\s+['"]([^'"]+)['"]""", text): spec = m.group(1) if spec.startswith((".", "/", "node:")): continue imports.add("/".join(spec.split("/")[:2]) if spec.startswith("@") else spec.split("/")[0]) print("declared:", sorted(declared)) print("external imports:", sorted(imports)) print("missing declarations:", sorted(imports - declared)) PY🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/bun/package.json` around lines 14 - 19, The package.json for libs/bun is missing runtime dependency declarations for modules used by the published executors; update the "dependencies" (or "peerDependencies"/"optionalDependencies" as appropriate) in package.json to include every external package imported from files under src (the external import list the review script prints), ensuring the executors referenced by "executors": "./executors.json" can load at runtime; verify the names match the import specifiers (use top-level package names or scoped names like `@scope/pkg`) and add any missing entries found by the provided import scan so imports in libs/bun/src/**/*.ts are declared.libs/tsc/src/executors/typecheck/executor.ts (1)
21-33:⚠️ Potential issue | 🟠 MajorDo not auto-fallback when
tsconfigwas explicitly provided.If a caller passes an invalid
--tsconfig, this still silently picks another config instead of failing fast.Suggested fix
-const tscTypecheckExecutor: Executor<TscTypecheckExecutorSchema> = async ( - { tsconfig = 'tsconfig.lib.json' }, - context, -) => { +const tscTypecheckExecutor: Executor<TscTypecheckExecutorSchema> = async ( + options, + context, +) => { + const explicitTsconfig = options.tsconfig; + let tsconfig = explicitTsconfig ?? 'tsconfig.lib.json'; const root = projectRoot(context); if (!existsSync(resolve(root, tsconfig))) { + if (explicitTsconfig) { + console.error(`Cannot find tsconfig: ${resolve(root, tsconfig)}`); + return { success: false }; + } for (const config of possibleTsConfigs) { if (existsSync(resolve(root, config))) { tsconfig = config; break; }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/tsc/src/executors/typecheck/executor.ts` around lines 21 - 33, The current executor uses a default value for the tsconfig parameter which prevents detecting whether the caller explicitly provided a --tsconfig; as a result the code silently falls back to other configs. Remove the inline default ({ tsconfig = 'tsconfig.lib.json' }) and instead treat tsconfig === undefined as "not provided"; only apply the possibleTsConfigs fallback when tsconfig is undefined, otherwise validate the explicitly provided tsconfig (using projectRoot, resolve, existsSync) and throw or return an error if it doesn't exist so we fail fast.tsconfig.base.json (1)
41-47:⚠️ Potential issue | 🟠 MajorSome renamed path aliases may still point to missing targets.
This appears to be the same unresolved alias-target issue previously flagged (now under
@riwi/*). Please re-verify target file existence before merge.#!/bin/bash set -euo pipefail python - <<'PY' import json from pathlib import Path cfg = json.loads(Path("tsconfig.base.json").read_text()) paths = cfg["compilerOptions"]["paths"] missing = [] for alias, targets in paths.items(): for t in targets: p = Path(t) if not p.exists(): missing.append((alias, t)) print("missing targets:", len(missing)) for alias, t in missing: print(f"{alias} -> {t}") PYAlso applies to: 55-55
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tsconfig.base.json` around lines 41 - 47, The tsconfig path aliases (e.g., "@riwi/node", "@riwi/nx", "@riwi/helper", "@riwi/http/body", "@riwi/http/error", "@riwi/http/multipart-form-data", "@riwi/http/route") point to target files that may not exist; verify each target path listed in compilerOptions.paths exists and either correct the target path to the real index.ts, add the missing file(s), or remove the stale alias; ensure aliases map to existing files (or update to new entry points) and re-run the provided verification snippet to confirm no missing targets remain before merging.
🧹 Nitpick comments (1)
libs/biomejs/CHANGELOG.md (1)
9-23: Consider collapsing repeated dependency bumps into grouped bullets.Multiple patch-step bullets for the same package make this hard to scan. Grouping by package with final version (and optionally a range) would keep the changelog clearer for users.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@libs/biomejs/CHANGELOG.md` around lines 9 - 23, The CHANGELOG contains many repeated bullets for the same packages (e.g., multiple "puppeteer-core" entries, multiple "vite" and "nx monorepo" entries); collapse these into grouped bullets per package showing the final bumped version and optionally the range (e.g., "puppeteer-core: v24.32.0 → v24.38.0 (patch bumps)") to improve readability—edit the CHANGELOG.md replacing the series of individual dependency lines with one grouped line per package (puppeteer-core, vite, nx monorepo) that lists the final version and a concise note about intermediate bumps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@libs/biomejs/CHANGELOG.md`:
- Around line 1-27: Add a "### ⚠️ Breaking Changes" subsection under the
existing "## 0.0.2 (2026-03-14)" header that documents the package scope rename
from `@container` → `@riwi`, listing explicit upgrade instructions mapping old
package/import paths to new ones (for example: import from "@container/xyz" →
"@riwi/xyz"), note any config or build changes consumers must update (tsconfig
paths, package.json dependencies, CI caching), and include a short migration
example and a one-line note advising consumers to run a search-and-replace for
occurrences of "@container" in their codebase.
In `@libs/docker/project.json`:
- Around line 18-32: The build target in project.json sets outputPath
"dist/libs/docker" but tsconfig.lib.json's outDir currently points to
../../dist/out-tsc (producing dist/out-tsc) and package.json entry points still
reference ./src/*; update tsconfig.lib.json by setting compilerOptions.outDir to
../../dist/libs/docker (so the compiled artifacts land under dist/libs/docker
matching the "build" target) and update libs/docker/package.json entry points
(main, typings, and any exports) to reference the built root files (e.g.,
./index.js and ./index.d.ts instead of ./src/index.js and ./src/index.d.ts) so
consumers use the packaged output.
In `@libs/helper/package.json`:
- Around line 8-19: The package.json exports point to ./src/index.js and
./src/index.d.ts but there is no build target wired to produce those artifacts;
add a "build" target to the helper project's project.json using the `@nx/js`:tsc
executor (mirroring libs/node's pattern), configure its options to compile src
into the expected output (emit JS + d.ts into the path referenced by
package.json or change exports to point to the actual dist output), and ensure
the build target's outputs and files list include the generated files so
releases publish them; specifically add a "build" target entry named "build"
with executor "@nx/js:tsc" and appropriate options and outputs so the exports
for "." (types and default) resolve to built artifacts.
In `@libs/nx/project.json`:
- Around line 25-27: Remove the "generatePackageJson" setting from the project
config so the manually maintained libs/nx/package.json (included via "assets")
is preserved; locate the property "generatePackageJson": true in the project
JSON and delete it (or set it to false) to prevent Nx from overwriting custom
exports like "./package.json" and the "type": "module" entry.
In `@libs/tsc/package.json`:
- Around line 15-20: The package.json exposes executors.json but does not
declare runtime dependencies used by those executors, so add the required
packages (e.g., `@nx/devkit`, `@riwi/docker`, `@riwi/nx`) to the "dependencies"
section of package.json (not devDependencies) so executor resolution works after
publish; update the dependencies field in package.json to include these modules
with appropriate version ranges and ensure executors.json remains listed in
"files".
In `@nx.json`:
- Around line 61-64: CI workflows still call bun while nx.json's "cli":
{"packageManager":"npm"} and "sharedGlobals" only tracks
"{workspaceRoot}/bun.lock"; update CI to use npm (replace bun install with npm
ci and bun nx/... with npm exec nx or npx nx) and adjust nx config: add
"{workspaceRoot}/package-lock.json" to "sharedGlobals" and remove or stop
relying on "{workspaceRoot}/bun.lock" so Nx cache inputs match the declared
package manager.
---
Duplicate comments:
In `@libs/bun/package.json`:
- Around line 14-19: The package.json for libs/bun is missing runtime dependency
declarations for modules used by the published executors; update the
"dependencies" (or "peerDependencies"/"optionalDependencies" as appropriate) in
package.json to include every external package imported from files under src
(the external import list the review script prints), ensuring the executors
referenced by "executors": "./executors.json" can load at runtime; verify the
names match the import specifiers (use top-level package names or scoped names
like `@scope/pkg`) and add any missing entries found by the provided import scan
so imports in libs/bun/src/**/*.ts are declared.
In `@libs/tsc/src/executors/typecheck/executor.ts`:
- Around line 21-33: The current executor uses a default value for the tsconfig
parameter which prevents detecting whether the caller explicitly provided a
--tsconfig; as a result the code silently falls back to other configs. Remove
the inline default ({ tsconfig = 'tsconfig.lib.json' }) and instead treat
tsconfig === undefined as "not provided"; only apply the possibleTsConfigs
fallback when tsconfig is undefined, otherwise validate the explicitly provided
tsconfig (using projectRoot, resolve, existsSync) and throw or return an error
if it doesn't exist so we fail fast.
In `@tsconfig.base.json`:
- Around line 41-47: The tsconfig path aliases (e.g., "@riwi/node", "@riwi/nx",
"@riwi/helper", "@riwi/http/body", "@riwi/http/error",
"@riwi/http/multipart-form-data", "@riwi/http/route") point to target files that
may not exist; verify each target path listed in compilerOptions.paths exists
and either correct the target path to the real index.ts, add the missing
file(s), or remove the stale alias; ensure aliases map to existing files (or
update to new entry points) and re-run the provided verification snippet to
confirm no missing targets remain before merging.
---
Nitpick comments:
In `@libs/biomejs/CHANGELOG.md`:
- Around line 9-23: The CHANGELOG contains many repeated bullets for the same
packages (e.g., multiple "puppeteer-core" entries, multiple "vite" and "nx
monorepo" entries); collapse these into grouped bullets per package showing the
final bumped version and optionally the range (e.g., "puppeteer-core: v24.32.0 →
v24.38.0 (patch bumps)") to improve readability—edit the CHANGELOG.md replacing
the series of individual dependency lines with one grouped line per package
(puppeteer-core, vite, nx monorepo) that lists the final version and a concise
note about intermediate bumps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 8eafb556-da5b-4748-b35e-2755be5843a4
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (30)
CHANGELOG.mdlibs/biomejs/CHANGELOG.mdlibs/biomejs/package.jsonlibs/biomejs/project.jsonlibs/biomejs/tsconfig.lib.jsonlibs/bun/CHANGELOG.mdlibs/bun/package.jsonlibs/bun/project.jsonlibs/bun/tsconfig.lib.jsonlibs/docker/CHANGELOG.mdlibs/docker/package.jsonlibs/docker/project.jsonlibs/docker/tsconfig.lib.jsonlibs/helper/package.jsonlibs/helper/tsconfig.lib.jsonlibs/node/CHANGELOG.mdlibs/node/package.jsonlibs/node/project.jsonlibs/nx/CHANGELOG.mdlibs/nx/package.jsonlibs/nx/project.jsonlibs/nx/tsconfig.lib.jsonlibs/tsc/CHANGELOG.mdlibs/tsc/package.jsonlibs/tsc/project.jsonlibs/tsc/src/executors/typecheck/executor.tslibs/tsc/tsconfig.lib.jsonnx.jsonpackage.jsontsconfig.base.json
✅ Files skipped from review due to trivial changes (5)
- libs/nx/CHANGELOG.md
- libs/docker/CHANGELOG.md
- libs/node/CHANGELOG.md
- CHANGELOG.md
- libs/bun/CHANGELOG.md
🚧 Files skipped from review as they are similar to previous changes (5)
- libs/node/package.json
- libs/biomejs/project.json
- libs/docker/package.json
- package.json
- libs/biomejs/package.json
Comment on lines
+1
to
+27
| ## 0.0.2 (2026-03-14) | ||
|
|
||
| ### 🚀 Features | ||
|
|
||
| - **nx-cloud:** setup nx cloud workspace ([5fc1448](https://github.com/philiplehmann/container/commit/5fc1448)) | ||
|
|
||
| ### 🩹 Fixes | ||
|
|
||
| - **deps:** update dependency puppeteer-core to v24.38.0 ([000dabf](https://github.com/philiplehmann/container/commit/000dabf)) | ||
| - **deps:** update dependency puppeteer-core to v24.37.5 ([63f1398](https://github.com/philiplehmann/container/commit/63f1398)) | ||
| - **deps:** update dependency puppeteer-core to v24.37.3 ([43cc319](https://github.com/philiplehmann/container/commit/43cc319)) | ||
| - **deps:** update dependency puppeteer-core to v24.37.2 ([1ca029a](https://github.com/philiplehmann/container/commit/1ca029a)) | ||
| - **deps:** update dependency puppeteer-core to v24.36.1 ([a798e94](https://github.com/philiplehmann/container/commit/a798e94)) | ||
| - **deps:** update dependency puppeteer-core to v24.36.0 ([33c08f6](https://github.com/philiplehmann/container/commit/33c08f6)) | ||
| - **deps:** update dependency puppeteer-core to v24.35.0 ([4ab5d45](https://github.com/philiplehmann/container/commit/4ab5d45)) | ||
| - **deps:** update dependency puppeteer-core to v24.34.0 ([b871cc7](https://github.com/philiplehmann/container/commit/b871cc7)) | ||
| - **deps:** update dependency puppeteer-core to v24.33.0 ([06fe28e](https://github.com/philiplehmann/container/commit/06fe28e)) | ||
| - **deps:** update dependency puppeteer-core to v24.32.1 ([27cee42](https://github.com/philiplehmann/container/commit/27cee42)) | ||
| - **deps:** update dependency puppeteer-core to v24.32.0 ([8204152](https://github.com/philiplehmann/container/commit/8204152)) | ||
| - **deps:** update dependency vite to v7.2.6 ([012dc0c](https://github.com/philiplehmann/container/commit/012dc0c)) | ||
| - **deps:** update nx monorepo to v22.1.3 ([50d86c7](https://github.com/philiplehmann/container/commit/50d86c7)) | ||
| - **deps:** update dependency vite to v7.2.4 ([f077c3a](https://github.com/philiplehmann/container/commit/f077c3a)) | ||
| - **deps:** update nx monorepo to v22.1.2 ([3cf0fa4](https://github.com/philiplehmann/container/commit/3cf0fa4)) | ||
|
|
||
| ### ❤️ Thank You | ||
|
|
||
| - Philip Lehmann No newline at end of file |
There was a problem hiding this comment.
Document the @container → @riwi rename as a breaking change.
This release entry omits the migration impact of the package scope rename. Please add a Breaking Changes section with explicit upgrade instructions (old package/import path → new package/import path) so consumers don’t break silently during upgrade.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/biomejs/CHANGELOG.md` around lines 1 - 27, Add a "### ⚠️ Breaking
Changes" subsection under the existing "## 0.0.2 (2026-03-14)" header that
documents the package scope rename from `@container` → `@riwi`, listing explicit
upgrade instructions mapping old package/import paths to new ones (for example:
import from "@container/xyz" → "@riwi/xyz"), note any config or build changes
consumers must update (tsconfig paths, package.json dependencies, CI caching),
and include a short migration example and a one-line note advising consumers to
run a search-and-replace for occurrences of "@container" in their codebase.
Comment on lines
+18
to
+32
| "build": { | ||
| "executor": "@nx/js:tsc", | ||
| "dependsOn": ["^build"], | ||
| "outputs": ["{options.outputPath}"], | ||
| "options": { | ||
| "outputPath": "dist/libs/docker", | ||
| "main": "libs/docker/src/index.ts", | ||
| "tsConfig": "libs/docker/tsconfig.lib.json", | ||
| "assets": [ | ||
| "libs/docker/package.json", | ||
| "libs/docker/executors.json", | ||
| "libs/docker/src/executors/**/*.json", | ||
| "libs/docker/README.md" | ||
| ] | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== build outputPath =="
jq -r '.targets.build.options.outputPath' libs/docker/project.json
echo "== tsconfig outDir =="
jq -r '.compilerOptions.outDir' libs/docker/tsconfig.lib.json
echo "== package entry points =="
jq -r '.main, .typings, .exports["."].default, .exports["."].types' libs/docker/package.json
echo
echo "Expected: all paths should align to dist/libs/docker/* for built output consumption."Repository: philiplehmann/container
Length of output: 324
Build target output contract is misaligned with library packaging configuration.
The build target defines outputPath as dist/libs/docker, but libs/docker/tsconfig.lib.json compiles to ../../dist/out-tsc (resulting in dist/out-tsc, not dist/libs/docker), and libs/docker/package.json entry points (main, typings, exports) reference ./src/* paths instead of the built output. This breaks the consumption/publishing contract for the built artifacts.
Align tsconfig.lib.json outDir to ../../dist/libs/docker and update package.json entry points from ./src/index.js to ./index.js and ./src/index.d.ts to ./index.d.ts.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/docker/project.json` around lines 18 - 32, The build target in
project.json sets outputPath "dist/libs/docker" but tsconfig.lib.json's outDir
currently points to ../../dist/out-tsc (producing dist/out-tsc) and package.json
entry points still reference ./src/*; update tsconfig.lib.json by setting
compilerOptions.outDir to ../../dist/libs/docker (so the compiled artifacts land
under dist/libs/docker matching the "build" target) and update
libs/docker/package.json entry points (main, typings, and any exports) to
reference the built root files (e.g., ./index.js and ./index.d.ts instead of
./src/index.js and ./src/index.d.ts) so consumers use the packaged output.
Comment on lines
+8
to
+19
| "exports": { | ||
| ".": { | ||
| "types": "./src/index.d.ts", | ||
| "default": "./src/index.js" | ||
| }, | ||
| "./package.json": "./package.json" | ||
| }, | ||
| "executors": "./executors.json", | ||
| "files": [ | ||
| "executors.json", | ||
| "src/**/*", | ||
| "README.md" |
There was a problem hiding this comment.
These exports point at artifacts helper does not currently build.
Lines 10-11 and Lines 16-18 now publish ./src/index.js / ./src/index.d.ts, but libs/helper/project.json:1-19 still has no build target. Unlike libs/node/project.json:18-33, nothing here emits those files during release, so the published package can resolve to missing entrypoints unless those artifacts are committed separately.
Run this to confirm the missing build wiring and advertised files:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json
from pathlib import Path
root = Path(".")
pkg = json.loads((root / "libs/helper/package.json").read_text())
proj = json.loads((root / "libs/helper/project.json").read_text())
print("helper build target present:", "build" in proj.get("targets", {}))
print("exports['.']:", pkg.get("exports", {}).get("."))
for rel in (
"libs/helper/src/index.js",
"libs/helper/src/index.d.ts",
"libs/helper/src/index.ts",
):
print(f"{rel}: {'exists' if (root / rel).exists() else 'missing'}")
PYExpected: helper build target present: False; if src/index.js / src/index.d.ts are also missing, this package needs the same @nx/js:tsc build target pattern that libs/node already has before these exports are released.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/helper/package.json` around lines 8 - 19, The package.json exports point
to ./src/index.js and ./src/index.d.ts but there is no build target wired to
produce those artifacts; add a "build" target to the helper project's
project.json using the `@nx/js`:tsc executor (mirroring libs/node's pattern),
configure its options to compile src into the expected output (emit JS + d.ts
into the path referenced by package.json or change exports to point to the
actual dist output), and ensure the build target's outputs and files list
include the generated files so releases publish them; specifically add a "build"
target entry named "build" with executor "@nx/js:tsc" and appropriate options
and outputs so the exports for "." (types and default) resolve to built
artifacts.
Comment on lines
+25
to
+27
| "generatePackageJson": true, | ||
| "tsConfig": "libs/nx/tsconfig.lib.json", | ||
| "assets": ["libs/nx/package.json", "libs/nx/README.md"] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== detect overlapping package.json strategies ==="
rg -n '"generatePackageJson"\s*:\s*true|"assets"\s*:\s*\[[^\]]*package\.json' libs/nx/project.jsonRepository: philiplehmann/container
Length of output: 224
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Get full build target context
echo "=== Full build options block ==="
sed -n '20,35p' libs/nx/project.json
echo ""
echo "=== Check if libs/nx/package.json exists ==="
test -f libs/nx/package.json && echo "✓ File exists" || echo "✗ File does not exist"
echo ""
echo "=== Check similar patterns in other targets ==="
rg -B 5 '"generatePackageJson"\s*:\s*true' libs/nx/project.json | head -30Repository: philiplehmann/container
Length of output: 771
🌐 Web query:
@nx/js:tsc generatePackageJson assets conflict behavior
💡 Result:
With @nx/js:tsc, assets are copied first, then the output package.json is (re)generated when generatePackageJson is enabled—so if one of your assets resolves to dist/.../package.json, it will be overwritten by the generated one. This is explicit in the executor implementation: it runs assetHandler.processAllAssetsOnce() and only then calls updatePackageJson(...) when options.generatePackageJson is truthy. [1]
Implications / “conflict” behavior:
assetsvs generatedpackage.json: generatedpackage.jsonwins (overwrites the copied one) whenevergeneratePackageJson: true. [1]- If you want to ship your own
package.jsonviaassets: setgeneratePackageJson: false(note the docs describe the option as generatingpackage.jsoninto the output folder). [2] - Watch mode nuance: in watch mode, Nx watches/re-copies assets, and (separately) re-runs
updatePackageJsonwhen the project’spackage.jsonchanges—so the same “generated wins” behavior persists over time. [1]
Sources:
[1] Nx source for @nx/js:tsc executor (tsc.impl.ts) showing assets copied before updatePackageJson and watch behavior.
[2] Nx docs for @nx/js:tsc executor options (generatePackageJson).
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Content of libs/nx/package.json ==="
cat libs/nx/package.json
echo ""
echo "=== Check if there are other similar build targets in the workspace ==="
rg '"generatePackageJson"\s*:\s*true' --type json -B 3 | head -50Repository: philiplehmann/container
Length of output: 743
Remove generatePackageJson to preserve custom package.json exports.
The manually maintained libs/nx/package.json defines custom exports (including "./package.json" subpath) and type: "module". When generatePackageJson: true is enabled, the generated package.json overwrites this file, losing the custom metadata and breaking the export configuration.
Either remove generatePackageJson: true to use the manually maintained package.json via assets, or regenerate the source package.json to include the required custom exports.
Suggested fix
"options": {
"outputPath": "dist/libs/nx",
"main": "libs/nx/src/index.ts",
- "generatePackageJson": true,
"tsConfig": "libs/nx/tsconfig.lib.json",
"assets": ["libs/nx/package.json", "libs/nx/README.md"]
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/nx/project.json` around lines 25 - 27, Remove the "generatePackageJson"
setting from the project config so the manually maintained libs/nx/package.json
(included via "assets") is preserved; locate the property "generatePackageJson":
true in the project JSON and delete it (or set it to false) to prevent Nx from
overwriting custom exports like "./package.json" and the "type": "module" entry.
Comment on lines
+15
to
+20
| "executors": "./executors.json", | ||
| "files": [ | ||
| "executors.json", | ||
| "src/**/*", | ||
| "README.md" | ||
| ] |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
python - <<'PY'
import json, re
from pathlib import Path
pkg_path = Path("libs/tsc/package.json")
pkg = json.loads(pkg_path.read_text())
declared = set()
for field in ("dependencies", "peerDependencies", "optionalDependencies"):
declared.update((pkg.get(field) or {}).keys())
imports = set()
for src in Path("libs/tsc/src").rglob("*.ts"):
text = src.read_text()
for m in re.finditer(r"""from\s+['"]([^'"]+)['"]""", text):
spec = m.group(1)
if spec.startswith((".", "/", "node:")):
continue
imports.add("/".join(spec.split("/")[:2]) if spec.startswith("@") else spec.split("/")[0])
print("declared:", sorted(declared))
print("external imports:", sorted(imports))
print("missing declarations:", sorted(imports - declared))
PYRepository: philiplehmann/container
Length of output: 251
Declare runtime dependencies for published executors.
This package exposes executors.json, but runtime imports used by executors (@nx/devkit, @riwi/docker, @riwi/nx) are not declared in dependencies, which will break executor resolution after publish.
Suggested fix
{
"name": "@riwi/tsc",
"version": "0.0.2",
- "dependencies": {},
+ "dependencies": {
+ "@nx/devkit": "*",
+ "@riwi/docker": "*",
+ "@riwi/nx": "*"
+ },
"type": "module",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@libs/tsc/package.json` around lines 15 - 20, The package.json exposes
executors.json but does not declare runtime dependencies used by those
executors, so add the required packages (e.g., `@nx/devkit`, `@riwi/docker`,
`@riwi/nx`) to the "dependencies" section of package.json (not devDependencies) so
executor resolution works after publish; update the dependencies field in
package.json to include these modules with appropriate version ranges and ensure
executors.json remains listed in "files".
Comment on lines
61
to
+64
| "sharedGlobals": ["{workspaceRoot}/bun.lock", "{workspaceRoot}/.github/**/*"] | ||
| }, | ||
| "cli": { | ||
| "packageManager": "bun" | ||
| "packageManager": "npm" |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== nx package manager ==="
python - <<'PY'
import json
cfg = json.load(open("nx.json"))
print(cfg.get("cli", {}).get("packageManager"))
print("sharedGlobals:", cfg.get("namedInputs", {}).get("sharedGlobals"))
PY
echo "=== workflow bun usages ==="
rg -n --type=yaml '\bbun\s+(install|nx)\b' .github/workflows || trueRepository: philiplehmann/container
Length of output: 1550
Update CI workflows to use npm and add package-lock.json to Nx cache inputs.
The nx.json declares npm as the package manager but all CI workflows still invoke bun install and bun nx .... This inconsistency breaks cache invalidation (sharedGlobals only tracks bun.lock, not package-lock.json) and creates fragility if bun is unavailable. Complete the migration by switching CI to use npm and updating the Nx config to track the npm lockfile.
Example fix for nx.json
- "sharedGlobals": ["{workspaceRoot}/bun.lock", "{workspaceRoot}/.github/**/*"]
+ "sharedGlobals": [
+ "{workspaceRoot}/package-lock.json",
+ "{workspaceRoot}/.github/**/*"
+ ]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| "sharedGlobals": ["{workspaceRoot}/bun.lock", "{workspaceRoot}/.github/**/*"] | |
| }, | |
| "cli": { | |
| "packageManager": "bun" | |
| "packageManager": "npm" | |
| "sharedGlobals": [ | |
| "{workspaceRoot}/package-lock.json", | |
| "{workspaceRoot}/.github/**/*" | |
| ] | |
| }, | |
| "cli": { | |
| "packageManager": "npm" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@nx.json` around lines 61 - 64, CI workflows still call bun while nx.json's
"cli": {"packageManager":"npm"} and "sharedGlobals" only tracks
"{workspaceRoot}/bun.lock"; update CI to use npm (replace bun install with npm
ci and bun nx/... with npm exec nx or npx nx) and adjust nx config: add
"{workspaceRoot}/package-lock.json" to "sharedGlobals" and remove or stop
relying on "{workspaceRoot}/bun.lock" so Nx cache inputs match the declared
package manager.
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 by CodeRabbit