[Kukks]

Problem

regtest.mjs loads the override file before dispatching rpc, ark, and arkd. loadEnv writes a colored “Loading overrides” banner to stdout. The passthrough command then writes its response to the same stream, so consumers expecting JSON or a number receive the banner first.

This broke arkade-os/ts-sdk master's exit-providers-rotation job on run 35994264981: chainInfo() parsed rpc getblockchaininfo as JSON, and coreBlockCount() converted rpc getblockcount to a number. ts-sdk #980 now filters this banner in its e2e helper, and master CI is green. This PR repairs the passthrough CLI's stdout contract for other consumers too.

Change

Keep loading overrides for all commands, but suppress the banner for the three raw passthrough commands. Interactive commands retain the banner. The optional third argument to loadEnv keeps existing callers compatible.

Verification

• The new regression test failed before this change with the colored banner on stdout and passes after it.
• The test also asserts a non-zero exit and docker's missing-container error naming the test's bitcoin container, so it fails if the CLI errors before the passthrough runs. Checked by throwing just before loadEnv in main(): the earlier version still passed, this one fails.
• node --test: 7 passed locally and in CI before the stack smoke test.
• The full CI smoke job passed on 17c2d40.
• Local node regtest.mjs rpc getblockcount --env <ts-sdk override> prints only the block count on stdout (239 in the test stack).

A ts-sdk submodule bump is no longer required to clear master CI; #980 already did that at the consumer.

[arkana-ai-bot]

Review

Verdict: Looks good — clean fix, well-motivated, regression test included.

What the change does

loadEnv gains an optional { quiet } option (default false) that suppresses the ANSI "Loading overrides" banner on stdout. main() passes quiet: true when the command is one of the three raw passthrough verbs (ark, arkd, rpc). Interactive commands are unaffected.

Correctness

lib/env.mjs (line 47/57): The destructured default { quiet = false } = {} is backward-compatible — every existing call site (there are only two: regtest.mjs and lib/setup/arkd.mjs) continues to work unchanged.

regtest.mjs (line 474): The quiet check uses argv[0] (raw process argv), which is the same slice used by the passthrough dispatcher a few lines below. This is consistent and correct.

Edge cases checked:

• Empty argv: argv[0] is undefined; [...].includes(undefined) → false; banner still prints for ambiguous invocations.
• --env as first positional is impossible because parseArgs strips option flags before this point and the passthrough block relies on the same argv[0].
• Double-loading .env.defaults when it is passed as --env (as the test does): harmless — same values override themselves, then lose to any pre-set process.env entries anyway.

Test (test/passthrough-output.test.mjs)

The regression test is correct:

• Uses join(root, '.env.defaults') as --env so findOverride always returns a path and the old code would have emitted the banner.
• Sets REGTEST_CONTAINER_PREFIX to no-such-test-<pid>- so docker exec fails; its error goes to stderr, leaving stdout empty.
• result.error being undefined only asserts the Node binary was launchable, not that docker succeeded — which is the right guard here.
• The assertion result.stdout === '' is the meaningful invariant: before this PR the banner landed on stdout; after, it doesn't.

CI (ci.yml)

node --test is inserted before node regtest.mjs start. Good ordering — unit tests gate the heavier stack boot rather than running after it.

Cross-repo impact

loadEnv is internal to this repo; nothing in the SDK clones imports it directly. The ts-sdk consumer (packages/ts-sdk/test/e2e/utils.ts, coreBlockCount() / chainInfo()) calls regtest.mjs as a subprocess — the subprocess's stdout contract is exactly what this PR fixes. No SDK type or API surface changes.

Minor nit (non-blocking)

The test has no assertion that result.status is non-zero and no assertion on result.stderr content. This is fine for the narrow invariant being tested, but a future reader might add assert.ok(result.status !== 0) to make it explicit that the docker exec was expected to fail and the clean stdout isn't an accidental success.

[arkana-ai-bot]

Follow-up pass — 1 new commit (17f7a2c) since 30662ea; 0 prior issues fixed, 1 still open (non-blocking), 0 new findings.

---

Incremental change

Commit 17f7a2c (ci: run regtest CLI tests before smoke stack) adds exactly three lines to .github/workflows/ci.yml:

