fix: BUG: domains — Bulk Domain Check tests make live network calls, so

[andrei-hasna]

Objective

BUG: domains — Bulk Domain Check tests make live network calls, so the suite can never pass in CI

Blocks CI adoption on hasna/domains. PR #15 added a correct CI workflow (checkout, setup-bun, frozen install, typecheck, bun test on pull_request + push main). Typecheck passed; bun test failed. PR closed rather than merged, because merging installs a permanently-red gate.

FAILING (run 30460064498, job 90603490735), all in src/db/monitoring.test.ts 'Bulk Domain Check':
checkAllDomains returns empty array when no domains [61366.65ms]
checkAllDomains returns structured results [6033.15ms]
checkAllDomains whois result structure [5995.36ms]

ROOT CAUSE: checkAllDomains() performs live whois, SSL and DNS lookups. A GitHub runner cannot reach those, so the calls hang to timeout — hence 61 seconds on a test whose own name says it expects an empty array. These are integration tests wearing unit-test clothing.

EXACT REMEDY (either is acceptable):

1. Inject a resolver/whois client so the tests use a fake by default, keeping the live path behind an explicit opt-in; or
2. Tag them as integration tests and exclude them from the default 'bun test' run, giving CI a separate opt-in job.
   Do NOT fix this by lengthening timeouts — the network is absent, not slow.

ACCEPTANCE: 'bun test' green from a clean clone with no network egress, then re-open a CI workflow PR.

Verification

• policy source: base 7954896 (immutable commit — agent-proof)
• containment: env — allowlist env, non-login shell, run-scoped HOME (registry auth seeded for install)
• install: pass
• typecheck: pass
• build: pass
• test: pass
• doctor (ci): ok — 11 checks passed (1 advisory)

---

Run run_3b495d5caf51 · backend codewith · task 5a08860d-ce66-4465-80de-bae9f0674009
🏭 Generated by @hasnaxyz/factory

---

^{Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.}

[andrei-hasna]

[REVIEW] NO_GO — #20 @ bdc4d61 — lens: correctness+security+gates, reviewer Augustus (1 of 1)

What I ran, with exit codes:

• git log --oneline origin/main..HEAD — exit 0
• git diff origin/main...HEAD --stat — exit 0
• git diff origin/main...HEAD -- src/db/monitoring.ts src/db/monitoring.test.ts — exit 0
• git diff --check origin/main...HEAD — exit 0
• BUN_INSTALL_CACHE_DIR=/home/hasna/.hasna/repos/worktrees/open-domains/pr20-review/.bun-install-cache bun install — exit 0
• bun run typecheck — exit 0
• bun test — exit 1
• Diagnostic rerun: env -u HASNA_DOMAINS_API_URL -u HASNA_DOMAINS_API_KEY -u HASNA_DOMAINS_STORAGE_MODE -u HASNA_DOMAINS_MODE -u DOMAINS_API_URL -u DOMAINS_API_KEY XDG_DATA_HOME=/home/hasna/.hasna/repos/worktrees/open-domains/pr20-review/.xdg-data XDG_CONFIG_HOME=/home/hasna/.hasna/repos/worktrees/open-domains/pr20-review/.xdg-config bun test — exit 0, 401 pass / 0 fail

What I read:

• Commit list and diff stat for origin/main..HEAD / origin/main...HEAD.
• Full diff for both changed files: src/db/monitoring.ts and src/db/monitoring.test.ts.
• Surrounding source: full src/db/monitoring.ts, full src/db/monitoring.test.ts, relevant dependency implementations in src/db/dns-tools.ts, storage resolution in src/db/store.ts, local database setup in src/db/database.ts, representative local-store tests in src/db/domains.test.ts, MCP/CLI test setup in src/mcp/http.test.ts and src/cli/compact-output.test.ts, plus package.json and tsconfig.json.

Blocking P0/P1 findings:

