fix: address Code Quality and Qodo comments on #161

[Minitour]

Summary

Addresses GitHub Code Quality and Qodo findings called out on the develop→main release PR (#161).

Changes

• Code Quality: remove unused imports in add-builders.ts, watch-project.ts, configure-routes.ts, mcp-meta-routes.ts
• Qodo: move statSync inside createWorkspaceSymlink try/catch
• Qodo: allow http://[::1] loopback in CORS (Bun keeps brackets in hostname)
• Qodo: accept localhost / IP / host:port for PAT auth via normalizeProviderHost

Test plan

• bun test src/server/__tests__/cors-origin.test.ts src/cli/commands/__tests__/auth-command.test.ts
• CI green on this PR

[qodo-free-for-open-source-projects]

PR Summary by Qodo

Fix PAT host validation, IPv6 loopback CORS, and symlink error handling

🐞 Bug fix 🧪 Tests 🕐 20-40 Minutes

AI Description

• Normalize and validate PAT provider hosts to allow localhost/IPs and optional ports.
• Allow IPv6 loopback origins (http://[::1]) in CORS checks.
• Improve symlink creation error handling and remove unused imports flagged by linters.

Diagram

graph TD
  A["CLI: authCommand"] --> B["normalizeProviderHost"] --> C["resolveTokenAuthTarget"] --> D["CapaDatabase"]
  E["Origin + allowed list"] --> F["isAllowedOrigin"] --> G["stripIpv6Brackets"]

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Centralize host normalization utilities

• ➕ Avoids duplicating IPv6 bracket-stripping across CLI auth and server CORS
• ➕ Creates a single source of truth for hostname/authority parsing rules
• ➕ Easier to extend later (e.g., additional loopback aliases or stricter DNS rules)
• ➖ Small additional refactor scope (new shared module + import churn)
• ➖ Requires deciding on shared package boundary (server vs cli vs shared)

2. Use a strict, well-tested authority parser library

• ➕ Reduces risk of edge cases in host:port and IPv6 parsing
• ➕ Can improve readability and long-term maintainability
• ➖ Adds a dependency for a relatively small validation need
• ➖ May not align with project dependency policy and Bun/Node compatibility expectations

Recommendation: The current approach (using WHATWG URL parsing with an injected scheme, plus explicit checks for loopback/IP/DNS) is pragmatic and keeps dependencies low. Consider a follow-up to centralize the IPv6 bracket normalization (and potentially other host parsing helpers) into a shared utility to prevent future drift between CLI auth and server CORS behavior.

Files changed (9) +155 / -15

Bug fix (3) +70 / -6

auth.tsNormalize/validate PAT provider host input (localhost, IPs, ports, IPv6) +59/-4

Normalize/validate PAT provider host input (localhost, IPs, ports, IPv6)

• Replaces strict domain-only validation with normalizeProviderHost, allowing localhost and IP literals with optional ports while rejecting schemes/paths. Updates error messaging and uses the normalized host throughout token target resolution and display.

src/cli/commands/auth.ts

symlink-workspace.tsMove statSync under try/catch for friendlier symlink errors +1/-1

Move statSync under try/catch for friendlier symlink errors

• Moves the directory detection (statSync) inside the try block so filesystem errors are caught and surfaced via the existing friendly error handler.

src/cli/utils/wrap/symlink-workspace.ts

cors-origin.tsAllow IPv6 loopback by normalizing bracketed hostnames +10/-1

Allow IPv6 loopback by normalizing bracketed hostnames

• Normalizes parsed hostnames by stripping IPv6 brackets so http://[::1]:port is recognized as loopback. Extends loopback allowlist to include ::1.

src/server/cors-origin.ts

Refactor (4) +3 / -8

add-builders.tsRemove unused path import +1/-1

Remove unused path import

• Drops an unused import to satisfy code quality checks.

src/cli/commands/add-builders.ts

watch-project.tsRemove unused existsSync import +1/-1

Remove unused existsSync import

• Drops an unused import flagged by code quality tooling.

src/cli/utils/wrap/watch-project.ts

configure-routes.tsRemove unused capabilities parser import +0/-1

Remove unused capabilities parser import

• Drops an unused import to address code quality findings.

src/server/configure-routes.ts

mcp-meta-routes.tsRemove unused skill-content imports +1/-5

Remove unused skill-content imports

• Reduces the skill-content imports to only the function used in this module.

src/server/mcp-meta-routes.ts

Tests (2) +82 / -1

auth-command.test.tsAdd tests for provider host normalization and localhost:port token storage +42/-1

Add tests for provider host normalization and localhost:port token storage

• Imports and exercises the new normalizeProviderHost helper, covering DNS hosts, localhost, IPv4/IPv6 literals, and optional ports. Adds an integration-style test ensuring tokens can be stored for a self-hosted localhost instance with a port.

src/cli/commands/tests/auth-command.test.ts

cors-origin.test.tsAdd CORS tests for IPv6 loopback and env allowlist +40/-0

Add CORS tests for IPv6 loopback and env allowlist

• Introduces unit tests verifying that localhost/127.0.0.1 and [::1] loopback origins are allowed over http, and that non-loopback origins are blocked unless explicitly included in CAPA_ALLOWED_ORIGINS.

src/server/tests/cors-origin.test.ts

[qodo-free-for-open-source-projects]

Code Review by Qodo

🐞 Bugs (1) 📘 Rule violations (0) 📜 Skill insights (0)

1. PAT host:port never matches 🐞 Bug ≡ Correctness

Description

normalizeProviderHost now normalizes self-hosted providers to include ports (e.g., localhost:8443)
and passes that through to be stored as integration.host, but AuthenticatedFetch.detectPlatform
compares only URL.hostname (no port) to integration.host, so the stored PAT is never selected and no
Authorization header is attached for those requests.

Code

src/cli/commands/auth.ts[R449-450]

+      const hostPart = ipVersion === 6 ? `[${hostname}]` : hostname;
+      return url.port ? `${hostPart}:${url.port}` : hostPart;

Evidence

The PR now returns/stores hosts including ports, but the fetch auth selection path compares only the
hostname (without port) to the stored host; when they differ, detection returns null and no auth
headers are added.

src/cli/commands/auth.ts[233-327]
src/cli/commands/auth.ts[414-457]
src/server/git-integration-manager.ts[201-233]
src/shared/authenticated-fetch.ts[41-66]
src/shared/authenticated-fetch.ts[173-189]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
Self-hosted PAT entries are stored with `host:port` (and possibly bracketed IPv6), but request-time detection uses `new URL(url).hostname` and compares it to `integration.host`. `hostname` excludes the port, so the integration lookup fails and the request proceeds unauthenticated.

### Issue Context
- `normalizeProviderHost()` now returns `host:port` when a port is provided, and `authCommand` stores that host for self-hosted PATs.
- `AuthenticatedFetch.getAuthHeaders()` returns `null` when `detectPlatform()` returns `null`, so no auth header is applied.

### Fix Focus Areas
- src/shared/authenticated-fetch.ts[41-63]
- src/shared/authenticated-fetch.ts[173-189]

### Suggested fix
Update `AuthenticatedFetch.detectPlatform()` to compare against a normalized authority that includes port, e.g.:
- Use `urlObj.host` (includes `:port`, and includes brackets for IPv6 in standard URL implementations) when matching `integration.host`.
- Optionally fall back to matching `urlObj.hostname` for backward compatibility with previously-stored entries that omitted ports.
- If you need to support runtimes where IPv6 bracket formatting differs, normalize both sides consistently before comparing.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

_{To customize comments, go to the Qodo configuration screen, or learn more in the docs.}