- name: Test CLI helpers
  run: node --test

inserted between the actions/setup-node step and node regtest.mjs start. No other file changed.

This is what my previous pass already described as correct: unit tests gate the heavier stack boot, so a broken CLI helper fails fast without spinning up Docker. The step is well-placed, and node --test with no arguments uses Node's built-in test runner to discover test/**/*.test.mjs, which is exactly test/passthrough-output.test.mjs added in the companion commit.

---

Prior findings closure

Nit (test/passthrough-output.test.mjs — no assertion on result.status or result.stderr content): Still open. Nothing in 17f7a2c touches the test file. Not made worse; remains non-blocking.

---

New findings

None. The three-line addition is consistent with the rest of the workflow, introduces no secret exposure, no injection surface, and no protocol-relevant change.

[Kukks]

@arkana-ai-bot Addressed your open nit in 17c2d40. The test now also asserts a non-zero exit and that stderr carries docker's missing-container error for the test's own bitcoin container (no-such-test-<pid>-bitcoin).

Your concern was real: with a throw inserted just before loadEnv in main(), the old assertions still passed 7/7, because a crash also leaves stdout empty. The new version fails in that case. It still fails when the banner suppression is removed, and when the passthrough targets the wrong container. CI smoke is green on 17c2d40, and the CI log shows the test running.

[arkana-ai-bot]

Nit closed — LGTM on 17c2d40.

Commit 17c2d40 adds two assertions to test/passthrough-output.test.mjs:

assert.notEqual(result.status, 0);
assert.match(result.stderr, new RegExp(`no-such-test-${process.pid}-bitcoin`));

Both are correct:

• Exit-code assertion — result.status !== 0 now distinguishes "docker exec failed cleanly" from "process crashed before docker was invoked". @Kukks's throw-before-loadEnv proof confirms the old assertions didn't catch that case; these do.
• Stderr content assertion — The regex is built from process.pid, the same value used to construct REGTEST_CONTAINER_PREFIX earlier in the test, so the container name in Docker's error message will always match. No regex-injection risk (PID is all digits).
• Ordering — The stdout invariant (result.stdout === '') is still checked first, which is the right precedence for this PR's core claim.

One minor observation (not blocking): assert.match will throw a TypeError if result.stderr is null or undefined rather than failing with a clean assertion message. If the test harness can ever capture stderr as null (e.g., if stdio is misconfigured), the failure mode would be slightly confusing. Worth keeping in mind if the test ever starts flaking on CI with a type error rather than an assertion error — but for the current setup it's fine.

Everything else from prior passes stands. No new findings. The PR is ready to merge from a code-review perspective.

[arkana-ai-bot]

Follow-up pass — 1 new commit (17c2d40) since 17f7a2c; 1 prior nit Fixed, 0 issues remain.

---

Prior findings closure

Nit (test/passthrough-output.test.mjs — no assertion on result.status or result.stderr): Fixed in 17c2d40.

The two added lines are correct:

assert.notEqual(result.status, 0);
assert.match(result.stderr, new RegExp(`no-such-test-${process.pid}-bitcoin`));

• assert.notEqual(result.status, 0) distinguishes "docker exec failed cleanly after the passthrough ran" from "process crashed before it got there." A crash also produces empty stdout, so the old test had a blind spot here — now closed.
• assert.match against new RegExp(\no-such-test-${process.pid}-bitcoin`)is correct. The PID is all digits, so the interpolated string is a safe literal pattern with no regex metacharacters. The container name is constructed identically to theREGTEST_CONTAINER_PREFIX` set earlier in the test, so the match is deterministic.
• Assertion order (error → stdout → status → stderr) is sensible: the invariant under test (no stdout banner) is checked before the confirmatory docker-reached check.
• One minor edge I noted previously — assert.match throws TypeError if stderr is null/undefined rather than a clean assertion failure — is not introduced by this commit and the spawnSync call uses encoding: 'utf8', which makes stderr a string. Non-issue in practice.

---

New findings

None. The two-line addition is the entire diff. No protocol, security, or cross-repo impact.

---

The PR is ready to merge.