Phase 1: types, zod runtime validation, structured errors, auth restore bugfix

[devin-ai-integration]

Summary

Phase 1 of the redesign plan — type safety, runtime validation, and the silent-logout bugfix. Stacked on top of #35 (Phase 0); base will auto-rebase to main once #35 merges.

What changes:

1. Concrete types in src/types/ (problem, user, solution, errors, plus an index re-export). These describe the shapes returned by LeetcodeServiceInterface methods — the projected envelopes the service emits, not the raw upstream GraphQL payloads. Existing credentials.ts and submission.ts are unchanged.

2. Zod runtime validators in src/leetcode/schemas.ts for the four LeetCode response shapes the server actually parses today:

   • SubmitResponseSchema (POST /problems/<slug>/submit/)
   • CheckResponseSchema (GET /submissions/detail/<id>/check/)
   • QuestionIdResponseSchema (questionId GraphQL query)
   • ValidateCredentialsResponseSchema (userStatus GraphQL query)

   Each schema is .passthrough() so unexpected fields don't fail the parse. CheckResponseSchema is intentionally loose on status_msg, code_answer, and expected_answer because LeetCode legitimately omits / changes their shape between PENDING and SUCCESS states.

3. Tightened LeetcodeServiceInterface — every Promise<any> (15+ of them) replaced with concrete typed returns drawn from src/types/. Method names and arity unchanged; only return types tightened. fetchSolutionArticleDetail() now correctly returns SolutionArticleDetail | null (LeetCode actually returns null for unknown topicIds; the previous as ... | undefined was a lie).

4. LeetCodeGlobalService updated to satisfy the new interface:

   • All as any casts replaced with proper projection / coercion
   • Stringly-typed throws (new Error("Authentication required")) replaced with LeetCodeError(ErrorCode.AUTH_REQUIRED, …) on every authenticated path; LeetCodeError(ErrorCode.PROBLEM_NOT_FOUND, …) on missing problems
   • submitSolution() validates both SubmitResponse and CheckResponse via safeParse() and throws LeetCodeError(ErrorCode.UPSTREAM_PAYLOAD_INVALID, …) (with the zod issue list as .cause) on schema failure
   • getQuestionId() validates QuestionIdResponseSchema and surfaces a typed error instead of a stringified shape
   • validateCredentials() validates ValidateCredentialsResponseSchema — on schema failure logs a warning and returns null (preserves the previous null-on-failure contract for the auth path)
   • Nullable upstream fields (runtime_percentile, total_correct, etc.) projected via ?? undefined so consumers see number | undefined, not number | null | undefined

5. Silent-logout-on-restart bugfix (the §3.2.1 issue from the assessment):

   • src/auth/auth-flow.ts (new): restoreCredentials(service, storage) loads persisted credentials, calls service.validateCredentials() to confirm they're still good, and on success calls service.updateCredentials() to push them into the running service. Returns a typed RestoreOutcome ('no_credentials' | 'invalid' | 'restored') for logging. Never throws.
   • src/index.ts: invokes restoreCredentials(leetcodeService) at startup before tools/prompts are registered. Best-effort; any failure leaves the server unauthenticated as before.
   • src/mcp/tools/auth-tools.ts: in check_auth_status, when validation succeeds we now also call leetcodeService.updateCredentials() so the very next authenticated tool call works without a server restart.

What this fixes: Before this PR, after a server restart ~/.leetcode-mcp/credentials.json was loaded from disk and validated, the user was told they were "authenticated", but the cookies were never pushed into the in-memory Credential the LeetCode client reads from. Every authenticated tool call then failed with "Authentication required" until the user re-pasted their cookies.

Tests: 146 → 151 (5 new auth-flow tests). All 151 pass; test:types clean; npm audit reports 0; build clean.

Review & Testing Checklist for Human

