diff --git a/TODO.md b/TODO.md index ee0f104d682f4ab5e21578ed963f217969cd3c14..e4b6e4b62be7fe06a1bffd01a8102e52bad217da 100644 --- a/TODO.md +++ b/TODO.md @@ -1,11 +1,9 @@ -- [x] Folder picker for new projects -- [x] Projects should be persisted so they're still visible after closing -- [x] Remove model selection from project creation -- [x] First message vanishes when thread is created?? Awful ux -- [x] Show more clearly when thinking is happening -- [x] Enable react compiler +## TODO -## Bigger overhauls - -- [x] Markdown rendering -- [x] Code block rendering +- [ ] Fix sort order for threads (newest first) +- [ ] Indicate in thread list which threads are currently working +- [ ] Logs should be per-thread, not a single events.txt +- [ ] Audit the logs to identify which events are (and aren't) worth showing to the user, i.e. drop "preamble" and "work" events +- [ ] Make the "end message" stand out from the work done prior (maybe just a bar between the two that says "worked for 10 minutes 31 seconds" or however long it was) +- [ ] Better logging of "time to complete" (I want this more than the timestamps tbh) +- [ ] Remove the left bars on messages to make UI feel "flatter" diff --git a/apps/desktop/src/codexAppServerManager.ts b/apps/desktop/src/codexAppServerManager.ts index 5b65d8b8589efc2a01a220e1bf226c39b86ae16f..c7a93c27860ee296ea1f03b00a1acd4ef4a496fc 100644 --- a/apps/desktop/src/codexAppServerManager.ts +++ b/apps/desktop/src/codexAppServerManager.ts @@ -10,6 +10,7 @@ import type { ProviderSessionStartInput, ProviderTurnStartResult, } from "@acme/contracts"; +import { normalizeModelSlug } from "@acme/contracts"; type PendingRequestKey = string; @@ -59,29 +60,20 @@ const BENIGN_ERROR_LOG_SNIPPETS = [ "state db missing rollout path for thread", "state db record_discrepancy: find_thread_path_by_id_str_in_subdir, falling_back", ]; -const MODEL_SLUG_ALIASES: Record = { - "5.3": "gpt-5.3-codex", - "gpt-5.3": "gpt-5.3-codex", -}; - export function normalizeCodexModelSlug( model: string | undefined | null, preferredId?: string, ): string | undefined { - if (typeof model !== "string") { + const normalized = normalizeModelSlug(model); + if (!normalized) { return undefined; } - const trimmed = model.trim(); - if (!trimmed) { - return undefined; - } - - if (preferredId?.endsWith("-codex") && preferredId !== trimmed) { + if (preferredId?.endsWith("-codex") && preferredId !== normalized) { return preferredId; } - return MODEL_SLUG_ALIASES[trimmed] ?? trimmed; + return normalized; } export function classifyCodexStderrLine( diff --git a/apps/desktop/src/main.ts b/apps/desktop/src/main.ts index ebfb867c98d2eb3a88c506954d1836fad95e8801..4f84aa55b7ce9d8ab7819524dd7b758726b2f7b3 100644 --- a/apps/desktop/src/main.ts +++ b/apps/desktop/src/main.ts @@ -314,6 +314,7 @@ async function bootstrap(): Promise { app.on("before-quit", () => { processManager.killAll(); providerManager.stopAll(); + providerManager.dispose(); }); app.whenReady().then(() => { diff --git a/apps/desktop/src/providerManager.test.ts b/apps/desktop/src/providerManager.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..ba44e312fdd93e8f37f0aea08ea0646e5aa42a04 --- /dev/null +++ b/apps/desktop/src/providerManager.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; + +import { ProviderManager } from "./providerManager"; + +describe("ProviderManager", () => { + it("detaches provider event listener and ends log stream on dispose", () => { + const manager = new ProviderManager(); + const internals = manager as unknown as { + codex: { listenerCount: (event: string) => number }; + logStream: { writableEnded: boolean; destroyed: boolean }; + }; + + expect(internals.codex.listenerCount("event")).toBe(1); + manager.dispose(); + + expect(internals.codex.listenerCount("event")).toBe(0); + expect( + internals.logStream.writableEnded || internals.logStream.destroyed, + ).toBe(true); + }); + + it("allows multiple dispose calls", () => { + const manager = new ProviderManager(); + + manager.dispose(); + expect(() => manager.dispose()).not.toThrow(); + }); +}); diff --git a/apps/desktop/src/providerManager.ts b/apps/desktop/src/providerManager.ts index 7083fda2ed7df835117dc562dea64033414b3444..68793311e96e761608b45cdd066fd3bc278d29d8 100644 --- a/apps/desktop/src/providerManager.ts +++ b/apps/desktop/src/providerManager.ts @@ -24,6 +24,15 @@ export interface ProviderManagerEvents { export class ProviderManager extends EventEmitter { private readonly codex = new CodexAppServerManager(); private readonly logStream: fs.WriteStream; + private disposed = false; + private readonly onCodexEvent = (event: ProviderEvent) => { + if (this.disposed) { + return; + } + + this.logStream.write(`${JSON.stringify(event)}\n`); + this.emit("event", event); + }; constructor() { super(); @@ -34,10 +43,7 @@ export class ProviderManager extends EventEmitter { flags: "a", }); - this.codex.on("event", (event) => { - this.logStream.write(`${JSON.stringify(event)}\n`); - this.emit("event", event); - }); + this.codex.on("event", this.onCodexEvent); } async startSession(raw: ProviderSessionStartInput): Promise { @@ -79,4 +85,14 @@ export class ProviderManager extends EventEmitter { stopAll(): void { this.codex.stopAll(); } + + dispose(): void { + if (this.disposed) { + return; + } + + this.disposed = true; + this.codex.off("event", this.onCodexEvent); + this.logStream.end(); + } } diff --git a/apps/renderer/src/model-logic.ts b/apps/renderer/src/model-logic.ts index 180c772afb866cf20813737bf36274c9fb49cfa5..6c2f3f5000b978dae0fea03ff097ea9866a6bf56 100644 --- a/apps/renderer/src/model-logic.ts +++ b/apps/renderer/src/model-logic.ts @@ -1,41 +1,9 @@ -export const MODEL_OPTIONS = [ - "gpt-5.3-codex", - "gpt-5.2-codex", - "gpt-5.2", -] as const; - -export const DEFAULT_MODEL = "gpt-5.3-codex"; +export { + DEFAULT_MODEL, + MODEL_OPTIONS, + normalizeModelSlug, + resolveModelSlug, +} from "@acme/contracts"; export const REASONING_OPTIONS = ["xhigh", "high", "medium", "low"] as const; export const DEFAULT_REASONING = "high"; - -const MODEL_SLUG_ALIASES: Record = { - "5.3": "gpt-5.3-codex", - "gpt-5.3": "gpt-5.3-codex", -}; - -export function normalizeModelSlug( - model: string | null | undefined, -): string | null { - if (typeof model !== "string") { - return null; - } - - const trimmed = model.trim(); - if (!trimmed) { - return null; - } - - return MODEL_SLUG_ALIASES[trimmed] ?? trimmed; -} - -export function resolveModelSlug(model: string | null | undefined): string { - const normalized = normalizeModelSlug(model); - if (!normalized) { - return DEFAULT_MODEL; - } - - return MODEL_OPTIONS.includes(normalized as (typeof MODEL_OPTIONS)[number]) - ? normalized - : DEFAULT_MODEL; -} diff --git a/apps/renderer/src/session-logic.test.ts b/apps/renderer/src/session-logic.test.ts index 69b0f2dc925f41eff9e4293682d704cbd97bc9fd..565d7283816f8d797ecc691f078461eb568e4e08 100644 --- a/apps/renderer/src/session-logic.test.ts +++ b/apps/renderer/src/session-logic.test.ts @@ -1,8 +1,39 @@ import { describe, expect, it } from "vitest"; -import type { WorkLogEntry } from "./session-logic"; -import { deriveTimelineEntries } from "./session-logic"; + +import type { ProviderEvent, ProviderSession } from "@acme/contracts"; +import { + type WorkLogEntry, + applyEventToMessages, + deriveTimelineEntries, + evolveSession, +} from "./session-logic"; import type { ChatMessage } from "./types"; +function makeEvent(overrides: Partial): ProviderEvent { + return { + id: "evt-1", + kind: "notification", + provider: "codex", + sessionId: "sess-1", + createdAt: "2026-02-08T10:00:00.000Z", + method: "noop", + ...overrides, + }; +} + +function makeSession( + overrides: Partial = {}, +): ProviderSession { + return { + sessionId: "sess-1", + provider: "codex", + status: "ready", + createdAt: "2026-02-08T09:59:00.000Z", + updatedAt: "2026-02-08T09:59:00.000Z", + ...overrides, + }; +} + describe("deriveTimelineEntries", () => { it("interleaves messages and work entries by timestamp", () => { const messages: ChatMessage[] = [ @@ -73,3 +104,242 @@ describe("deriveTimelineEntries", () => { ]); }); }); + +describe("evolveSession", () => { + it("updates thread id when thread starts", () => { + const previous = makeSession(); + const next = evolveSession( + previous, + makeEvent({ + method: "thread/started", + createdAt: "2026-02-08T10:01:00.000Z", + payload: { thread: { id: "thread-1" } }, + }), + ); + + expect(next.threadId).toBe("thread-1"); + expect(next.updatedAt).toBe("2026-02-08T10:01:00.000Z"); + }); + + it("moves to running and records active turn on turn start", () => { + const previous = makeSession(); + const next = evolveSession( + previous, + makeEvent({ + method: "turn/started", + turnId: "turn-1", + createdAt: "2026-02-08T10:02:00.000Z", + }), + ); + + expect(next.status).toBe("running"); + expect(next.activeTurnId).toBe("turn-1"); + expect(next.updatedAt).toBe("2026-02-08T10:02:00.000Z"); + }); + + it("returns to ready and clears active turn on successful completion", () => { + const previous = makeSession({ + status: "running", + activeTurnId: "turn-1", + lastError: "older error", + }); + const next = evolveSession( + previous, + makeEvent({ + method: "turn/completed", + createdAt: "2026-02-08T10:03:00.000Z", + payload: { turn: { id: "turn-1", status: "completed" } }, + }), + ); + + expect(next.status).toBe("ready"); + expect(next.activeTurnId).toBeUndefined(); + expect(next.lastError).toBe("older error"); + }); + + it("marks session as error when turn fails", () => { + const previous = makeSession({ + status: "running", + activeTurnId: "turn-1", + }); + const next = evolveSession( + previous, + makeEvent({ + method: "turn/completed", + createdAt: "2026-02-08T10:03:00.000Z", + payload: { + turn: { id: "turn-1", status: "failed", error: { message: "boom" } }, + }, + }), + ); + + expect(next.status).toBe("error"); + expect(next.activeTurnId).toBeUndefined(); + expect(next.lastError).toBe("boom"); + }); + + it("moves to error on runtime error events", () => { + const previous = makeSession(); + const next = evolveSession( + previous, + makeEvent({ + kind: "error", + method: "runtime/error", + createdAt: "2026-02-08T10:04:00.000Z", + message: "runtime failure", + }), + ); + + expect(next.status).toBe("error"); + expect(next.lastError).toBe("runtime failure"); + }); + + it("closes session on close lifecycle events", () => { + const previous = makeSession({ + status: "running", + activeTurnId: "turn-1", + }); + const next = evolveSession( + previous, + makeEvent({ + method: "session/closed", + createdAt: "2026-02-08T10:05:00.000Z", + message: "closed", + }), + ); + + expect(next.status).toBe("closed"); + expect(next.activeTurnId).toBeUndefined(); + expect(next.lastError).toBe("closed"); + }); +}); + +describe("applyEventToMessages", () => { + it("handles start/delta/completed flow for assistant messages", () => { + const activeAssistantItemRef = { current: null as string | null }; + const started = applyEventToMessages( + [], + makeEvent({ + method: "item/started", + createdAt: "2026-02-08T10:00:00.000Z", + payload: { + item: { + id: "item-1", + type: "agentMessage", + text: "Hello", + }, + }, + }), + activeAssistantItemRef, + ); + + expect(started).toEqual([ + { + id: "item-1", + role: "assistant", + text: "Hello", + createdAt: "2026-02-08T10:00:00.000Z", + streaming: true, + }, + ]); + expect(activeAssistantItemRef.current).toBe("item-1"); + + const withDelta = applyEventToMessages( + started, + makeEvent({ + method: "item/agentMessage/delta", + createdAt: "2026-02-08T10:00:01.000Z", + itemId: "item-1", + textDelta: " world", + }), + activeAssistantItemRef, + ); + expect(withDelta[0]?.text).toBe("Hello world"); + expect(withDelta[0]?.streaming).toBe(true); + + const completed = applyEventToMessages( + withDelta, + makeEvent({ + method: "item/completed", + createdAt: "2026-02-08T10:00:02.000Z", + payload: { + item: { + id: "item-1", + type: "agentMessage", + text: "Hello world!", + }, + }, + }), + activeAssistantItemRef, + ); + expect(completed[0]?.text).toBe("Hello world!"); + expect(completed[0]?.streaming).toBe(false); + expect(activeAssistantItemRef.current).toBeNull(); + }); + + it("supports out-of-order delta before started/completed", () => { + const activeAssistantItemRef = { current: null as string | null }; + const withDelta = applyEventToMessages( + [], + makeEvent({ + method: "item/agentMessage/delta", + createdAt: "2026-02-08T10:00:01.000Z", + itemId: "item-2", + textDelta: "Partial", + }), + activeAssistantItemRef, + ); + expect(withDelta[0]?.id).toBe("item-2"); + expect(withDelta[0]?.text).toBe("Partial"); + expect(withDelta[0]?.streaming).toBe(true); + expect(activeAssistantItemRef.current).toBe("item-2"); + + const completed = applyEventToMessages( + withDelta, + makeEvent({ + method: "item/completed", + createdAt: "2026-02-08T10:00:02.000Z", + payload: { + item: { + id: "item-2", + type: "agentMessage", + text: "Partial + final", + }, + }, + }), + activeAssistantItemRef, + ); + + expect(completed[0]?.text).toBe("Partial + final"); + expect(completed[0]?.streaming).toBe(false); + }); + + it("clears all streaming flags when turn completes", () => { + const previous: ChatMessage[] = [ + { + id: "m-user", + role: "user", + text: "Hi", + createdAt: "2026-02-08T10:00:00.000Z", + streaming: false, + }, + { + id: "m-assistant", + role: "assistant", + text: "Typing", + createdAt: "2026-02-08T10:00:01.000Z", + streaming: true, + }, + ]; + + const next = applyEventToMessages( + previous, + makeEvent({ + method: "turn/completed", + }), + { current: null }, + ); + + expect(next.every((entry) => entry.streaming === false)).toBe(true); + }); +}); diff --git a/packages/contracts/src/index.ts b/packages/contracts/src/index.ts index f996b088f7fd82aee43e673b38854112f1fe766c..6861cff7c5ef32cb5f7e607dfb82d81ad3fdb85a 100644 --- a/packages/contracts/src/index.ts +++ b/packages/contracts/src/index.ts @@ -3,3 +3,4 @@ export * from "./ipc"; export * from "./agent"; export * from "./terminal"; export * from "./provider"; +export * from "./model"; diff --git a/packages/contracts/src/model.test.ts b/packages/contracts/src/model.test.ts new file mode 100644 index 0000000000000000000000000000000000000000..d701b1b39700450c1de67e82275726619b5468a2 --- /dev/null +++ b/packages/contracts/src/model.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from "vitest"; + +import { + DEFAULT_MODEL, + MODEL_OPTIONS, + normalizeModelSlug, + resolveModelSlug, +} from "./model"; + +describe("normalizeModelSlug", () => { + it("maps known aliases to canonical slugs", () => { + expect(normalizeModelSlug("5.3")).toBe("gpt-5.3-codex"); + expect(normalizeModelSlug("gpt-5.3")).toBe("gpt-5.3-codex"); + }); + + it("returns null for empty or missing values", () => { + expect(normalizeModelSlug("")).toBeNull(); + expect(normalizeModelSlug(" ")).toBeNull(); + expect(normalizeModelSlug(null)).toBeNull(); + expect(normalizeModelSlug(undefined)).toBeNull(); + }); + + it("preserves non-aliased model slugs", () => { + expect(normalizeModelSlug("gpt-5.2")).toBe("gpt-5.2"); + expect(normalizeModelSlug("gpt-5.2-codex")).toBe("gpt-5.2-codex"); + }); +}); + +describe("resolveModelSlug", () => { + it("returns default for missing and unknown models", () => { + expect(resolveModelSlug(undefined)).toBe(DEFAULT_MODEL); + expect(resolveModelSlug("gpt-4.1")).toBe(DEFAULT_MODEL); + }); + + it("resolves only supported model options", () => { + for (const model of MODEL_OPTIONS) { + expect(resolveModelSlug(model)).toBe(model); + } + }); +}); diff --git a/packages/contracts/src/model.ts b/packages/contracts/src/model.ts new file mode 100644 index 0000000000000000000000000000000000000000..caece242f31cc627e4b8d1855d11317eb17983b2 --- /dev/null +++ b/packages/contracts/src/model.ts @@ -0,0 +1,38 @@ +export const MODEL_OPTIONS = [ + "gpt-5.3-codex", + "gpt-5.2-codex", + "gpt-5.2", +] as const; + +export const DEFAULT_MODEL = "gpt-5.3-codex"; + +export const MODEL_SLUG_ALIASES: Record = { + "5.3": "gpt-5.3-codex", + "gpt-5.3": "gpt-5.3-codex", +}; + +export function normalizeModelSlug( + model: string | null | undefined, +): string | null { + if (typeof model !== "string") { + return null; + } + + const trimmed = model.trim(); + if (!trimmed) { + return null; + } + + return MODEL_SLUG_ALIASES[trimmed] ?? trimmed; +} + +export function resolveModelSlug(model: string | null | undefined): string { + const normalized = normalizeModelSlug(model); + if (!normalized) { + return DEFAULT_MODEL; + } + + return MODEL_OPTIONS.includes(normalized as (typeof MODEL_OPTIONS)[number]) + ? normalized + : DEFAULT_MODEL; +}