• P1: The exact required test gate is not green in this review environment. Plain bun test exited 1 with 145 failing tests and 5 errors. The failures route through the cloud HTTP store and include duplicate-domain 409s such as POST /domains -> 409 for existing fixture names, then cascades like POST /domains/undefined/dns -> 404 and SQLite NOT NULL failures after setup domains were not created. I verified only environment variable names, not values: HASNA_DOMAINS_API_URL, HASNA_DOMAINS_API_KEY, and HASNA_DOMAINS_STORAGE_MODE are present in this session. A diagnostic rerun with those domain cloud variables unset passed all 401 tests, so the likely remedy is to make the test harness force local storage / scrub ambient domain cloud env, or to make the required gate encode that isolation. Until the exact requested bun test gate is green, I am not merging.

Changed-code correctness/security review:

• No concrete P0/P1 correctness or security defect found in the two changed files. checkAllDomains() still defaults to live dependencies for production callers, the injection is optional, WHOIS/SSL errors are still caught per domain, and the invalid-domain regression continues to exercise the real validators without creating the shell marker.

Non-blocking follow-ups:

• None for the changed files. The ambient-env test isolation issue above is blocking only because it breaks the required test gate.

Disposition:

• NO_GO; PR left open. Required next step is to make plain bun test pass from this worktree when HASNA_DOMAINS_API_URL, HASNA_DOMAINS_API_KEY, and HASNA_DOMAINS_STORAGE_MODE exist in the caller environment, or to change the repo's documented/required test gate so local test isolation is explicit.

[andrei-hasna]

[REVIEW] NO_GO — #20 @ bdc4d61 — lens: correctness+security+gates, reviewer Augustus (1 of 1)

What I ran on the original candidate:

• git log --oneline origin/main..HEAD exit 0: one commit, bdc4d61 fix: BUG: domains — Bulk Domain Check tests make live network calls, so
• git diff origin/main...HEAD --stat exit 0: src/db/monitoring.ts and src/db/monitoring.test.ts, 69 insertions and 14 deletions
• bun install exit 0
• bun run typecheck exit 0
• bun test exit 1

What I read:

• Full diff for src/db/monitoring.ts and src/db/monitoring.test.ts
• Surrounding source in src/db/monitoring.ts, src/db/monitoring.test.ts, src/db/dns-tools.ts, src/db/domains.ts, src/db/store.ts, and the MCP check_all_domains caller in src/mcp/index.ts
• package.json scripts and current PR head/base metadata

Blocking P0/P1 findings:

• P1 broken test gate and unsafe live-test mutation path: bun test on the exact candidate fails because the suite inherits domains cloud client environment variables from the shell. Test files set DOMAINS_DIR, but getStore() still resolves the cloud HTTP store when HASNA_DOMAINS_API_URL and related storage settings are present, so unit tests write to the live domains API instead of temp SQLite. Evidence from the failed gate: 71 failed tests plus 7 errors, including src/db/monitoring.test.ts, with repeated POST /domains -> 409 duplicate-domain failures such as mcp-compact-*.example. This is currently reachable for any reviewer or CI worker with those domains client variables exported, and it means the required bun test gate is not green.

Non-blocking follow-ups:

• None from the code diff itself. The dependency injection in checkAllDomains() preserves the zero-argument production caller and correctly lets the monitoring tests avoid live WHOIS/SSL calls.

Disposition:

• Small named remedy implemented in follow-up commit: add a Bun test preload that strips domains cloud client environment variables during tests and forces test execution onto a temp local SQLite directory.

[andrei-hasna]

[REVIEW-FIX] #20 — reviewer Augustus — pushed remedy commit 64410d1

Fixed the blocking gate failure by adding:

• bunfig.toml with a Bun test preload
• src/test/setup.ts, which removes domains cloud client environment variables for test runs, clears direct DB path overrides, and gives the suite a temp local SQLite directory

Post-fix verification on commit 64410d15e8f421c4d3dd39041b89893ebd58948e:

• bun install exit 0
• bun run typecheck exit 0
• bun test exit 0: 401 pass, 0 fail, 1182 assertions across 44 files
• staged credential-value scan before commit exit 0
• git diff --cached --check before commit exit 0

I did not merge the PR because the required review verdict for the original requested candidate was NO_GO; this follow-up only pushes the small named remedy and records the now-green gates on the repaired PR head.

[andrei-hasna]

[REVIEW] GO — #20 @ 64410d1 — lens: correctness+security+gates, reviewer Augustus (1 of 1)

What I ran:

