Skip to content

wave10: config hygiene (#97, #98) - #124

Merged
AndresL230 merged 8 commits into
mainfrom
wave10/config-hygiene
May 24, 2026
Merged

wave10: config hygiene (#97, #98)#124
AndresL230 merged 8 commits into
mainfrom
wave10/config-hygiene

Conversation

@AndresL230

@AndresL230 AndresL230 commented May 24, 2026

Copy link
Copy Markdown
Contributor

Closes #97
Closes #98

Summary

Both fixes are orthogonal and additive. Format ^local- regex still matches the new scanId (verified no callsite splits on the inner structure).

Test plan

  • npm run test:scanner — existing suite + new config (3 cases: defaults, env override, whitespace fallback) + new scan-id (3 cases: format regex, 1000-call uniqueness, monotonic timestamp) all pass.
  • npm run build — dashboard + webview + extension build clean.
  • Grep gate — api.recost.dev / recost.dev/dashboard only in src/config.ts (+ one JSDoc in registry.ts, out of scope); local-${Date.now()} only in src/scan-id.ts:11 (the definition).
  • D1 benchmark Δ +0.00pp sanity (no scanner change expected).
  • EDH manual gate — confirm status bar + "Get a key" link + dashboard link all still resolve to production URLs (no visual change expected).

Coexistence with Wave 8 (PR #123)

Wave 8 modifies the same four production files. If Wave 8 merges first, this branch needs a rebase that:

  1. Resolves merge conflicts in the four shared files.
  2. Migrates the 8th local-${Date.now()} site Wave 8 added in the 429 branch of scan-publishing-handler.ts to newLocalScanId() (import already present).

The grep gate will catch the 8th site automatically if missed.

Plan: docs/superpowers/plans/2026-05-23-wave10-config-hygiene.md. Design spec: docs/superpowers/specs/2026-05-23-wave10-config-hygiene-design.md.

🤖 Generated with Claude Code

Summary by CodeRabbit

  • Documentation

    • Added Wave 10 config hygiene implementation plan and design specification documents.
  • Refactor

    • Introduced configurable API and dashboard base URLs across the codebase.
    • Improved local scan ID generation algorithm to reduce collision risk.
  • Tests

    • Added configuration and scan ID validation tests.
  • Chores

    • Updated test scripts and TypeScript compiler configuration for new test modules.

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 24, 2026

Copy link
Copy Markdown

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

This PR implements Wave 10 "Config Hygiene," addressing two defects: hard-coded ReCost base URLs scattered across multiple call sites and timestamp-only local scan ID generation with a 1ms collision window. It introduces two new modules (src/config.ts and src/scan-id.ts), updates eight call sites across the extension, CLI, and webview code, adds comprehensive tests with environment and uniqueness validation, and updates build configuration.

Changes

Wave 10 Config Hygiene

Layer / File(s) Summary
Config module and base URL centralization
src/config.ts, src/api-client.ts, src/chat/providers/eco.ts, src/extension.ts, src/webview-provider.ts
src/config.ts exports RECOST_API_BASE_URL and RECOST_DASHBOARD_BASE_URL as environment-overridable constants with production defaults; four call sites replace hard-coded https://api.recost.dev and https://recost.dev literals with imports from the new module.
Scan ID collision prevention
src/scan-id.ts, src/cli/scan.ts, src/webview/chat-handler.ts, src/webview/scan-publishing-handler.ts
src/scan-id.ts exports newLocalScanId() generating IDs as local-<Date.now()>-<8-char UUID suffix> to eliminate 1ms timestamp collisions; five call sites across CLI and webview handlers replace local-${Date.now()} with the new collision-resistant generator.
Config and scan ID test suites
src/test/config.test.ts, src/test/scan-id.test.ts
Subprocess-based config test validates environment override, default fallback, and whitespace handling; scan ID test verifies format, uniqueness across 1000 calls, and non-decreasing timestamp progression.
Build and test infrastructure
package.json, tsconfig.scanner-tests.json
TypeScript test configuration now includes new config and scan-id modules; npm test:scanner script appends invocations for both new test files.
Planning and design documentation
docs/superpowers/specs/2026-05-23-wave10-config-hygiene-design.md, docs/superpowers/plans/2026-05-23-wave10-config-hygiene.md
Design spec and implementation plan document approach, call-site migrations, test expectations, and verification gates for both config centralization and scan ID collision prevention features.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Scattered URLs once wild and free,
Now cozy in one config tree.
Timestamps ceased their collisions fond,
Crypto brought peace, with random wand.
Wave 10 brings hygiene, clean and bright!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 16.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'wave10: config hygiene (#97, #98)' directly and clearly summarizes the main changes—centralizing ReCost base URLs and replacing local scanId generation—and references the two primary issues being closed.
Linked Issues check ✅ Passed All coding requirements from both issues are met: issue #97's centralized config module (src/config.ts) with env-overridable RECOST_API_BASE_URL/RECOST_DASHBOARD_BASE_URL and five call-site migrations; issue #98's newLocalScanId() with timestamp-plus-entropy format replacing local-${Date.now()} across seven sites. Tests validate format, uniqueness, and monotonicity.
Out of Scope Changes check ✅ Passed All changes are tightly scoped to the two linked issues: config module creation, scanId generator, call-site migrations, test additions, and build config updates. No unrelated code refactoring or feature additions are present.

✏️ 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 wave10/config-hygiene

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 ESLint

If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.

ESLint skipped: no ESLint configuration detected in root package.json. To enable, add eslint to devDependencies.


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

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

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
src/config.ts (1)

4-8: ⚡ Quick win

Normalize trailing slashes for base URLs before export.

Line 5 and Line 8 trim whitespace but keep trailing /. With downstream string concatenation, env overrides like https://host/ become https://host//projects....

Suggested patch
 const PROD_API_BASE_URL = "https://api.recost.dev";
 const PROD_DASHBOARD_BASE_URL = "https://recost.dev";
 
+function resolveBaseUrl(value: string | undefined, fallback: string): string {
+  const trimmed = value?.trim();
+  if (!trimmed) return fallback;
+  return trimmed.replace(/\/+$/, "");
+}
+
 export const RECOST_API_BASE_URL =
-  process.env.RECOST_API_BASE_URL?.trim() || PROD_API_BASE_URL;
+  resolveBaseUrl(process.env.RECOST_API_BASE_URL, PROD_API_BASE_URL);
 
 export const RECOST_DASHBOARD_BASE_URL =
-  process.env.RECOST_DASHBOARD_BASE_URL?.trim() || PROD_DASHBOARD_BASE_URL;
+  resolveBaseUrl(process.env.RECOST_DASHBOARD_BASE_URL, PROD_DASHBOARD_BASE_URL);
🤖 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 `@src/config.ts` around lines 4 - 8, The exported RECOST_API_BASE_URL and
RECOST_DASHBOARD_BASE_URL currently trim whitespace but leave trailing slashes
which can cause double-slash concatenation; update the initialization of both
constants to remove any trailing slashes from the environment override (e.g.,
use process.env.RECOST_API_BASE_URL?.trim().replace(/\/+$/,'') ||
PROD_API_BASE_URL and similarly for RECOST_DASHBOARD_BASE_URL) so that
downstream concatenation won't produce duplicate slashes while preserving
protocol prefixes.
🤖 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.

Nitpick comments:
In `@src/config.ts`:
- Around line 4-8: The exported RECOST_API_BASE_URL and
RECOST_DASHBOARD_BASE_URL currently trim whitespace but leave trailing slashes
which can cause double-slash concatenation; update the initialization of both
constants to remove any trailing slashes from the environment override (e.g.,
use process.env.RECOST_API_BASE_URL?.trim().replace(/\/+$/,'') ||
PROD_API_BASE_URL and similarly for RECOST_DASHBOARD_BASE_URL) so that
downstream concatenation won't produce duplicate slashes while preserving
protocol prefixes.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 72ae64b2-c6fe-4ea6-81b6-80e7e15f856e

📥 Commits

Reviewing files that changed from the base of the PR and between aa2314c and 6fd2086.

📒 Files selected for processing (18)
  • .gitignore
  • docs/superpowers/plans/2026-05-23-wave10-config-hygiene.md
  • docs/superpowers/specs/2026-05-22-wave8-status-error-ux-design.md
  • docs/superpowers/specs/2026-05-23-wave10-config-hygiene-design.md
  • package.json
  • src/api-client.ts
  • src/chat/providers/eco.ts
  • src/cli/scan.ts
  • src/config.ts
  • src/extension.ts
  • src/scan-id.ts
  • src/test/config.test.ts
  • src/test/scan-id.test.ts
  • src/webview-provider.ts
  • src/webview/chat-handler.ts
  • src/webview/scan-publishing-handler.ts
  • tsconfig.scanner-tests.json
  • webview/tsconfig.tsbuildinfo
💤 Files with no reviewable changes (1)
  • webview/tsconfig.tsbuildinfo

@AndresL230
AndresL230 force-pushed the wave10/config-hygiene branch from 6fd2086 to 20a0771 Compare May 24, 2026 01:50
@AndresL230
AndresL230 merged commit 039fe4e into main May 24, 2026
2 of 3 checks passed
AndresL230 added a commit that referenced this pull request May 27, 2026
Wave 3 (#114/#115/#116, PR #126) merged; also corrected stale statuses for
waves 7 (#122), 8 (#123), 10 (#124) that merged earlier but were never marked
complete. All platform waves (6-10) + accuracy Wave 3 now shipped.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

1 participant