diff --git a/.plans/01-shared-model-normalization.md b/.plans/01-shared-model-normalization.md new file mode 100644 index 0000000000000000000000000000000000000000..e1e5f77db484aaaff5d3fba55e6f38a2a3d8b51c --- /dev/null +++ b/.plans/01-shared-model-normalization.md @@ -0,0 +1,42 @@ +# Plan: Centralize Model Normalization in Contracts + +## Summary +Move model alias/default normalization into `packages/contracts` so desktop and renderer use one shared source of truth. + +## Motivation +- Removes duplicated logic between: + - `apps/desktop/src/codexAppServerManager.ts` + - `apps/renderer/src/model-logic.ts` +- Prevents behavior drift when model aliases/defaults are updated. + +## Scope +- Add shared model utilities to contracts. +- Update desktop and renderer to consume shared utilities. +- Keep renderer-specific display options in renderer. + +## Proposed Changes +1. Add `packages/contracts/src/model.ts` with: + - Canonical model list + - Alias map + - `normalizeModelSlug` + - `resolveModelSlug` + - `DEFAULT_MODEL` +2. Export model utilities from `packages/contracts/src/index.ts`. +3. Update `apps/desktop/src/codexAppServerManager.ts` to replace local alias map/helper. +4. Update `apps/renderer/src/model-logic.ts` to wrap or re-export shared functions. +5. Update tests: + - Move/duplicate normalization tests to contracts. + - Keep renderer tests focused on renderer-only behavior. + +## Risks +- Desktop/renderer may currently rely on slightly different fallback behavior. +- Import graph must avoid bundling issues for Electron main/preload. + +## Validation +- `bun run test` +- `bun run typecheck` +- Manual check that model selection and session start still send expected model slug. + +## Done Criteria +- No duplicated alias/default map in desktop and renderer. +- Shared model utilities are contract-tested. diff --git a/.plans/02-typed-ipc-boundaries.md b/.plans/02-typed-ipc-boundaries.md new file mode 100644 index 0000000000000000000000000000000000000000..4e910043561a3b6aa9414722562076d0cb0f1670 --- /dev/null +++ b/.plans/02-typed-ipc-boundaries.md @@ -0,0 +1,37 @@ +# Plan: Strengthen Typed IPC Boundaries in Main Process + +## Summary +Replace loose payload casting in IPC handlers with strict schema parsing and typed helper wrappers. + +## Motivation +- `apps/desktop/src/main.ts` currently uses casts like `payload as Parameters<...>`. +- Casts can hide contract breakages until runtime. + +## Scope +- Desktop main process IPC registration. +- Optional shared helper for handler registration. + +## Proposed Changes +1. Add IPC helper utility (e.g. `apps/desktop/src/ipcHelpers.ts`) to: + - Parse payload(s) with Zod schemas + - Standardize typed handler signatures +2. Refactor provider IPC handlers in `apps/desktop/src/main.ts` to use: + - `providerSessionStartInputSchema.parse` + - `providerSendTurnInputSchema.parse` + - `providerInterruptTurnInputSchema.parse` + - `providerStopSessionInputSchema.parse` +3. Apply same pattern to agent/terminal handlers where possible. +4. Add tests for handler parsing failure paths (invalid payloads). + +## Risks +- Refactor can subtly change IPC error shape/messages. +- Helper abstraction should stay simple and not obscure control flow. + +## Validation +- `bun run test` +- `bun run typecheck` +- Manual invalid payload check from renderer/devtools to confirm fast failure. + +## Done Criteria +- No provider handler uses `payload as Parameters<...>`. +- All IPC entrypoints parse unknown payloads at boundary. diff --git a/.plans/03-split-codex-app-server-manager.md b/.plans/03-split-codex-app-server-manager.md new file mode 100644 index 0000000000000000000000000000000000000000..e425a6ae7d2c400cff64eff7bb35e68664c9be5d --- /dev/null +++ b/.plans/03-split-codex-app-server-manager.md @@ -0,0 +1,41 @@ +# Plan: Decompose CodexAppServerManager + +## Summary +Split `CodexAppServerManager` into smaller modules with clear responsibilities. + +## Motivation +- `apps/desktop/src/codexAppServerManager.ts` is large and mixes: + - Process lifecycle + - JSON-RPC parsing/routing + - Session state transitions + - Event emission +- This increases regression risk and slows changes. + +## Scope +- Desktop provider internals only. +- Keep external behavior/API stable. + +## Proposed Changes +1. Extract modules: + - `codex/processLifecycle.ts` + - `codex/jsonrpcRouter.ts` + - `codex/sessionState.ts` + - `codex/parsing.ts` +2. Keep `CodexAppServerManager` as thin orchestrator/facade. +3. Move pure helpers (`classifyCodexStderrLine`, route parsing) into unit-testable files. +4. Add targeted unit tests for: + - Message classification + - Request/notification/response routing + - Session state transitions + +## Risks +- Reordering event handling can change behavior. +- Must preserve pending request timeout/cancellation semantics. + +## Validation +- Existing tests pass. +- Add module-level tests for parsing and transition logic. + +## Done Criteria +- Main manager file materially smaller and orchestration-focused. +- Core protocol/state logic covered by focused tests. diff --git a/.plans/04-split-chatview-component.md b/.plans/04-split-chatview-component.md new file mode 100644 index 0000000000000000000000000000000000000000..3d6f949e9c5cce498fd6294d7d898361399b557b --- /dev/null +++ b/.plans/04-split-chatview-component.md @@ -0,0 +1,40 @@ +# Plan: Split ChatView into Smaller UI/Logic Units + +## Summary +Refactor `ChatView.tsx` into composable pieces with isolated responsibilities. + +## Motivation +- `apps/renderer/src/components/ChatView.tsx` is large and handles: + - Session orchestration + - Send/interrupt actions + - Timeline rendering + - Header/status UI + - Composer UI +- Hard to test and maintain as one component. + +## Scope +- Renderer component boundaries and hooks. +- Keep visual behavior unchanged. + +## Proposed Changes +1. Create hook: `apps/renderer/src/hooks/useChatSession.ts` + - `ensureSession` + - `sendTurn` + - `interruptTurn` +2. Split presentational components: + - `components/chat/ThreadHeader.tsx` + - `components/chat/MessageTimeline.tsx` + - `components/chat/ComposerBar.tsx` +3. Keep `ChatView.tsx` as container wiring store + hook + child components. +4. Add focused tests for hook behavior (error handling, session reuse). + +## Risks +- Refactor can break subtle UI interactions (auto-scroll, menu close, keyboard send). + +## Validation +- `bun run test` +- Manual smoke: send, stream, interrupt, model switch. + +## Done Criteria +- `ChatView.tsx` significantly reduced and easier to scan. +- Session logic isolated from rendering. diff --git a/.plans/05-zod-persisted-state-validation.md b/.plans/05-zod-persisted-state-validation.md new file mode 100644 index 0000000000000000000000000000000000000000..51fcf805d887924d0cf063780513021f5fac2429 --- /dev/null +++ b/.plans/05-zod-persisted-state-validation.md @@ -0,0 +1,34 @@ +# Plan: Move Renderer Persisted-State Validation to Zod + +## Summary +Use explicit Zod schemas for localStorage state parsing and migration. + +## Motivation +- `apps/renderer/src/store.ts` has large manual sanitize functions. +- Manual type guards are verbose and easier to get wrong during schema evolution. + +## Scope +- Renderer state hydration/persistence path. +- No backend/protocol changes. + +## Proposed Changes +1. Add schema module: `apps/renderer/src/persistenceSchema.ts` + - Persisted payload versions (`v1`, `v2`) + - Thread/message/project schemas +2. Replace `sanitizeProjects/sanitizeThreads/sanitizeMessages` with schema parsing + transforms. +3. Keep migration logic explicit (legacy model migration and key migration). +4. Add tests for: + - Invalid payload fallback to initial state + - Legacy payload migration + - Unknown thread/project references filtered + +## Risks +- Overly strict schemas could drop valid historical data unexpectedly. + +## Validation +- Unit tests for migration/hydration. +- Manual reload test with existing localStorage data. + +## Done Criteria +- Store hydration logic is schema-driven. +- Migration behavior is tested and documented. diff --git a/.plans/06-provider-logstream-lifecycle.md b/.plans/06-provider-logstream-lifecycle.md new file mode 100644 index 0000000000000000000000000000000000000000..71d1cbd95bd17339827d4c358bd4576ca9011189 --- /dev/null +++ b/.plans/06-provider-logstream-lifecycle.md @@ -0,0 +1,31 @@ +# Plan: Add Provider Log Stream Lifecycle Management + +## Summary +Ensure `ProviderManager` logging stream is initialized, rotated/structured, and closed safely. + +## Motivation +- `apps/desktop/src/providerManager.ts` opens a write stream in constructor. +- Stream lifecycle is not explicit on shutdown. + +## Scope +- Desktop provider logging behavior. +- App shutdown integration. + +## Proposed Changes +1. Add explicit `dispose()` on `ProviderManager`: + - Remove event listeners + - End/close log stream +2. Call `providerManager.dispose()` from app shutdown path in `apps/desktop/src/main.ts`. +3. Optional: change log format to JSON lines with stable fields. +4. Optional: per-session log files under `.logs/providers/`. + +## Risks +- Improper close sequencing may lose final log lines. + +## Validation +- Manual run/quit cycle to ensure no open handle warnings. +- Confirm logs flush on quit and file descriptors are not leaked. + +## Done Criteria +- ProviderManager owns complete log stream lifecycle. +- Shutdown path explicitly disposes provider resources. diff --git a/.plans/07-ci-quality-gates.md b/.plans/07-ci-quality-gates.md new file mode 100644 index 0000000000000000000000000000000000000000..5ebcb0448793b2bd4663e334318c7ea68d61e9ae --- /dev/null +++ b/.plans/07-ci-quality-gates.md @@ -0,0 +1,34 @@ +# Plan: Add CI Workflow for Core Quality Gates + +## Summary +Add GitHub Actions workflow to run lint/typecheck/test (and optionally smoke-test) on pushes and PRs. + +## Motivation +- Repository currently has no CI workflow files. +- Quality checks are only local/manual. + +## Scope +- `.github/workflows/ci.yml` +- Bun + Turbo setup in CI. + +## Proposed Changes +1. Add `ci.yml` with jobs: + - Setup Bun and Node environment + - Install deps + - `bun run lint` + - `bun run typecheck` + - `bun run test` +2. Add separate optional job for `bun run smoke-test` (desktop/Electron). +3. Configure caching for Bun/Turbo as appropriate. + +## Risks +- Smoke test may be flaky in headless CI environments. +- CI runtime can grow if caching is misconfigured. + +## Validation +- Verify workflow runs on a branch PR. +- Ensure failures surface clearly by job name. + +## Done Criteria +- CI blocks regressions in lint/typecheck/test. +- Workflow docs added to README. diff --git a/.plans/08-precommit-format-and-lint.md b/.plans/08-precommit-format-and-lint.md new file mode 100644 index 0000000000000000000000000000000000000000..e892892781d592a1f6e055280a8504a5ed8f4249 --- /dev/null +++ b/.plans/08-precommit-format-and-lint.md @@ -0,0 +1,32 @@ +# Plan: Add Pre-Commit Formatting/Lint Hooks + +## Summary +Introduce pre-commit automation so formatting and basic lint checks happen before commits. + +## Motivation +- Current lint failures include formatting-only issues. +- Shift-left feedback reduces noisy CI failures and cleanup churn. + +## Scope +- Root tooling config and package scripts. +- No runtime code changes. + +## Proposed Changes +1. Add hook tooling (e.g. Husky + lint-staged or Lefthook). +2. Configure staged-file tasks: + - `biome format --write` + - `biome check` +3. Add setup docs in README. +4. Keep checks fast to avoid developer friction. + +## Risks +- Slow hooks can frustrate contributors and be bypassed. +- Need to ensure compatibility with Bun workspace setup. + +## Validation +- Create sample staged changes and verify hook behavior. +- Confirm formatting fixes are applied automatically. + +## Done Criteria +- Pre-commit hook installed and documented. +- Formatting-only lint failures drop significantly. diff --git a/.plans/09-event-state-test-expansion.md b/.plans/09-event-state-test-expansion.md new file mode 100644 index 0000000000000000000000000000000000000000..226ebf4cdabf5abcca3bb6d8c0f742befbcc7707 --- /dev/null +++ b/.plans/09-event-state-test-expansion.md @@ -0,0 +1,35 @@ +# Plan: Expand Event/State Transition Test Coverage + +## Summary +Add focused tests for renderer event handling and session evolution logic. + +## Motivation +- Core behavior is event-driven and stateful. +- Existing renderer tests cover only a subset of timeline/model behavior. + +## Scope +- `apps/renderer/src/session-logic.test.ts` +- Optional reducer tests for `apps/renderer/src/store.ts`. + +## Proposed Changes +1. Add tests for `evolveSession`: + - `thread/started` + - `turn/started` + - `turn/completed` success/failure + - error/session closed events +2. Add tests for `applyEventToMessages`: + - start/delta/completed flow + - out-of-order event cases + - turn completion clearing streaming flags +3. Add reducer integration tests for `APPLY_EVENT`. + +## Risks +- Tests may be brittle if event payload fixtures are too coupled to implementation details. + +## Validation +- `bun run test` +- Ensure new tests remain deterministic and fast. + +## Done Criteria +- High-risk event transitions are covered by unit tests. +- Regressions in stream assembly/session status are caught quickly. diff --git a/.plans/10-unify-process-session-abstraction.md b/.plans/10-unify-process-session-abstraction.md new file mode 100644 index 0000000000000000000000000000000000000000..c3feec020f24ca21b31b265adbce208e502f9f78 --- /dev/null +++ b/.plans/10-unify-process-session-abstraction.md @@ -0,0 +1,35 @@ +# Plan: Unify Process and PTY Session Abstractions in ProcessManager + +## Summary +Refactor `ProcessManager` to use a single runtime-session interface for child-process and PTY modes. + +## Motivation +- `apps/desktop/src/processManager.ts` maintains parallel maps and branch-heavy logic. +- New execution backends/providers will multiply complexity. + +## Scope +- Desktop process execution internals. +- Preserve public `ProcessManager` API. + +## Proposed Changes +1. Introduce internal interface (e.g. `RuntimeSession`): + - `write(data)` + - `kill()` + - lifecycle/output event hooks +2. Implement: + - `ChildProcessSession` + - `PtySession` +3. Replace dual maps with one `Map`. +4. Keep output/exit event contract unchanged. +5. Add tests for both implementations. + +## Risks +- PTY behavior differs by platform; abstraction must not hide required differences. + +## Validation +- Existing `processManager.test.ts` passes. +- Add PTY-path tests where feasible. + +## Done Criteria +- Manager no longer branches per backend in `write/kill/killAll`. +- Session backends are independently testable. diff --git a/.plans/README.md b/.plans/README.md new file mode 100644 index 0000000000000000000000000000000000000000..7bb69a3b9126ef4c40bbf74c161364f63ef49827 --- /dev/null +++ b/.plans/README.md @@ -0,0 +1,12 @@ +# Maintainability Plans + +1. `01-shared-model-normalization.md` +2. `02-typed-ipc-boundaries.md` +3. `03-split-codex-app-server-manager.md` +4. `04-split-chatview-component.md` +5. `05-zod-persisted-state-validation.md` +6. `06-provider-logstream-lifecycle.md` +7. `07-ci-quality-gates.md` +8. `08-precommit-format-and-lint.md` +9. `09-event-state-test-expansion.md` +10. `10-unify-process-session-abstraction.md` diff --git a/apps/renderer/src/components/ChatView.tsx b/apps/renderer/src/components/ChatView.tsx index ec4c5d45d515a5a2cc2717077db1e4f4c1f423fe..641bc58164693b300635b94eef7814838f40e207 100644 --- a/apps/renderer/src/components/ChatView.tsx +++ b/apps/renderer/src/components/ChatView.tsx @@ -16,6 +16,7 @@ import { } from "../model-logic"; import { derivePhase, + deriveTimelineEntries, deriveWorkLogEntries, formatElapsed, formatTimestamp, @@ -83,11 +84,18 @@ export default function ChatView() { } return map; }, [activeThread?.events]); - const recentWorkLogEntries = workLogEntries.slice(-12); + const timelineEntries = useMemo( + () => + deriveTimelineEntries( + activeThread?.messages ?? [], + isWorking ? workLogEntries : [], + ), + [activeThread?.messages, isWorking, workLogEntries], + ); // Auto-scroll on new messages const messageCount = activeThread?.messages.length ?? 0; - const workLogCount = recentWorkLogEntries.length; + const workLogCount = workLogEntries.length; // biome-ignore lint/correctness/useExhaustiveDependencies: trigger on message count change useEffect(() => { messagesEndRef.current?.scrollIntoView({ behavior: "smooth" }); @@ -341,73 +349,85 @@ export default function ChatView() { ) : (
- {activeThread.messages.map((msg) => ( -
- {msg.role === "user" ? ( -
-
- 
-                        {msg.text}
-
-

- {formatTimestamp(msg.createdAt)} -

-
-
- ) : ( -
- - {msg.streaming && ( -
- - - - - - - Thinking - -
- )} -

- {formatMessageMeta( - msg.createdAt, - msg.streaming - ? formatElapsed(msg.createdAt, nowIso) - : formatElapsed( - msg.createdAt, - assistantCompletionByItemId.get(msg.id), - ), - )} -

-
- )} -
- ))} - {/* Inline activity feed */} - {isWorking && ( -
- {recentWorkLogEntries.map((entry) => ( + {timelineEntries.map((timelineEntry) => + timelineEntry.kind === "work" ? ( +

- {entry.detail ? ( + {timelineEntry.entry.detail ? ( <> - {entry.label} + {timelineEntry.entry.label} - {entry.detail} + {timelineEntry.entry.detail} ) : ( - entry.label + timelineEntry.entry.label )}

- ))} +
+ ) : ( +
+ {timelineEntry.message.role === "user" ? ( +
+
+ 
+                          {timelineEntry.message.text}
+
+

+ {formatTimestamp(timelineEntry.message.createdAt)} +

+
+
+ ) : ( +
+ + {timelineEntry.message.streaming && ( +
+ + + + + + + Thinking + +
+ )} +

+ {formatMessageMeta( + timelineEntry.message.createdAt, + timelineEntry.message.streaming + ? formatElapsed( + timelineEntry.message.createdAt, + nowIso, + ) + : formatElapsed( + timelineEntry.message.createdAt, + assistantCompletionByItemId.get( + timelineEntry.message.id, + ), + ), + )} +

+
+ )} +
+ ), + )} + {isWorking && ( +
diff --git a/apps/renderer/src/session-logic.test.ts b/apps/renderer/src/session-logic.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..69b0f2dc925f41eff9e4293682d704cbd97bc9fd --- /dev/null +++ b/apps/renderer/src/session-logic.test.ts @@ -0,0 +1,75 @@ +import { describe, expect, it } from "vitest"; +import type { WorkLogEntry } from "./session-logic"; +import { deriveTimelineEntries } from "./session-logic"; +import type { ChatMessage } from "./types"; + +describe("deriveTimelineEntries", () => { + it("interleaves messages and work entries by timestamp", () => { + const messages: ChatMessage[] = [ + { + id: "m-user", + role: "user", + text: "Hi", + createdAt: "2026-02-08T10:00:00.000Z", + streaming: false, + }, + { + id: "m-assistant", + role: "assistant", + text: "Hello", + createdAt: "2026-02-08T10:05:00.000Z", + streaming: false, + }, + ]; + const workEntries: WorkLogEntry[] = [ + { + id: "w-1", + label: "Tool call", + createdAt: "2026-02-08T10:02:00.000Z", + tone: "tool", + }, + { + id: "w-2", + label: "Preamble", + createdAt: "2026-02-08T10:03:00.000Z", + tone: "thinking", + }, + ]; + + const timeline = deriveTimelineEntries(messages, workEntries); + + expect(timeline.map((entry) => entry.id)).toEqual([ + "message:m-user", + "work:w-1", + "work:w-2", + "message:m-assistant", + ]); + }); + + it("prefers work entries when timestamps are equal", () => { + const messages: ChatMessage[] = [ + { + id: "m-1", + role: "assistant", + text: "Done", + createdAt: "2026-02-08T10:00:00.000Z", + streaming: false, + }, + ]; + const workEntries: WorkLogEntry[] = [ + { + id: "w-1", + label: "Tool call", + createdAt: "2026-02-08T10:00:00.000Z", + tone: "tool", + }, + ]; + + const timeline = deriveTimelineEntries(messages, workEntries); + + expect(timeline.map((entry) => entry.id)).toEqual([ + "work:w-1", + "message:m-1", + ]); + }); +}); diff --git a/apps/renderer/src/session-logic.ts b/apps/renderer/src/session-logic.ts index 70e1c63a41d1a14171e5ada155df79c395f47bcc..ccba13b9e2c275d8c6456d550a24fd373a9376e6 100644 --- a/apps/renderer/src/session-logic.ts +++ b/apps/renderer/src/session-logic.ts @@ -72,6 +72,20 @@ export interface WorkLogEntry { tone: "thinking" | "tool" | "info" | "error"; } +export type TimelineEntry = + | { + id: string; + kind: "message"; + createdAt: string; + message: ChatMessage; + } + | { + id: string; + kind: "work"; + createdAt: string; + entry: WorkLogEntry; + }; + function truncateLine(value: string, limit = 140): string { if (value.length <= limit) return value; return `${value.slice(0, limit - 1)}…`; @@ -277,6 +291,75 @@ export function deriveWorkLogEntries( return entries; } +function toTimestamp(isoDate: string): number { + const parsed = Date.parse(isoDate); + return Number.isNaN(parsed) ? Number.POSITIVE_INFINITY : parsed; +} + +export function deriveTimelineEntries( + messages: ChatMessage[], + workEntries: WorkLogEntry[], +): TimelineEntry[] { + const timeline: TimelineEntry[] = []; + let messageIndex = 0; + let workIndex = 0; + + while (messageIndex < messages.length || workIndex < workEntries.length) { + const message = messages[messageIndex]; + const workEntry = workEntries[workIndex]; + + if (!message && workEntry) { + timeline.push({ + id: `work:${workEntry.id}`, + kind: "work", + createdAt: workEntry.createdAt, + entry: workEntry, + }); + workIndex += 1; + continue; + } + + if (!workEntry && message) { + timeline.push({ + id: `message:${message.id}`, + kind: "message", + createdAt: message.createdAt, + message, + }); + messageIndex += 1; + continue; + } + + if (!message || !workEntry) { + break; + } + + const messageAt = toTimestamp(message.createdAt); + const workAt = toTimestamp(workEntry.createdAt); + + if (workAt <= messageAt) { + timeline.push({ + id: `work:${workEntry.id}`, + kind: "work", + createdAt: workEntry.createdAt, + entry: workEntry, + }); + workIndex += 1; + continue; + } + + timeline.push({ + id: `message:${message.id}`, + kind: "message", + createdAt: message.createdAt, + message, + }); + messageIndex += 1; + } + + return timeline; +} + export function derivePhase(session: ProviderSession | null): SessionPhase { if (!session || session.status === "closed") return "disconnected"; if (session.status === "connecting") return "connecting";