feat: add Verdaccio-based sanity suite for real npm install testing#503
feat: add Verdaccio-based sanity suite for real npm install testing#503
Conversation
Add a parallel sanity suite that tests the actual `npm install -g` flow using Verdaccio as a local npm registry. This catches publish pipeline bugs that the copy-based Dockerfile cannot detect: - Missing files in package tarball (symlinks not followed by npm pack) - Broken postinstall scripts - Wrong bin field in package.json - Dependency resolution failures at install time - Native binary platform detection failures Pipeline: build → publish to Verdaccio → npm install -g → run sanity tests Files added: - Dockerfile.verdaccio: builds packages, publishes at runtime via entrypoint - docker-compose.verdaccio.yml: Verdaccio + postgres + mongodb services - verdaccio/config.yaml: permissive config (publish: $all, 500mb max body) - verdaccio/entrypoint.sh: publish both packages, npm install -g, run tests - run-verdaccio.sh: convenience wrapper Already found real bug: postinstall fails because bin/altimate-code is a symlink that breaks when npm pack/unpacks the tarball in a different directory. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review to trigger a review and subscribe this PR to future pushes, or @claude review once for a one-time review.
Tip: disable this comment in your organization's Code Review settings.
📝 WalkthroughWalkthroughThis PR adds a Verdaccio-backed end-to-end sanity testing infrastructure for validating the real Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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 |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (3)
test/sanity/Dockerfile.verdaccio (1)
4-6: Add--no-install-recommendsto reduce image bloat and attack surface.This directly addresses the static-analysis hint and keeps the runtime slimmer.
Suggested change
RUN apt-get update && apt-get install -y \ + --no-install-recommends \ git python3 python3-pip python3-venv curl sqlite3 npm \ && rm -rf /var/lib/apt/lists/*🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/sanity/Dockerfile.verdaccio` around lines 4 - 6, The RUN command in the Dockerfile installs packages without using --no-install-recommends, which increases image size and attack surface; update the RUN instruction that performs "apt-get update && apt-get install -y \ git python3 ..." to include --no-install-recommends immediately after apt-get install -y so the install omits recommended packages, then keep the existing cleanup "&& rm -rf /var/lib/apt/lists/*".test/sanity/verdaccio/entrypoint.sh (1)
68-107: Consider reusing canonical publish logic instead of rebuilding package metadata in-shell.This block duplicates packaging rules already defined in
packages/opencode/script/publish.ts(asset list + generated manifest). Drift here can create false failures or false passes over time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/sanity/verdaccio/entrypoint.sh` around lines 68 - 107, The script duplicates the packaging logic from packages/opencode/script/publish.ts (asset copy list and generated package.json); instead of rebuilding that manifest in test/sanity/verdaccio/entrypoint.sh, invoke the canonical publish logic from packages/opencode/script/publish.ts (or export a reusable helper from that module) to produce the same published output into $PUBLISH_DIR and copy assets (bin, postinstall.mjs, skills, dbt-tools, LICENSE, CHANGELOG.md); update entrypoint.sh to call the publish routine (or a small Node wrapper that imports and runs the publish function) so the package.json and asset list come from the single source of truth (publish.ts) rather than duplicated shell code.test/sanity/docker-compose.verdaccio.yml (1)
19-20: Remove unused build args to avoid config drift.
REGISTRY_URLis defined under build args here but not consumed bytest/sanity/Dockerfile.verdaccio.Suggested change
build: context: ../.. dockerfile: test/sanity/Dockerfile.verdaccio - args: - REGISTRY_URL: http://verdaccio:4873🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@test/sanity/docker-compose.verdaccio.yml` around lines 19 - 20, The docker-compose service defines a build arg REGISTRY_URL that is not referenced by the Dockerfile (Dockerfile.verdaccio); remove the REGISTRY_URL entry from the service's args block to avoid config drift (or if it was intended to be used, instead add ARG REGISTRY_URL and use it in Dockerfile.verdaccio). Locate the args block containing REGISTRY_URL in the verdaccio service and either delete that line or add matching ARG handling in Dockerfile.verdaccio so the build arg is actually consumed.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@test/sanity/docker-compose.verdaccio.yml`:
- Around line 8-9: The docker-compose service currently publishes ports with
host-wide exposure (e.g., the ports entry "- \"4873:4873\"" in
test/sanity/docker-compose.verdaccio.yml); change these to bind only to
localhost (e.g., use "127.0.0.1:4873:4873") or remove the host publish entirely
if external access isn’t needed; update all matching ports mappings (the entries
around lines shown: the one for 4873 and the other two mappings noted) so
Verdaccio and other test services bind to 127.0.0.1 instead of 0.0.0.0.
In `@test/sanity/run-verdaccio.sh`:
- Around line 4-38: Because set -e causes the docker compose failure to abort
the script before capturing EXIT_CODE and doing the custom summary/cleanup,
temporarily disable errexit around the compose invocation: before the docker
compose command run set +e, run the compose, capture EXIT_CODE=$?, then restore
set -euo pipefail; leave the existing cleanup() and trap cleanup EXIT in place
and finally exit with the captured EXIT_CODE. Reference: the docker compose
block (the command that currently runs up ... --exit-code-from sanity), the
EXIT_CODE variable, and the cleanup() function.
In `@test/sanity/verdaccio/config.yaml`:
- Around line 11-15: The '@altimateai/*' scope in the Verdaccio config currently
has 'proxy: npmjs', which enables uplink fallback and can mask local
publish/packaging issues; remove the 'proxy: npmjs' line (or set the proxy to
null/disable for that scope) under the "@altimateai/*" block so the registry
will not fallback to the public npm uplink when resolving that scope.
In `@test/sanity/verdaccio/entrypoint.sh`:
- Line 61: The test script entrypoint.sh currently suppresses failures from the
npm publish step by appending "|| echo ..." to the npm publish command; remove
that suppression so failures are propagated (i.e., run npm publish --registry
"$REGISTRY_URL" without the "|| echo ..." fallback) or explicitly capture and
log the error and exit non‑zero, ensuring the npm publish invocation in
entrypoint.sh fails the pipeline when publishing fails.
- Around line 76-78: The script currently masks missing dbt-tools artifacts by
appending "|| true" to the three copy commands for "altimate-dbt",
"dist/index.js", and "dist/node_python_bridge.py"; remove the "|| true" suffixes
and make the script fail fast by checking each cp exit status (or enable set -e
at the top) and emit a clear error and exit non-zero if any required file is
missing so packaging sanity breaks immediately when those artifacts are absent.
---
Nitpick comments:
In `@test/sanity/docker-compose.verdaccio.yml`:
- Around line 19-20: The docker-compose service defines a build arg REGISTRY_URL
that is not referenced by the Dockerfile (Dockerfile.verdaccio); remove the
REGISTRY_URL entry from the service's args block to avoid config drift (or if it
was intended to be used, instead add ARG REGISTRY_URL and use it in
Dockerfile.verdaccio). Locate the args block containing REGISTRY_URL in the
verdaccio service and either delete that line or add matching ARG handling in
Dockerfile.verdaccio so the build arg is actually consumed.
In `@test/sanity/Dockerfile.verdaccio`:
- Around line 4-6: The RUN command in the Dockerfile installs packages without
using --no-install-recommends, which increases image size and attack surface;
update the RUN instruction that performs "apt-get update && apt-get install -y \
git python3 ..." to include --no-install-recommends immediately after apt-get
install -y so the install omits recommended packages, then keep the existing
cleanup "&& rm -rf /var/lib/apt/lists/*".
In `@test/sanity/verdaccio/entrypoint.sh`:
- Around line 68-107: The script duplicates the packaging logic from
packages/opencode/script/publish.ts (asset copy list and generated
package.json); instead of rebuilding that manifest in
test/sanity/verdaccio/entrypoint.sh, invoke the canonical publish logic from
packages/opencode/script/publish.ts (or export a reusable helper from that
module) to produce the same published output into $PUBLISH_DIR and copy assets
(bin, postinstall.mjs, skills, dbt-tools, LICENSE, CHANGELOG.md); update
entrypoint.sh to call the publish routine (or a small Node wrapper that imports
and runs the publish function) so the package.json and asset list come from the
single source of truth (publish.ts) rather than duplicated shell code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 83a11407-cec3-44a7-b094-526146654f55
📒 Files selected for processing (6)
test/sanity/Dockerfile.verdacciotest/sanity/docker-compose.verdaccio.ymltest/sanity/run-verdaccio.shtest/sanity/verdaccio/config.yamltest/sanity/verdaccio/entrypoint.shtest/sanity/verdaccio/htpasswd
| ports: | ||
| - "4873:4873" |
There was a problem hiding this comment.
Limit host exposure of test services (especially open Verdaccio).
These mappings bind on all interfaces by default. Given this suite’s permissive registry policy, prefer localhost-only binds (or remove host publishes when not needed).
Suggested change
ports:
- - "4873:4873"
+ - "127.0.0.1:4873:4873"
@@
ports:
- - "15432:5432"
+ - "127.0.0.1:15432:5432"
@@
ports:
- - "17017:27017"
+ - "127.0.0.1:17017:27017"Also applies to: 42-43, 50-51
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/sanity/docker-compose.verdaccio.yml` around lines 8 - 9, The
docker-compose service currently publishes ports with host-wide exposure (e.g.,
the ports entry "- \"4873:4873\"" in test/sanity/docker-compose.verdaccio.yml);
change these to bind only to localhost (e.g., use "127.0.0.1:4873:4873") or
remove the host publish entirely if external access isn’t needed; update all
matching ports mappings (the entries around lines shown: the one for 4873 and
the other two mappings noted) so Verdaccio and other test services bind to
127.0.0.1 instead of 0.0.0.0.
| set -euo pipefail | ||
|
|
||
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | ||
| REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" | ||
|
|
||
| echo "========================================" | ||
| echo " Verdaccio Sanity Suite" | ||
| echo " Tests the real npm install -g flow" | ||
| echo " Time: $(date -u +%Y-%m-%dT%H:%M:%SZ)" | ||
| echo "========================================" | ||
| echo "" | ||
|
|
||
| # Ensure binary is built | ||
| if [ ! -d "$REPO_ROOT/packages/opencode/dist/@altimateai" ]; then | ||
| echo "ERROR: No built binary found at packages/opencode/dist/@altimateai/" | ||
| echo " Run: cd packages/opencode && bun run build" | ||
| exit 1 | ||
| fi | ||
|
|
||
| # Clean up on exit | ||
| cleanup() { | ||
| echo "" | ||
| echo "Cleaning up Docker containers..." | ||
| docker compose -f "$SCRIPT_DIR/docker-compose.verdaccio.yml" down --volumes --remove-orphans 2>/dev/null || true | ||
| } | ||
| trap cleanup EXIT | ||
|
|
||
| # Run the suite | ||
| docker compose -f "$SCRIPT_DIR/docker-compose.verdaccio.yml" up \ | ||
| --build \ | ||
| --abort-on-container-exit \ | ||
| --exit-code-from sanity | ||
|
|
||
| EXIT_CODE=$? | ||
|
|
There was a problem hiding this comment.
errexit currently bypasses your custom failure summary path.
Because of Line 4 (set -euo pipefail), a non-zero return from the compose command at Line 32-35 exits before Line 37 is reached.
Suggested change
# Run the suite
+set +e
docker compose -f "$SCRIPT_DIR/docker-compose.verdaccio.yml" up \
--build \
--abort-on-container-exit \
--exit-code-from sanity
-
EXIT_CODE=$?
+set -e📝 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.
| set -euo pipefail | |
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | |
| REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" | |
| echo "========================================" | |
| echo " Verdaccio Sanity Suite" | |
| echo " Tests the real npm install -g flow" | |
| echo " Time: $(date -u +%Y-%m-%dT%H:%M:%SZ)" | |
| echo "========================================" | |
| echo "" | |
| # Ensure binary is built | |
| if [ ! -d "$REPO_ROOT/packages/opencode/dist/@altimateai" ]; then | |
| echo "ERROR: No built binary found at packages/opencode/dist/@altimateai/" | |
| echo " Run: cd packages/opencode && bun run build" | |
| exit 1 | |
| fi | |
| # Clean up on exit | |
| cleanup() { | |
| echo "" | |
| echo "Cleaning up Docker containers..." | |
| docker compose -f "$SCRIPT_DIR/docker-compose.verdaccio.yml" down --volumes --remove-orphans 2>/dev/null || true | |
| } | |
| trap cleanup EXIT | |
| # Run the suite | |
| docker compose -f "$SCRIPT_DIR/docker-compose.verdaccio.yml" up \ | |
| --build \ | |
| --abort-on-container-exit \ | |
| --exit-code-from sanity | |
| EXIT_CODE=$? | |
| set -euo pipefail | |
| SCRIPT_DIR="$(cd "$(dirname "$0")" && pwd)" | |
| REPO_ROOT="$(cd "$SCRIPT_DIR/../.." && pwd)" | |
| echo "========================================" | |
| echo " Verdaccio Sanity Suite" | |
| echo " Tests the real npm install -g flow" | |
| echo " Time: $(date -u +%Y-%m-%dT%H:%M:%SZ)" | |
| echo "========================================" | |
| echo "" | |
| # Ensure binary is built | |
| if [ ! -d "$REPO_ROOT/packages/opencode/dist/@altimateai" ]; then | |
| echo "ERROR: No built binary found at packages/opencode/dist/@altimateai/" | |
| echo " Run: cd packages/opencode && bun run build" | |
| exit 1 | |
| fi | |
| # Clean up on exit | |
| cleanup() { | |
| echo "" | |
| echo "Cleaning up Docker containers..." | |
| docker compose -f "$SCRIPT_DIR/docker-compose.verdaccio.yml" down --volumes --remove-orphans 2>/dev/null || true | |
| } | |
| trap cleanup EXIT | |
| # Run the suite | |
| set +e | |
| docker compose -f "$SCRIPT_DIR/docker-compose.verdaccio.yml" up \ | |
| --build \ | |
| --abort-on-container-exit \ | |
| --exit-code-from sanity | |
| EXIT_CODE=$? | |
| set -e | |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/sanity/run-verdaccio.sh` around lines 4 - 38, Because set -e causes the
docker compose failure to abort the script before capturing EXIT_CODE and doing
the custom summary/cleanup, temporarily disable errexit around the compose
invocation: before the docker compose command run set +e, run the compose,
capture EXIT_CODE=$?, then restore set -euo pipefail; leave the existing
cleanup() and trap cleanup EXIT in place and finally exit with the captured
EXIT_CODE. Reference: the docker compose block (the command that currently runs
up ... --exit-code-from sanity), the EXIT_CODE variable, and the cleanup()
function.
| "@altimateai/*": | ||
| access: $all | ||
| publish: $all | ||
| unpublish: $all | ||
| proxy: npmjs |
There was a problem hiding this comment.
Disable uplink proxy for @altimateai/* to prevent false-positive installs.
At Line 15, proxy: npmjs allows fallback to public npm for the same scope being validated. That can hide local publish/packaging failures in this suite.
Suggested change
"@altimateai/*":
access: $all
publish: $all
unpublish: $all
- proxy: npmjs
+ # no proxy: fail fast if the package was not published locally📝 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.
| "@altimateai/*": | |
| access: $all | |
| publish: $all | |
| unpublish: $all | |
| proxy: npmjs | |
| "@altimateai/*": | |
| access: $all | |
| publish: $all | |
| unpublish: $all | |
| # no proxy: fail fast if the package was not published locally |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/sanity/verdaccio/config.yaml` around lines 11 - 15, The '@altimateai/*'
scope in the Verdaccio config currently has 'proxy: npmjs', which enables uplink
fallback and can mask local publish/packaging issues; remove the 'proxy: npmjs'
line (or set the proxy to null/disable for that scope) under the "@altimateai/*"
block so the registry will not fallback to the public npm uplink when resolving
that scope.
| # Patch version in binary package.json to sanitized version | ||
| cd "$BIN_PKG_DIR" | ||
| node -e "const p=require('./package.json');p.version='$VERSION';require('fs').writeFileSync('package.json',JSON.stringify(p,null,2)+'\n')" | ||
| npm publish --registry "$REGISTRY_URL" 2>&1 || echo " (binary publish failed or already published)" |
There was a problem hiding this comment.
Do not suppress binary publish failures in this validation pipeline.
At Line 61, swallowing npm publish failures can hide the root cause and invalidate suite confidence.
Suggested change
-npm publish --registry "$REGISTRY_URL" 2>&1 || echo " (binary publish failed or already published)"
+npm publish --registry "$REGISTRY_URL" 2>&1📝 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.
| npm publish --registry "$REGISTRY_URL" 2>&1 || echo " (binary publish failed or already published)" | |
| npm publish --registry "$REGISTRY_URL" 2>&1 |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/sanity/verdaccio/entrypoint.sh` at line 61, The test script
entrypoint.sh currently suppresses failures from the npm publish step by
appending "|| echo ..." to the npm publish command; remove that suppression so
failures are propagated (i.e., run npm publish --registry "$REGISTRY_URL"
without the "|| echo ..." fallback) or explicitly capture and log the error and
exit non‑zero, ensuring the npm publish invocation in entrypoint.sh fails the
pipeline when publishing fails.
| cp "$BUILD_DIR/packages/dbt-tools/bin/altimate-dbt" "$PUBLISH_DIR/dbt-tools/bin/altimate-dbt" 2>/dev/null || true | ||
| cp "$BUILD_DIR/packages/dbt-tools/dist/index.js" "$PUBLISH_DIR/dbt-tools/dist/" 2>/dev/null || true | ||
| cp "$BUILD_DIR/packages/dbt-tools/dist/node_python_bridge.py" "$PUBLISH_DIR/dbt-tools/dist/" 2>/dev/null || true |
There was a problem hiding this comment.
Fail fast when required dbt-tools artifacts are missing.
The || true on Line 76-78 masks missing files that should fail packaging sanity immediately.
Suggested change
-cp "$BUILD_DIR/packages/dbt-tools/bin/altimate-dbt" "$PUBLISH_DIR/dbt-tools/bin/altimate-dbt" 2>/dev/null || true
-cp "$BUILD_DIR/packages/dbt-tools/dist/index.js" "$PUBLISH_DIR/dbt-tools/dist/" 2>/dev/null || true
-cp "$BUILD_DIR/packages/dbt-tools/dist/node_python_bridge.py" "$PUBLISH_DIR/dbt-tools/dist/" 2>/dev/null || true
+cp "$BUILD_DIR/packages/dbt-tools/bin/altimate-dbt" "$PUBLISH_DIR/dbt-tools/bin/altimate-dbt"
+cp "$BUILD_DIR/packages/dbt-tools/dist/index.js" "$PUBLISH_DIR/dbt-tools/dist/"
+cp "$BUILD_DIR/packages/dbt-tools/dist/node_python_bridge.py" "$PUBLISH_DIR/dbt-tools/dist/"📝 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.
| cp "$BUILD_DIR/packages/dbt-tools/bin/altimate-dbt" "$PUBLISH_DIR/dbt-tools/bin/altimate-dbt" 2>/dev/null || true | |
| cp "$BUILD_DIR/packages/dbt-tools/dist/index.js" "$PUBLISH_DIR/dbt-tools/dist/" 2>/dev/null || true | |
| cp "$BUILD_DIR/packages/dbt-tools/dist/node_python_bridge.py" "$PUBLISH_DIR/dbt-tools/dist/" 2>/dev/null || true | |
| cp "$BUILD_DIR/packages/dbt-tools/bin/altimate-dbt" "$PUBLISH_DIR/dbt-tools/bin/altimate-dbt" | |
| cp "$BUILD_DIR/packages/dbt-tools/dist/index.js" "$PUBLISH_DIR/dbt-tools/dist/" | |
| cp "$BUILD_DIR/packages/dbt-tools/dist/node_python_bridge.py" "$PUBLISH_DIR/dbt-tools/dist/" |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@test/sanity/verdaccio/entrypoint.sh` around lines 76 - 78, The script
currently masks missing dbt-tools artifacts by appending "|| true" to the three
copy commands for "altimate-dbt", "dist/index.js", and
"dist/node_python_bridge.py"; remove the "|| true" suffixes and make the script
fail fast by checking each cp exit status (or enable set -e at the top) and emit
a clear error and exit non-zero if any required file is missing so packaging
sanity breaks immediately when those artifacts are absent.
Summary
npm install -g altimate-codeflow using a local npm registrynpm install -gfrom Verdaccio → run sanity testsWhat this catches that the old approach didn't
npm pack)postinstall.mjsscriptsbinfield in package.jsonAlready found a real bug
The postinstall script fails because
bin/altimate-codeis a symlink →altimatethat breaks whennpm packcreates a tarball and npm unpacks it in a different directory. This bug was invisible with the old copy-based Dockerfile.Files
Dockerfile.verdacciodocker-compose.verdaccio.ymlverdaccio/config.yamlpublish: $all,max_body_size: 500mbverdaccio/entrypoint.shnpm install -g, run sanity testsrun-verdaccio.shbash test/sanity/run-verdaccio.shTest plan
npm install -gruns and triggers postinstall🤖 Generated with Claude Code
Summary by CodeRabbit