authorgravatar for julius0216@outlook.comJulius Marminge <julius0216@outlook.com> 2026-02-18 11:03:09-08:00
committergravatar for jmarminge@gmail.comJulius Marminge <jmarminge@gmail.com> 2026-02-18 21:59:17-08:00
log8e744269a2edb2e51d3fa81e576fc29e64d5b186
tree356bc6fbc2a5b4c8f5cc415172999cc6ca601263
parentc90e245038970029f777200201048513da8f84e8

Address latest PR review comments

Co-authored-by: codex <codex@users.noreply.github.com>

6 files changed, 145 insertions(+), 48 deletions(-)

apps/server/src/codexAppServerManager.test.ts+34
...@@ -143,6 +143,40 @@ describe("isRecoverableThreadResumeError", () => {...@@ -143,6 +143,40 @@ describe("isRecoverableThreadResumeError", () => {
143 });143 });
144});144});
145145
146describe("startSession", () => {
147 it("emits session/startFailed when resolving cwd throws before process launch", async () => {
148 const manager = new CodexAppServerManager();
149 const events: Array<{ method: string; kind: string; message?: string }> = [];
150 manager.on("event", (event) => {
151 events.push({
152 method: event.method,
153 kind: event.kind,
154 ...(event.message ? { message: event.message } : {}),
155 });
156 });
157
158 const processCwd = vi.spyOn(process, "cwd").mockImplementation(() => {
159 throw new Error("cwd missing");
160 });
161 try {
162 await expect(
163 manager.startSession({
164 provider: "codex",
165 }),
166 ).rejects.toThrow("cwd missing");
167 expect(events).toHaveLength(1);
168 expect(events[0]).toEqual({
169 method: "session/startFailed",
170 kind: "error",
171 message: "cwd missing",
172 });
173 } finally {
174 processCwd.mockRestore();
175 manager.stopAll();
176 }
177 });
178});
179
146describe("sendTurn", () => {180describe("sendTurn", () => {
147 it("sends text and image user input items to turn/start", async () => {181 it("sends text and image user input items to turn/start", async () => {
148 const { manager, context, requireSession, sendRequest, updateSession } =182 const { manager, context, requireSession, sendRequest, updateSession } =
apps/server/src/codexAppServerManager.ts+50-36
...@@ -148,41 +148,43 @@ export class CodexAppServerManager extends EventEmitter<CodexAppServerManagerEve...@@ -148,41 +148,43 @@ export class CodexAppServerManager extends EventEmitter<CodexAppServerManagerEve
148 async startSession(input: ProviderSessionStartInput): Promise<ProviderSession> {148 async startSession(input: ProviderSessionStartInput): Promise<ProviderSession> {
149 const sessionId = randomUUID();149 const sessionId = randomUUID();
150 const now = new Date().toISOString();150 const now = new Date().toISOString();
151 const resolvedCwd = input.cwd ?? process.cwd();151 let context: CodexSessionContext | undefined;
152152
153 const session: ProviderSession = {153 try {
154 sessionId,154 const resolvedCwd = input.cwd ?? process.cwd();
155 provider: "codex",
156 status: "connecting",
157 model: normalizeCodexModelSlug(input.model),
158 cwd: resolvedCwd,
159 createdAt: now,
160 updatedAt: now,
161 };
162155
163 const child = spawn("codex", ["app-server"], {156 const session: ProviderSession = {
164 cwd: resolvedCwd,157 sessionId,
165 env: process.env,158 provider: "codex",
166 stdio: ["pipe", "pipe", "pipe"],159 status: "connecting",
167 });160 model: normalizeCodexModelSlug(input.model),
168 const output = readline.createInterface({ input: child.stdout });161 cwd: resolvedCwd,
169162 createdAt: now,
170 const context: CodexSessionContext = {163 updatedAt: now,
171 session,164 };
172 child,
173 output,
174 pending: new Map(),
175 pendingApprovals: new Map(),
176 nextRequestId: 1,
177 stopping: false,
178 };
179165
180 this.sessions.set(sessionId, context);166 const child = spawn("codex", ["app-server"], {
181 this.attachProcessListeners(context);167 cwd: resolvedCwd,
168 env: process.env,
169 stdio: ["pipe", "pipe", "pipe"],
170 });
171 const output = readline.createInterface({ input: child.stdout });
172
173 context = {
174 session,
175 child,
176 output,
177 pending: new Map(),
178 pendingApprovals: new Map(),
179 nextRequestId: 1,
180 stopping: false,
181 };
182182
183 this.emitLifecycleEvent(context, "session/connecting", "Starting codex app-server");183 this.sessions.set(sessionId, context);
184 this.attachProcessListeners(context);
185
186 this.emitLifecycleEvent(context, "session/connecting", "Starting codex app-server");
184187
185 try {
186 await this.sendRequest(context, "initialize", {188 await this.sendRequest(context, "initialize", {
187 clientInfo: {189 clientInfo: {
188 name: "t3code_desktop",190 name: "t3code_desktop",
...@@ -252,12 +254,24 @@ export class CodexAppServerManager extends EventEmitter<CodexAppServerManagerEve...@@ -252,12 +254,24 @@ export class CodexAppServerManager extends EventEmitter<CodexAppServerManagerEve
252 return { ...context.session };254 return { ...context.session };
253 } catch (error) {255 } catch (error) {
254 const message = error instanceof Error ? error.message : "Failed to start Codex session.";256 const message = error instanceof Error ? error.message : "Failed to start Codex session.";
255 this.updateSession(context, {257 if (context) {
256 status: "error",258 this.updateSession(context, {
257 lastError: message,259 status: "error",
258 });260 lastError: message,
259 this.emitErrorEvent(context, "session/startFailed", message);261 });
260 this.stopSession(sessionId);262 this.emitErrorEvent(context, "session/startFailed", message);
263 this.stopSession(sessionId);
264 } else {
265 this.emitEvent({
266 id: randomUUID(),
267 kind: "error",
268 provider: "codex",
269 sessionId,
270 createdAt: new Date().toISOString(),
271 method: "session/startFailed",
272 message,
273 });
274 }
261 throw new Error(message, { cause: error });275 throw new Error(message, { cause: error });
262 }276 }
263 }277 }
apps/server/src/filesystemCheckpointStore.test.ts+7
...@@ -204,4 +204,11 @@ describe("FilesystemCheckpointStore", () => {...@@ -204,4 +204,11 @@ describe("FilesystemCheckpointStore", () => {
204 await rm(cwd, { recursive: true, force: true });204 await rm(cwd, { recursive: true, force: true });
205 }205 }
206 });206 });
207
208 it("returns false when repository path does not exist", async () => {
209 const store = new FilesystemCheckpointStore();
210 const cwd = path.join(os.tmpdir(), `t3-fs-checkpoints-missing-${Date.now()}`);
211
212 expect(await store.isGitRepository(cwd)).toBe(false);
213 });
207});214});
apps/server/src/filesystemCheckpointStore.ts+8-4
...@@ -32,10 +32,14 @@ interface GitOptions {...@@ -32,10 +32,14 @@ interface GitOptions {
3232
33export class FilesystemCheckpointStore {33export class FilesystemCheckpointStore {
34 async isGitRepository(cwd: string): Promise<boolean> {34 async isGitRepository(cwd: string): Promise<boolean> {
35 const result = await this.runGit(cwd, ["rev-parse", "--is-inside-work-tree"], {35 try {
36 allowNonZeroExit: true,36 const result = await this.runGit(cwd, ["rev-parse", "--is-inside-work-tree"], {
37 });37 allowNonZeroExit: true,
38 return result.code === 0 && result.stdout.trim() === "true";38 });
39 return result.code === 0 && result.stdout.trim() === "true";
40 } catch {
41 return false;
42 }
39 }43 }
4044
41 async captureCheckpoint(input: {45 async captureCheckpoint(input: {
apps/web/src/store.test.ts+16-8
...@@ -618,10 +618,10 @@ describe("store reducer thread continuity", () => {...@@ -618,10 +618,10 @@ describe("store reducer thread continuity", () => {
618 ],618 ],
619 activeThreadId: "thread-b",619 activeThreadId: "thread-b",
620 runtimeMode: "full-access",620 runtimeMode: "full-access",
621 diffOpen: false,621 diffOpen: true,
622 diffThreadId: null,622 diffThreadId: "thread-b",
623 diffTurnId: null,623 diffTurnId: "turn-b",
624 diffFilePath: null,624 diffFilePath: "src/removed.ts",
625 };625 };
626626
627 const next = reducer(state, {627 const next = reducer(state, {
...@@ -646,6 +646,10 @@ describe("store reducer thread continuity", () => {...@@ -646,6 +646,10 @@ describe("store reducer thread continuity", () => {
646 expect(next.threads[0]?.id).toBe("thread-a");646 expect(next.threads[0]?.id).toBe("thread-a");
647 expect(next.threads[0]?.projectId).toBe("project-new-a");647 expect(next.threads[0]?.projectId).toBe("project-new-a");
648 expect(next.activeThreadId).toBe("thread-a");648 expect(next.activeThreadId).toBe("thread-a");
649 expect(next.diffOpen).toBe(false);
650 expect(next.diffThreadId).toBeNull();
651 expect(next.diffTurnId).toBeNull();
652 expect(next.diffFilePath).toBeNull();
649 });653 });
650654
651 it("treats empty scripts from sync as authoritative", () => {655 it("treats empty scripts from sync as authoritative", () => {
...@@ -753,10 +757,10 @@ describe("store reducer thread continuity", () => {...@@ -753,10 +757,10 @@ describe("store reducer thread continuity", () => {
753 ],757 ],
754 activeThreadId: "thread-a",758 activeThreadId: "thread-a",
755 runtimeMode: "full-access",759 runtimeMode: "full-access",
756 diffOpen: false,760 diffOpen: true,
757 diffThreadId: null,761 diffThreadId: "thread-a",
758 diffTurnId: null,762 diffTurnId: "turn-a",
759 diffFilePath: null,763 diffFilePath: "src/a.ts",
760 };764 };
761765
762 const next = reducer(state, {766 const next = reducer(state, {
...@@ -769,6 +773,10 @@ describe("store reducer thread continuity", () => {...@@ -769,6 +773,10 @@ describe("store reducer thread continuity", () => {
769 expect(next.threads).toHaveLength(1);773 expect(next.threads).toHaveLength(1);
770 expect(next.threads[0]?.id).toBe("thread-b");774 expect(next.threads[0]?.id).toBe("thread-b");
771 expect(next.activeThreadId).toBe("thread-b");775 expect(next.activeThreadId).toBe("thread-b");
776 expect(next.diffOpen).toBe(false);
777 expect(next.diffThreadId).toBeNull();
778 expect(next.diffTurnId).toBeNull();
779 expect(next.diffFilePath).toBeNull();
772 });780 });
773781
774 it("marks the active thread as visited when selected", () => {782 it("marks the active thread as visited when selected", () => {
apps/web/src/store.ts+30
...@@ -198,6 +198,32 @@ function updateThread(...@@ -198,6 +198,32 @@ function updateThread(
198 return threads.map((t) => (t.id === threadId ? updater(t) : t));198 return threads.map((t) => (t.id === threadId ? updater(t) : t));
199}199}
200200
201function resetDiffTargetIfMissing(state: AppState, threads: Thread[]) {
202 if (!state.diffThreadId) {
203 return {
204 diffOpen: state.diffOpen,
205 diffThreadId: state.diffThreadId,
206 diffTurnId: state.diffTurnId,
207 diffFilePath: state.diffFilePath,
208 };
209 }
210 const hasThread = threads.some((thread) => thread.id === state.diffThreadId);
211 if (hasThread) {
212 return {
213 diffOpen: state.diffOpen,
214 diffThreadId: state.diffThreadId,
215 diffTurnId: state.diffTurnId,
216 diffFilePath: state.diffFilePath,
217 };
218 }
219 return {
220 diffOpen: false,
221 diffThreadId: null,
222 diffTurnId: null,
223 diffFilePath: null,
224 };
225}
226
201function mergeTurnDiffSummaries(227function mergeTurnDiffSummaries(
202 existing: Thread["turnDiffSummaries"],228 existing: Thread["turnDiffSummaries"],
203 next: Thread["turnDiffSummaries"],229 next: Thread["turnDiffSummaries"],
...@@ -602,12 +628,14 @@ export function reducer(state: AppState, action: Action): AppState {...@@ -602,12 +628,14 @@ export function reducer(state: AppState, action: Action): AppState {
602 const activeThreadId = nextThreads.some((thread) => thread.id === state.activeThreadId)628 const activeThreadId = nextThreads.some((thread) => thread.id === state.activeThreadId)
603 ? state.activeThreadId629 ? state.activeThreadId
604 : (nextThreads[0]?.id ?? null);630 : (nextThreads[0]?.id ?? null);
631 const diffState = resetDiffTargetIfMissing(state, nextThreads);
605632
606 return {633 return {
607 ...state,634 ...state,
608 projects: nextProjects,635 projects: nextProjects,
609 threads: nextThreads,636 threads: nextThreads,
610 activeThreadId,637 activeThreadId,
638 ...diffState,
611 };639 };
612 }640 }
613641
...@@ -629,12 +657,14 @@ export function reducer(state: AppState, action: Action): AppState {...@@ -629,12 +657,14 @@ export function reducer(state: AppState, action: Action): AppState {
629 const activeThreadId = threads.some((thread) => thread.id === state.activeThreadId)657 const activeThreadId = threads.some((thread) => thread.id === state.activeThreadId)
630 ? state.activeThreadId658 ? state.activeThreadId
631 : (threads[0]?.id ?? null);659 : (threads[0]?.id ?? null);
660 const diffState = resetDiffTargetIfMissing(state, threads);
632661
633 return {662 return {
634 ...state,663 ...state,
635 projects,664 projects,
636 threads,665 threads,
637 activeThreadId,666 activeThreadId,
667 ...diffState,
638 };668 };
639 }669 }
640670