• Spot-check that the new types in src/types/ look reasonable (in particular, SubmissionRow.id is string | number because leetcode-query types it as number while raw payloads can carry strings — verify this is what you want)
• Verify the auth-restore flow works end-to-end against your real LeetCode account: stop the server, restart it, run an authenticated tool (fetch_user_status) without re-pasting cookies, confirm it succeeds
• Confirm the LEETCODE_MCP_* defaults from the previous summary (none added in this PR — strict mode etc. land in later phases)
• Optional: test the upstream-payload-invalid path by mocking a malformed submit response and confirming LeetCodeError(UPSTREAM_PAYLOAD_INVALID, …) is thrown rather than a generic Error

Notes

• Stacked on top of Phase 0: npm audit fix + tests/e2e placeholder #35; merging Phase 0: npm audit fix + tests/e2e placeholder #35 first will auto-rebase this PR to main.
• No tools, prompts, or resources added or removed; no MCP wire-protocol behavior changed except the auth restore at startup.
• The drive-by prettier reformat of scripts/sync-version.cjs is in its own commit (chore: prettier sweep ...) so it can be reverted independently if undesired.
• Fixed two test mocks (tests/services/{problem,solution}-services.test.ts) where the old Promise<any> return masked accesses that didn't typecheck; one assertion changed from toBeNull() against a stale .toBeUndefined() typo to .toBeNull() matching what LeetCode actually returns.
• Phase 2 (real e2e harness over StdioClientTransport + nock) starts next.

Link to Devin session: https://app.devin.ai/sessions/d003a60939484686b2953ae32fe2794d
Requested by: @SPerekrestova

[devin-ai-integration]

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

• Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
• Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

• Disable automatic comment and CI monitoring

[SPerekrestova]

Review summary

Phase 1 mostly does what the description claims — concrete types replace any, zod gates the upstream parses, and the silent-logout-on-restart hole is plugged at startup. The auth-flow test suite exercises the new helper well. A few concrete issues below; the first three are worth fixing before merge.

---

1. LeetcodeServiceInterface and its implementation disagree on two signatures

• fetchUserAllSubmissions — interface declares lang?: string | null and status?: string | null (leetcode-service-interface.ts:72-73), implementation declares lang?: string and status?: string (leetcode-global-service.ts:115-116). A caller passing null per the interface is rejected by the implementation — the implementation type is strictly narrower than the contract.
• fetchUserProgressQuestionList — interface requires filters: { offset: number; limit: number; ... } (filters and both numbers required, leetcode-service-interface.ts:87-92), implementation accepts options?: { offset?: number; limit?: number; ... } (leetcode-global-service.ts:323-328). The default-coalescing in the body (options?.offset || 0) only does anything if the args are optional — under the interface as written the caller must always pass {offset, limit} even when they want defaults. Pick one and align both.

2. restoreCredentials swallows errors that can never escape

auth-flow.ts:45-56 wraps service.validateCredentials(...) in try/catch and maps any throw to {status: "invalid", reason: "expired"}. But the real LeetCodeGlobalService.validateCredentials already wraps everything in try { ... } catch { return null; } (leetcode-global-service.ts:705-707). The catch arm in restoreCredentials is dead code with the only existing implementation, and the auth-flow test at tests/auth/auth-flow.test.ts:78-94 only verifies that defensive code — i.e., it asserts behavior the production code path can't produce. Either remove the try/catch (rely on validateCredentials's string | null contract) or stop swallowing errors inside validateCredentials so the layered handling means something.

3. LeetCodeError.cause shadows the native ES2022 property instead of using it

src/types/errors.ts:39-49 redeclares public readonly cause?: unknown and assigns it manually rather than calling super(message, { cause }). With useDefineForClassFields (default for modern TS targets), the class field is installed on the instance via Object.defineProperty and shadows the inherited Error.cause. Tools/loggers that walk the standard chain (err.cause?.cause?...) will see whatever the field stores, but anything that relied on super(message, opts) setting it will be inconsistent. Prefer super(message, { cause }); this.code = code; and drop the field.

---

Smaller things

