fix(antseed): ship ids.js and queue.js in the sidecar image - #95
Conversation
control.js gained `require('./ids.js')` in #94 but Dockerfile.antseed's COPY
named files individually and was never updated. The sidecar's control server
died at import ("Cannot find module './ids.js'"), :8379 never bound, and every
wallet endpoint returned 502 — the dashboard Deposit button surfaced only a
Cloudflare error page. Nothing else noticed: the market/status writers do not
import it and the buyer proxy is a separate process.
COPY now globs antseed/*.js + *.mjs and drops the .test.js files. A new test
pins that every local require in a shipped module resolves to another shipped
file, and that the COPY stays pattern-based.
📝 WalkthroughWalkthroughThe AntSeed sidecar image now copies JavaScript modules through wildcard patterns and removes test modules. A new test checks required files, local import targets, and the Dockerfile copy pattern. ChangesAntSeed packaging
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ 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: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/test_antseed_node.py`:
- Around line 84-100: Expand the packaging assertions in the test around the
shipped-file and Dockerfile checks to cover the complete sidecar contract:
require queue.js and all expected antseed/*.mjs files alongside control.js and
ids.js, verify entrypoint.sh is included, and assert test files are removed from
the packaged output. Ensure the Dockerfile.antseed COPY rules are pattern-based
for each required artifact and preserve the existing local-import closure
validation.
🪄 Autofix
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: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 0350cec0-72f4-4c55-b4f9-05c1b2631723
📒 Files selected for processing (2)
Dockerfile.antseedtests/test_antseed_node.py
| assert "control.js" in shipped and "ids.js" in shipped, shipped | ||
|
|
||
| missing = [] | ||
| for name in sorted(shipped): | ||
| for spec in _LOCAL_IMPORT.findall((antseed / name).read_text()): | ||
| target = spec[2:] # drop the leading "./" | ||
| if target not in shipped: | ||
| missing.append(f"{name} imports {spec!r}, which is not shipped") | ||
| assert not missing, "\n".join(missing) | ||
|
|
||
| # The COPY must be pattern-based; an explicit list is what drifted. | ||
| copy_lines = [ln for ln in (_REPO_ROOT / "Dockerfile.antseed").read_text().splitlines() | ||
| if ln.startswith("COPY ") and "/usr/local/lib/antseed/" in ln] | ||
| assert copy_lines, "Dockerfile.antseed has no COPY into /usr/local/lib/antseed/" | ||
| assert any("antseed/*.js" in ln for ln in copy_lines), ( | ||
| "COPY must glob antseed/*.js — naming files individually is how ids.js " | ||
| f"and queue.js were left out of the image: {copy_lines}") |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Assert the complete sidecar packaging contract.
The test requires only control.js and ids.js at Line [84]. The Dockerfile check at Lines [98-100] only requires antseed/*.js. It does not protect queue.js, antseed/*.mjs, entrypoint.sh, or the test-file cleanup. A future packaging regression could therefore pass this test.
Proposed assertions
- assert "control.js" in shipped and "ids.js" in shipped, shipped
+ assert {"control.js", "ids.js", "queue.js"} <= shipped, shipped
+ dockerfile_text = (_REPO_ROOT / "Dockerfile.antseed").read_text()
- copy_lines = [ln for ln in (_REPO_ROOT / "Dockerfile.antseed").read_text().splitlines()
+ copy_lines = [ln for ln in dockerfile_text.splitlines()
if ln.startswith("COPY ") and "/usr/local/lib/antseed/" in ln]
...
- assert any("antseed/*.js" in ln for ln in copy_lines), (
- "COPY must glob antseed/*.js — naming files individually is how ids.js "
- f"and queue.js were left out of the image: {copy_lines}")
+ required_sources = (
+ "antseed/*.js",
+ "antseed/*.mjs",
+ "antseed/entrypoint.sh",
+ )
+ assert all(
+ any(source in line for line in copy_lines)
+ for source in required_sources
+ ), f"COPY must include {required_sources}: {copy_lines}"
+ assert "*.test.js" in dockerfile_text🧰 Tools
🪛 ast-grep (0.45.0)
[warning] 87-87: XPath query is request-/variable-derived; use parameterized XPath to prevent injection.
Context: _LOCAL_IMPORT.findall((antseed / name).read_text())
Note: [CWE-643] Improper Neutralization of Data within XPath Expressions ('XPath Injection').
(xpath-injection-python)
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@tests/test_antseed_node.py` around lines 84 - 100, Expand the packaging
assertions in the test around the shipped-file and Dockerfile checks to cover
the complete sidecar contract: require queue.js and all expected antseed/*.mjs
files alongside control.js and ids.js, verify entrypoint.sh is included, and
assert test files are removed from the packaged output. Ensure the
Dockerfile.antseed COPY rules are pattern-based for each required artifact and
preserve the existing local-import closure validation.
The repo had no pre-merge checks. notify-ci.yml fires on push to main — after the merge button — and only dispatches to the private CI repo; PR checks were CodeRabbit alone, and branch protection required a review but zero status checks. tests/test_antseed_node.py was consequently red for a month across #63/#68/#69/#70/#71 and every one merged. Three jobs: - tests: pytest against a UTF8 Postgres service (SQL_ASCII makes psycopg return TEXT as bytes, which silently voids settings overrides), submodules checked out so the engine-backed tests are real, node present for the sidecar suite. - core-tests: the Lua policy core's unit + golden conformance vectors. - images: build both images, then smoke the artifact. The image smoke is the one that would have caught #95. A green suite proves the REPO is consistent and says nothing about what COPY put in the image — which is exactly how a control.js requiring ./ids.js shipped without it, died at import, and took every wallet endpoint down with a 502. scripts/check_sidecar_modules.js resolves (never executes) each shipped module's local imports inside the built image; verified to reproduce that failure against the old COPY list.
7cb3c7a to
812168c
Compare
…#96) * ci: gate merges on tests, core conformance and a built-image smoke The repo had no pre-merge checks. notify-ci.yml fires on push to main — after the merge button — and only dispatches to the private CI repo; PR checks were CodeRabbit alone, and branch protection required a review but zero status checks. tests/test_antseed_node.py was consequently red for a month across #63/#68/#69/#70/#71 and every one merged. Three jobs: - tests: pytest against a UTF8 Postgres service (SQL_ASCII makes psycopg return TEXT as bytes, which silently voids settings overrides), submodules checked out so the engine-backed tests are real, node present for the sidecar suite. - core-tests: the Lua policy core's unit + golden conformance vectors. - images: build both images, then smoke the artifact. The image smoke is the one that would have caught #95. A green suite proves the REPO is consistent and says nothing about what COPY put in the image — which is exactly how a control.js requiring ./ids.js shipped without it, died at import, and took every wallet endpoint down with a 502. scripts/check_sidecar_modules.js resolves (never executes) each shipped module's local imports inside the built image; verified to reproduce that failure against the old COPY list. * ci: give the image smoke a database and fail fast on a dead container The router opens a host-store pool at startup, so booting it without Postgres proved only that it can fail to connect. Adds a postgres service to the images job and runs the container with --network host so it can reach it (a bridged container cannot see the runner's localhost). Also drops the '|| true' after docker run: a container that fails to start must fail the job immediately, not fall through to a curl loop that reports the same thing sixty seconds later.
Regression from #94, live in prod since 14:09 UTC.
control.jsgainedrequire('./ids.js')in #94, butDockerfile.antseednames its COPY files individually and was never updated. The sidecar's control server dies at import::8379never binds, so every wallet endpoint 502s —/x/wallet/deposit,/withdraw, and all four/reclaim/*phases. The only symptom is a Cloudflare error page on the dashboard Deposit button.Confirmed in the running pod (
v-db2c533): image contains 8 files,ids.jsandqueue.jsabsent.Impact. The remediation path for the wedged AntSeed escrow is down — the balance is $0.18 available against $15.64 stranded across 24 active channels, and neither the dashboard nor the wallet keeper can act on it. The keeper is disabled by default, so nothing tried and silently failed; had it been enabled it would have halted on its first cycle. AntSeed routing itself is unaffected (the tourniquet correctly suppresses offers, the buyer proxy and market/status writers are separate and healthy).
Fix.
COPY antseed/*.js antseed/*.mjs antseed/entrypoint.sh+ drop the.test.jsfiles, instead of an explicit list that drifts.Why it wasn't caught. The sidecar's node tests run against the repo, so they pass whether or not a file reaches the image —
ids.test.jsandqueue.test.jswere green throughout.test_every_local_import_is_shipped_in_the_sidecar_imagecloses that: every localrequire/importin a shipped module must resolve to another shipped file, and the COPY must stay pattern-based. Verified red against the old Dockerfile:Validation:
726 passed, 2 skipped.Deploy: needs a sidecar image rebuild; router and sidecar bump in lockstep via
llm-router-bump.yml.Summary by CodeRabbit
Bug Fixes
Tests