Follow-up from code review on the MCP server feature (#50).
Code-quality polish. None of these affect functionality; all are <1-hour cleanups whenever the file is next touched.
- Access-denied returns HTML; rest of OAuth uses 302 with
error= codes (mcp/src/oauth/github.ts:112-124). Inconsistent. Pick one - either HTML for the user-facing 403, or RFC 6749 4.1.2.1-style redirect with ?error=access_denied.
- ApiError rendered string includes HTTP status twice (
mcp/src/tools/result.ts:42-46). e.g. "list_tasks failed (HTTP 404): HTTP 404 Not Found: ...". Just log e.message (already self-describing) or extract the body and render once.
- Non-null assertions after a missing-field filter check (
mcp/src/oauth/authorize.ts:65,70,83-87 and mcp/src/oauth/token.ts). Pattern works but is fragile - reordering the validation drops type safety. Replace with explicit destructuring + null checks so TS narrows naturally.
Source: review of feature/mcp-server-#50.
Follow-up from code review on the MCP server feature (#50).
Code-quality polish. None of these affect functionality; all are <1-hour cleanups whenever the file is next touched.
error=codes (mcp/src/oauth/github.ts:112-124). Inconsistent. Pick one - either HTML for the user-facing 403, or RFC 6749 4.1.2.1-style redirect with?error=access_denied.mcp/src/tools/result.ts:42-46). e.g."list_tasks failed (HTTP 404): HTTP 404 Not Found: ...". Just loge.message(already self-describing) or extract the body and render once.mcp/src/oauth/authorize.ts:65,70,83-87andmcp/src/oauth/token.ts). Pattern works but is fragile - reordering the validation drops type safety. Replace with explicit destructuring + null checks so TS narrows naturally.Source: review of feature/mcp-server-#50.