• git rev-parse HEAD — exit 0; confirmed 64410d15e8f421c4d3dd39041b89893ebd58948e.
• git log --oneline origin/main..HEAD — exit 0; reviewed commits 64410d1 and bdc4d61.
• git diff origin/main...HEAD --stat — exit 0; changed files are bunfig.toml, src/db/monitoring.test.ts, src/db/monitoring.ts, and src/test/setup.ts.
• git diff origin/main...HEAD -- bunfig.toml src/db/monitoring.test.ts src/db/monitoring.ts src/test/setup.ts — exit 0; read the full patch.
• Surrounding source reads with sed/rg — mostly exit 0; one exploratory rg "checkAllDomains|BulkCheckDependencies|monitoring" -n src test . --glob '!node_modules' returned exit 2 because this repo has no top-level test directory, while still showing the relevant matches. I did not treat that as a gate.
• bun install — exit 0.
• bun run typecheck — exit 0.
• bun test — exit 0; 401 pass, 0 fail, across 44 files.

What I read:

• Full PR diff for bunfig.toml, src/db/monitoring.test.ts, src/db/monitoring.ts, and src/test/setup.ts.
• Surrounding source in src/db/monitoring.ts, src/db/monitoring.test.ts, src/test/setup.ts, src/db/database.ts, src/db/store.ts, src/db/dns-tools.ts, src/mcp/index.ts, src/db/domains.test.ts, src/db/dns-tools.test.ts, and package.json.

Blocking P0/P1 findings: none.

Non-blocking follow-ups: none. The optional dependency injection preserves the live default behavior of checkAllDomains(), and the test setup prevents cloud/client env leakage during Bun tests without changing production behavior.

[andrei-hasna]

[REVIEW] GO — #20 @ 6a718fa — lens: correctness+security+gates, reviewer Augustus (1 of 1)

What I ran with exit codes:

• git rebase origin/main — exit 1 initially because src/test/setup.ts had an add/add conflict with current origin/main; resolved by preserving the PR's test isolation and main's explicit local storage mode.
• git diff --cached --check before continuing the rebase — exit 0.
• Staged credential-value scan before continuing the rebase — exit 0 after rerunning with valid scanner patterns; no credential value patterns found.
• GIT_EDITOR=true git rebase --continue — exit 0; new local head 6a718fa4b3242834f175fdd5defd6250f1f9dc63.
• git log --oneline origin/main..HEAD — exit 0; reviewed commits 7c57323 and 6a718fa.
• git diff origin/main...HEAD --stat — exit 0; changed files are src/db/monitoring.test.ts, src/db/monitoring.ts, and src/test/setup.ts.
• git diff origin/main...HEAD -- bunfig.toml src/db/monitoring.test.ts src/db/monitoring.ts src/test/setup.ts — exit 0; read the full rebased patch.
• bun install — exit 0.
• bun run typecheck — exit 0.
• bun test — exit 0; 419 pass, 0 fail, 1255 assertions across 46 files.
• git diff --check origin/main...HEAD — exit 0.
• Branch credential-value scan over git diff origin/main...HEAD --binary — exit 0; no credential value patterns found.
• git push --force-with-lease=refs/heads/factory/5a08860d-ce66-4465-80de-bae9f067-bd99d74a:64410d15e8f421c4d3dd39041b89893ebd58948e origin HEAD:refs/heads/factory/5a08860d-ce66-4465-80de-bae9f067-bd99d74a — exit 0.

What I read:

• Full rebased PR diff for src/db/monitoring.test.ts, src/db/monitoring.ts, and src/test/setup.ts.
• The conflicting origin/main:src/test/setup.ts and origin/main:bunfig.toml.
• Surrounding source in src/db/monitoring.ts, src/db/monitoring.test.ts, src/test/setup.ts, src/db/database.ts, src/db/store.ts, src/db/dns-tools.ts, src/mcp/index.ts, src/db/domains.test.ts, src/db/dns-tools.test.ts, and package.json.

Blocking P0/P1 findings: none.

Non-blocking follow-ups: none. The rebased setup keeps tests isolated from ambient domains cloud client credentials and direct DB path overrides while preserving explicit local storage mode; checkAllDomains() still uses live dependencies by default for production callers.