test: project-scan — ClickHouse env var detection and DATABASE_URL scheme parsing

[anandgupta42]

Summary

detectEnvVars() — src/altimate/tools/project-scan.ts (4 new tests)

The ClickHouse warehouse driver was added in PR #574, which included new env var detection entries (CLICKHOUSE_HOST, CLICKHOUSE_URL) and new DATABASE_URL scheme mappings (clickhouse://, clickhouse+http://, clickhouse+https://) in detectEnvVars(). However, no tests were added for these code paths. All other warehouse types (Snowflake, BigQuery, Databricks, Postgres, MySQL, Redshift) had detection tests — ClickHouse was the only gap.

Why it matters: Without these tests, a regression that removes or breaks the ClickHouse entries in detectEnvVars() would go undetected. Users with CLICKHOUSE_HOST or DATABASE_URL=clickhouse://... set would get no auto-detection during project_scan, breaking the "just works" onboarding experience. The DATABASE_URL scheme tests are especially important because a missing scheme entry silently falls through to the "postgres" default — a silent misclassification.

Specific scenarios covered:

• ClickHouse detection via CLICKHOUSE_HOST with full config (host, port, database, user, password masking)
• ClickHouse detection via CLICKHOUSE_URL (connection_string signal path)
• DATABASE_URL with clickhouse:// scheme → correctly classified as clickhouse (not postgres)
• DATABASE_URL with clickhouse+http:// and clickhouse+https:// schemes → both classified as clickhouse

Also fixed: Updated clearWarehouseEnvVars() test helper to clear all 8 CLICKHOUSE_* env vars, ensuring test isolation.

Type of change

• New feature (non-breaking change which adds functionality)

Issue for this PR

N/A — proactive test coverage for ClickHouse driver added in #574

How did you verify your code works?

bun test test/tool/project-scan.test.ts    # 77 pass (73 existing + 4 new)
bun run script/upstream/analyze.ts --markers --base main --strict   # ok

Checklist

• I have added tests that prove my fix is effective or that my feature works
• New and existing unit tests pass locally with my changes

https://claude.ai/code/session_014pS7RRkb53MH36pc6qhSBT

Summary by CodeRabbit

Release Notes

• Tests
  • Expanded test coverage for ClickHouse database detection across multiple environment variable formats.
  • Added validation for proper environment variable masking in ClickHouse configuration detection scenarios.

[claude]

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.}

[coderabbitai]

📝 Walkthrough

Walkthrough

The test suite for detectEnvVars adds four new test cases for ClickHouse detection and extends environment variable cleanup to include CLICKHOUSE_* keys. Changes verify ClickHouse detection via multiple environment variable formats and password masking behavior.

Changes

Cohort / File(s)	Summary
ClickHouse Detection Tests packages/opencode/test/tool/project-scan.test.ts	Extended warehouse env-var reset list to include CLICKHOUSE_* keys. Added four test cases validating ClickHouse detection via CLICKHOUSE_HOST, CLICKHOUSE_URL, and DATABASE_URL with ClickHouse schemes, including password masking verification.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Hop, hop, hooray! The ClickHouse tests now shine so bright,
With passwords masked and URLs caught just right,
New test cases dance in the warehouse's glow,
Four scenarios deep for the detectors to know! 🏠✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly and specifically describes the main change: adding tests for ClickHouse env var detection and DATABASE_URL scheme parsing.
Description check	✅ Passed	The description covers all required template sections with substantial detail: explains what changed (4 new tests), why it matters (prevents regressions), test scenarios covered, and verification results.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch test/hourly-20260402-1909

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

packages/opencode/test/tool/project-scan.test.ts (1)

550-561: Consider adding connection_string masking assertion for consistency.

The clickhouse+http/clickhouse+https test validates type and signal but doesn't assert connection_string masking like the clickhouse:// scheme test does (line 547). This is optional since the masking behavior is uniform across all DATABASE_URL paths, but adding it would make the test more complete.

💡 Optional enhancement

       const ch = result.find((r) => r.type === "clickhouse")
       expect(ch).toBeDefined()
       expect(ch!.signal).toBe("DATABASE_URL")
       expect(ch!.type).toBe("clickhouse")
+      expect(ch!.config.connection_string).toBe("***")
     }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@packages/opencode/test/tool/project-scan.test.ts` around lines 550 - 561, The
test "detects ClickHouse via DATABASE_URL with clickhouse+http and
clickhouse+https schemes" checks type and signal but omits asserting the masked
connection string; update the loop in that test to also assert the masked
connection string returned by detectEnvVars: after finding ch (const ch =
result.find((r) => r.type === "clickhouse")), add an expectation like
expect(ch!.connection_string).toBe(`${scheme}://default:***@clickhouse.example.com:8443/analytics`)
so the masking behavior for clickhouse+http and clickhouse+https matches the
existing clickhouse:// test.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@packages/opencode/test/tool/project-scan.test.ts`:
- Around line 550-561: The test "detects ClickHouse via DATABASE_URL with
clickhouse+http and clickhouse+https schemes" checks type and signal but omits
asserting the masked connection string; update the loop in that test to also
assert the masked connection string returned by detectEnvVars: after finding ch
(const ch = result.find((r) => r.type === "clickhouse")), add an expectation
like
expect(ch!.connection_string).toBe(`${scheme}://default:***@clickhouse.example.com:8443/analytics`)
so the masking behavior for clickhouse+http and clickhouse+https matches the
existing clickhouse:// test.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 6d67e099-ebd6-4af2-ab3f-6578e069d12e

📥 Commits

Reviewing files that changed from the base of the PR and between 0d34855 and 6fa086c.

📒 Files selected for processing (1)

• packages/opencode/test/tool/project-scan.test.ts

[dev-punia-altimate]

❌ Tests — Failures Detected

TypeScript — 15 failure(s)

• connection_refused [2.69ms]
• timeout [2.35ms]
• permission_denied [2.69ms]
• parse_error [2.41ms]
• oom [2.62ms]
• network_error [2.50ms]
• auth_failure [2.74ms]
• rate_limit [2.88ms]
• internal_error [2.87ms]
• empty_error [0.27ms]
• connection_refused [0.15ms]
• timeout [0.08ms]
• permission_denied [0.07ms]
• parse_error [0.07ms]
• oom [0.08ms]

cc @anandgupta42
_{Tested at 6fa086c0 | Run log | Powered by QA Autopilot}