From 890708a55af88c7855ff7f50e06daf02b2949d44 Mon Sep 17 00:00:00 2001 From: Mohamed Boudra Date: Mon, 29 Dec 2025 00:19:30 +0700 Subject: [PATCH] Checkpoint typecheck and code quality review --- REPORT-typecheck-code-quality.md | 25 +++++++++++++++++++++++++ plan.md | 9 ++++++++- 2 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 REPORT-typecheck-code-quality.md diff --git a/REPORT-typecheck-code-quality.md b/REPORT-typecheck-code-quality.md new file mode 100644 index 000000000..6a240596d --- /dev/null +++ b/REPORT-typecheck-code-quality.md @@ -0,0 +1,25 @@ +# Typecheck + Code Quality Review + +## Typecheck +- Command: `npm run typecheck` +- Result: Success (server and app workspaces ran without errors). + +## Scope Reviewed +- `packages/app/src/app/agent/new.tsx` +- `packages/app/src/hooks/use-agent-form-state.ts` +- `packages/app/src/components/create-agent-modal.tsx` +- `packages/app/src/contexts/session-context.tsx` +- `packages/app/src/components/agent-stream-view.tsx` +- `packages/app/src/app/git-diff.tsx` +- `packages/server/src/server/agent/providers/claude-agent.ts` +- `packages/server/src/server/agent/providers/codex-mcp-agent.ts` +- `packages/server/src/server/daemon.e2e.test.ts` + +## Findings +1. URL params in `packages/app/src/app/agent/new.tsx` are cast to `AgentProvider` without validation. + - `resolvedProvider` is a string from `useLocalSearchParams`, then forced via `as AgentProvider` when building `initialValues`. + - Risk: invalid provider values (deep links, manual edits) bypass type checks and may put the form into an inconsistent state. + - Suggested fix: validate `resolvedProvider` against `providerDefinitions` (or a known provider list) before assigning, and drop invalid values. + +## Notes +- No new `any` usage introduced in the reviewed sections beyond existing patterns. diff --git a/plan.md b/plan.md index fae64385a..29583cc9e 100644 --- a/plan.md +++ b/plan.md @@ -94,13 +94,20 @@ Improvements to the new agent screen in the app. - Remove it from the UI. - **Done (2025-12-29 00:16)**: WHAT: `packages/app/src/app/git-diff.tsx:81-210` removes the Host label from the git diff header and error state; `packages/app/src/app/git-diff.tsx:253-307` drops the unused `metaText` style. RESULT: git diff screen no longer shows the host label. EVIDENCE: Not run (not requested). -- [ ] **Checkpoint**: Run typecheck and review code quality. +- [x] **Checkpoint**: Run typecheck and review code quality. - Run `npm run typecheck` and fix any errors. - Review all code changes made in this plan (Iterations 1-9). - Look for brittle TypeScript: `as` casts, `any` types, weakly typed code. - Ensure proper type inference and type safety. - Add fix tasks for any issues found. + - **Done (2025-12-29 00:18)**: WHAT: `plan.md:97` marked this checkpoint complete; `plan.md:106` added follow-up fix task for URL-param validation; `REPORT-typecheck-code-quality.md:1` documented typecheck + code-quality findings. RESULT: typecheck passed; identified URL-param cast risk and created fix task. EVIDENCE: `npm run typecheck` (workspace typecheck completed with exit code 0). + +- [ ] **Fix**: Validate URL-derived provider/mode/model on new agent screen. + + - Guard `useLocalSearchParams` values in `packages/app/src/app/agent/new.tsx`. + - Only accept provider/mode/model values that exist in current definitions/options. + - Drop invalid values instead of casting to `AgentProvider`. - [ ] **Checkpoint**: Review daemon test coverage for fixes in this plan.