• fetchProblem returns Promise<Problem> but fetchProblemSimplified immediately checks if (!problem) throw PROBLEM_NOT_FOUND — meaning the upstream genuinely returns null and the tightened return type is a lie. Either narrow fetchProblem to Problem | null (mirroring the fetchSolutionArticleDetail cleanup the PR description calls out) or have fetchProblem itself throw on miss.
• fetchUserStatus (leetcode-global-service.ts:102-107) defaults username: "" and avatar: "" on null. With isSignedIn: false and username: "" consumers can't distinguish "signed out" from "signed in, no avatar". Marking these string | null in UserStatus would represent the upstream more honestly.
• restoreCredentials calls service.validateCredentials and then service.updateCredentials on the same csrf/session it just validated. The check_auth_status tool now does the same thing (auth-tools.ts:217-223). Worth pulling into a small helper to avoid the third copy when more entry points need it.
• submitSolution validates the submit and check payloads with safeParse, but getQuestionId does the same check inline rather than reusing the same pattern — minor consistency nit, not a bug.

Nothing security-sensitive jumped out — credentials are still short-lived, file mode is still 0o600, and zod parsing is passthrough so it can't widen a tainted field into a validated one.

---

Generated by Claude Code

[devin-ai-integration]

Thanks — all five concrete points addressed in three follow-up commits on this branch (a62a2e5, 5191a31, 6f3d1b3).

1. Interface vs impl signature drift — fixed both:

• fetchUserAllSubmissions: dropped | null from the interface for lang / status. No callers ever pass null and the impl signature was the narrower of the two; this aligns by widening neither side.
• fetchUserProgressQuestionList: tightened the impl to require filters: { offset: number; limit: number; ... } matching the interface. The single caller (get_user_progress_questions) zod-defaults offset/limit, so they're always passed. Renamed the local filters to upstreamFilters so the parameter name follows the interface.

2. Dead try/catch in restoreCredentials — removed. Went with the "trust the contract" arm of your alternative: validateCredentials is Promise<string | null> per the interface, so restoreCredentials no longer wraps it. If a future impl violates the contract by throwing, the error now propagates and we get a real signal instead of a silent "expired" lie. The validate→updateCredentials sequence is also extracted into applyValidatedCredentials(service, csrf, session) so restoreCredentials and check_auth_status share one path (covers your "third copy" concern preemptively). The dead test was replaced with one asserting exceptions propagate, plus two unit tests for the new helper.

3. LeetCodeError.cause shadowing — fixed. Now uses super(message, cause === undefined ? undefined : { cause }) and drops the redeclared field. Tested mentally against both useDefineForClassFields: true (default for ES2022 lib) and false; in both cases err.cause now refers to the native chain.

Smaller items:

• fetchProblem return type lying about null — fixed. fetchProblem now throws LeetCodeError(PROBLEM_NOT_FOUND, …) itself, so the signature Promise<Problem> is honest and fetchProblemSimplified no longer needs its own null check. Updated the interface JSDoc to declare @throws LeetCodeError(PROBLEM_NOT_FOUND). The other consumer (problem-resources.ts) already wraps in try/catch, so the contract change is non-disruptive.

• fetchUserStatus returning "" for null — fixed. UserStatus.username and UserStatus.avatar are now string | null and the impl returns null instead of "". Consumers that JSON.stringify the envelope are unaffected; consumers reading the field can now distinguish the two states.

• Helper extraction — done as part of (2) above. applyValidatedCredentials lives in src/auth/auth-flow.ts next to restoreCredentials. save_leetcode_credentials was left alone because its flow inserts a disk-write between validate and updateCredentials, which doesn't fit the helper's shape — but it's already only one of three call sites instead of three.

• getQuestionId consistency — looked at it; it already uses the same safeParse + LeetCodeError(UPSTREAM_PAYLOAD_INVALID) pattern as submitSolution. Left as-is; happy to factor a parseUpstream<T>(schema, payload, contextLabel) wrapper in Phase 8 (the per-domain client split) where there'll be more call sites to share it across.

CI: 153/153 passing (was 151; +2 for applyValidatedCredentials), test:types clean, build clean. npm audit now flags one new transitive hono advisory that landed since Phase 0 — handling that in a separate hygiene PR rather than mixing with this review iteration.