feat: Add GPT-5 model variants and improve Codex execution logic. Addressed code review comments

apps/server/tests/unit/lib/git-log-parser.test.ts

@@ -0,0 +1,196 @@
+import { describe, it, expect } from 'vitest';
+import { parseGitLogOutput } from '../../../src/lib/git-log-parser.js';
+
+// Mock data: fields within each commit are newline-separated,
+// commits are NUL-separated (matching the parser contract).
+const mockGitOutput = [
+  'a1b2c3d4e5f67890abcd1234567890abcd1234\na1b2c3\nJohn Doe\njohn@example.com\n2023-01-01T12:00:00Z\nInitial commit\nThis is the commit body',
+  'e5f6g7h8i9j0klmnoprstuv\ne5f6g7\nJane Smith\njane@example.com\n2023-01-02T12:00:00Z\nFix bug\nFixed the bug with ---END--- in the message',
+  'q1w2e3r4t5y6u7i8o9p0asdfghjkl\nq1w2e3\nBob Johnson\nbob@example.com\n2023-01-03T12:00:00Z\nAnother commit\nEmpty body',
+].join('\0');
+
+// Mock data where commit bodies contain ---END--- markers
+const mockOutputWithEndMarker = [
+  'a1b2c3d4e5f67890abcd1234567890abcd1234\na1b2c3\nJohn Doe\njohn@example.com\n2023-01-01T12:00:00Z\nInitial commit\nThis is the commit body\n---END--- is in this message',
+  'e5f6g7h8i9j0klmnoprstuv\ne5f6g7\nJane Smith\njane@example.com\n2023-01-02T12:00:00Z\nFix bug\nFixed the bug with ---END--- in the message',
+  'q1w2e3r4t5y6u7i8o9p0asdfghjkl\nq1w2e3\nBob Johnson\nbob@example.com\n2023-01-03T12:00:00Z\nAnother commit\nEmpty body',
+].join('\0');
+
+// Single-commit mock: fields newline-separated, no trailing NUL needed
+const singleCommitOutput =
+  'a1b2c3d4e5f67890abcd1234567890abcd1234\na1b2c3\nJohn Doe\njohn@example.com\n2023-01-01T12:00:00Z\nSingle commit\nSingle commit body';
+
+describe('parseGitLogOutput', () => {
+  describe('normal parsing (three commits)', () => {
+    it('returns the correct number of commits', () => {
+      const commits = parseGitLogOutput(mockGitOutput);
+      expect(commits.length).toBe(3);
+    });
+
+    it('parses the first commit fields correctly', () => {
+      const commits = parseGitLogOutput(mockGitOutput);
+      expect(commits[0].hash).toBe('a1b2c3d4e5f67890abcd1234567890abcd1234');
+      expect(commits[0].shortHash).toBe('a1b2c3');
+      expect(commits[0].author).toBe('John Doe');
+      expect(commits[0].authorEmail).toBe('john@example.com');
+      expect(commits[0].date).toBe('2023-01-01T12:00:00Z');
+      expect(commits[0].subject).toBe('Initial commit');
+      expect(commits[0].body).toBe('This is the commit body');
+    });
+
+    it('parses the second commit fields correctly', () => {
+      const commits = parseGitLogOutput(mockGitOutput);
+      expect(commits[1].hash).toBe('e5f6g7h8i9j0klmnoprstuv');
+      expect(commits[1].shortHash).toBe('e5f6g7');
+      expect(commits[1].author).toBe('Jane Smith');
+      expect(commits[1].subject).toBe('Fix bug');
+      expect(commits[1].body).toMatch(/---END---/);
+    });
+
+    it('parses the third commit fields correctly', () => {
+      const commits = parseGitLogOutput(mockGitOutput);
+      expect(commits[2].hash).toBe('q1w2e3r4t5y6u7i8o9p0asdfghjkl');
+      expect(commits[2].shortHash).toBe('q1w2e3');
+      expect(commits[2].author).toBe('Bob Johnson');
+      expect(commits[2].subject).toBe('Another commit');
+      expect(commits[2].body).toBe('Empty body');
+    });
+  });
+
+  describe('parsing with ---END--- in commit messages', () => {
+    it('returns the correct number of commits', () => {
+      const commits = parseGitLogOutput(mockOutputWithEndMarker);
+      expect(commits.length).toBe(3);
+    });
+
+    it('preserves ---END--- text in the body of the first commit', () => {
+      const commits = parseGitLogOutput(mockOutputWithEndMarker);
+      expect(commits[0].subject).toBe('Initial commit');
+      expect(commits[0].body).toMatch(/---END---/);
+    });
+
+    it('preserves ---END--- text in the body of the second commit', () => {
+      const commits = parseGitLogOutput(mockOutputWithEndMarker);
+      expect(commits[1].subject).toBe('Fix bug');
+      expect(commits[1].body).toMatch(/---END---/);
+    });
+
+    it('parses the third commit without ---END--- interference', () => {
+      const commits = parseGitLogOutput(mockOutputWithEndMarker);
+      expect(commits[2].subject).toBe('Another commit');
+      expect(commits[2].body).toBe('Empty body');
+    });
+  });
+
+  describe('empty output', () => {
+    it('returns an empty array for an empty string', () => {
+      const commits = parseGitLogOutput('');
+      expect(commits).toEqual([]);
+      expect(commits.length).toBe(0);
+    });
+  });
+
+  describe('single-commit output', () => {
+    it('returns exactly one commit', () => {
+      const commits = parseGitLogOutput(singleCommitOutput);
+      expect(commits.length).toBe(1);
+    });
+
+    it('parses the single commit fields correctly', () => {
+      const commits = parseGitLogOutput(singleCommitOutput);
+      expect(commits[0].hash).toBe('a1b2c3d4e5f67890abcd1234567890abcd1234');
+      expect(commits[0].shortHash).toBe('a1b2c3');
+      expect(commits[0].author).toBe('John Doe');
+      expect(commits[0].authorEmail).toBe('john@example.com');
+      expect(commits[0].date).toBe('2023-01-01T12:00:00Z');
+      expect(commits[0].subject).toBe('Single commit');
+      expect(commits[0].body).toBe('Single commit body');
+    });
+  });
+
+  describe('multi-line commit body', () => {
+    // Test vector from test-proper-nul-format.js: commit with a 3-line body
+    const multiLineBodyOutput =
+      [
+        'abc123\nabc1\nJohn Doe\njohn@example.com\n2023-01-01T12:00:00Z\nInitial commit\nThis is a normal commit body',
+        'def456\ndef4\nJane Smith\njane@example.com\n2023-01-02T12:00:00Z\nFix bug\nFixed the bug with ---END--- in this message',
+        'ghi789\nghi7\nBob Johnson\nbob@example.com\n2023-01-03T12:00:00Z\nAnother commit\nThis body has multiple lines\nSecond line\nThird line',
+      ].join('\0') + '\0';
+
+    it('returns 3 commits', () => {
+      const commits = parseGitLogOutput(multiLineBodyOutput);
+      expect(commits.length).toBe(3);
+    });
+
+    it('parses the first commit correctly', () => {
+      const commits = parseGitLogOutput(multiLineBodyOutput);
+      expect(commits[0].hash).toBe('abc123');
+      expect(commits[0].shortHash).toBe('abc1');
+      expect(commits[0].author).toBe('John Doe');
+      expect(commits[0].authorEmail).toBe('john@example.com');
+      expect(commits[0].date).toBe('2023-01-01T12:00:00Z');
+      expect(commits[0].subject).toBe('Initial commit');
+      expect(commits[0].body).toBe('This is a normal commit body');
+    });
+
+    it('parses the second commit with ---END--- in body correctly', () => {
+      const commits = parseGitLogOutput(multiLineBodyOutput);
+      expect(commits[1].hash).toBe('def456');
+      expect(commits[1].shortHash).toBe('def4');
+      expect(commits[1].author).toBe('Jane Smith');
+      expect(commits[1].subject).toBe('Fix bug');
+      expect(commits[1].body).toContain('---END---');
+    });
+
+    it('parses the third commit with a multi-line body correctly', () => {
+      const commits = parseGitLogOutput(multiLineBodyOutput);
+      expect(commits[2].hash).toBe('ghi789');
+      expect(commits[2].shortHash).toBe('ghi7');
+      expect(commits[2].author).toBe('Bob Johnson');
+      expect(commits[2].subject).toBe('Another commit');
+      expect(commits[2].body).toBe('This body has multiple lines\nSecond line\nThird line');
+    });
+  });
+
+  describe('commit with empty body (trailing blank lines after subject)', () => {
+    // Test vector from test-proper-nul-format.js: empty body commit
+    const emptyBodyOutput =
+      'empty123\nempty1\nAlice Brown\nalice@example.com\n2023-01-04T12:00:00Z\nEmpty body commit\n\n\0';
+
+    it('returns 1 commit', () => {
+      const commits = parseGitLogOutput(emptyBodyOutput);
+      expect(commits.length).toBe(1);
+    });
+
+    it('parses the commit subject correctly', () => {
+      const commits = parseGitLogOutput(emptyBodyOutput);
+      expect(commits[0].hash).toBe('empty123');
+      expect(commits[0].shortHash).toBe('empty1');
+      expect(commits[0].author).toBe('Alice Brown');
+      expect(commits[0].subject).toBe('Empty body commit');
+    });
+
+    it('produces an empty body string when only blank lines follow the subject', () => {
+      const commits = parseGitLogOutput(emptyBodyOutput);
+      expect(commits[0].body).toBe('');
+    });
+  });
+
+  describe('leading empty lines in a commit block', () => {
+    // Blocks that start with blank lines before the hash field
+    const outputWithLeadingBlanks =
+      '\n\nabc123\nabc1\nJohn Doe\njohn@example.com\n2023-01-01T12:00:00Z\nSubject here\nBody here';
+
+    it('returns 1 commit despite leading blank lines', () => {
+      const commits = parseGitLogOutput(outputWithLeadingBlanks);
+      expect(commits.length).toBe(1);
+    });
+
+    it('parses the commit fields correctly when block has leading empty lines', () => {
+      const commits = parseGitLogOutput(outputWithLeadingBlanks);
+      expect(commits[0].hash).toBe('abc123');
+      expect(commits[0].subject).toBe('Subject here');
+      expect(commits[0].body).toBe('Body here');
+    });
+  });
+});

apps/server/tests/unit/lib/nul-delimiter.test.ts

@@ -0,0 +1,83 @@
+// Automated tests for NUL character behavior in git commit parsing
+
+import { describe, it, expect } from 'vitest';
+
+describe('NUL character behavior', () => {
+  // Create a string with NUL characters
+  const str1 =
+    'abc123\x00abc1\x00John Doe\x00john@example.com\x002023-01-01T12:00:00Z\x00Initial commit\x00This is a normal commit body\x00';
+
+  describe('split on NUL character', () => {
+    const parts = str1.split('\0');
+
+    it('should produce the expected number of parts', () => {
+      // 7 fields + 1 trailing empty string from the trailing \x00
+      expect(parts.length).toBe(8);
+    });
+
+    it('should contain the expected part values', () => {
+      expect(parts[0]).toBe('abc123');
+      expect(parts[1]).toBe('abc1');
+      expect(parts[2]).toBe('John Doe');
+      expect(parts[3]).toBe('john@example.com');
+      expect(parts[4]).toBe('2023-01-01T12:00:00Z');
+      expect(parts[5]).toBe('Initial commit');
+      expect(parts[6]).toBe('This is a normal commit body');
+      expect(parts[7]).toBe('');
+    });
+
+    it('should have correct lengths for each part', () => {
+      expect(parts[0].length).toBe(6); // 'abc123'
+      expect(parts[1].length).toBe(4); // 'abc1'
+      expect(parts[2].length).toBe(8); // 'John Doe'
+      expect(parts[3].length).toBe(16); // 'john@example.com'
+      expect(parts[4].length).toBe(20); // '2023-01-01T12:00:00Z'
+      expect(parts[5].length).toBe(14); // 'Initial commit'
+      expect(parts[6].length).toBe(28); // 'This is a normal commit body'
+      expect(parts[7].length).toBe(0); // trailing empty
+    });
+  });
+
+  describe('git format split and filter', () => {
+    const gitFormat = `abc123\x00abc1\x00John Doe\x00john@example.com\x002023-01-01T12:00:00Z\x00Initial commit\x00Body text here\x00def456\x00def4\x00Jane Smith\x00jane@example.com\x002023-01-02T12:00:00Z\x00Second commit\x00Body with ---END--- text\x00`;
+
+    const gitParts = gitFormat.split('\0').filter((block) => block.trim());
+
+    it('should produce the expected number of non-empty parts after filtering', () => {
+      // 14 non-empty field strings (7 fields per commit × 2 commits); trailing empty is filtered out
+      expect(gitParts.length).toBe(14);
+    });
+
+    it('should contain correct field values for the first commit', () => {
+      const fields = gitParts.slice(0, 7);
+      expect(fields.length).toBe(7);
+      expect(fields[0]).toBe('abc123'); // hash
+      expect(fields[1]).toBe('abc1'); // shortHash
+      expect(fields[2]).toBe('John Doe'); // author
+      expect(fields[3]).toBe('john@example.com'); // authorEmail
+      expect(fields[4]).toBe('2023-01-01T12:00:00Z'); // date
+      expect(fields[5]).toBe('Initial commit'); // subject
+      expect(fields[6]).toBe('Body text here'); // body
+    });
+
+    it('should contain correct field values for the second commit', () => {
+      const fields = gitParts.slice(7, 14);
+      expect(fields.length).toBe(7);
+      expect(fields[0]).toBe('def456'); // hash
+      expect(fields[1]).toBe('def4'); // shortHash
+      expect(fields[2]).toBe('Jane Smith'); // author
+      expect(fields[3]).toBe('jane@example.com'); // authorEmail
+      expect(fields[4]).toBe('2023-01-02T12:00:00Z'); // date
+      expect(fields[5]).toBe('Second commit'); // subject
+      expect(fields[6]).toBe('Body with ---END--- text'); // body (---END--- handled correctly)
+    });
+
+    it('each part should have the expected number of newline-delimited fields', () => {
+      // Each gitPart is a single field value (no internal newlines), so split('\n') yields 1 field
+      gitParts.forEach((block) => {
+        const fields = block.split('\n');
+        expect(fields.length).toBe(1);
+      });
+    });
+  });
+});

apps/server/tests/unit/providers/codex-provider.test.ts

@@ -247,6 +247,12 @@ describe('codex-provider.ts', () => {
 
     it('uses the SDK when no tools are requested and an API key is present', async () => {
       process.env[OPENAI_API_KEY_ENV] = 'sk-test';
+      // Override auth indicators so CLI-native auth doesn't take priority over API key
+      vi.mocked(getCodexAuthIndicators).mockResolvedValue({
+        hasAuthFile: false,
+        hasOAuthToken: false,
+        hasApiKey: false,
+      });
       codexRunMock.mockResolvedValue({ finalResponse: 'Hello from SDK' });
 
       const results = await collectAsyncGenerator<ProviderMessage>(
@@ -264,6 +270,12 @@ describe('codex-provider.ts', () => {
 
     it('uses the SDK when API key is present, even for tool requests (to avoid OAuth issues)', async () => {
       process.env[OPENAI_API_KEY_ENV] = 'sk-test';
+      // Override auth indicators so CLI-native auth doesn't take priority over API key
+      vi.mocked(getCodexAuthIndicators).mockResolvedValue({
+        hasAuthFile: false,
+        hasOAuthToken: false,
+        hasApiKey: false,
+      });
       vi.mocked(spawnJSONLProcess).mockReturnValue((async function* () {})());
 
       await collectAsyncGenerator(

apps/server/tests/unit/routes/worktree/switch-branch.test.ts

@@ -1,27 +1,15 @@
-import { describe, it, expect, vi, beforeEach, type Mock } from 'vitest';
+import { describe, it, expect, vi, beforeEach } from 'vitest';
 import type { Request, Response } from 'express';
 import { createMockExpressContext } from '../../../utils/mocks.js';
 
-vi.mock('child_process', async (importOriginal) => {
-  const actual = await importOriginal<typeof import('child_process')>();
-  return {
-    ...actual,
-    execFile: vi.fn(),
-  };
-});
+vi.mock('@/services/worktree-branch-service.js', () => ({
+  performSwitchBranch: vi.fn(),
+}));
 
-vi.mock('util', async (importOriginal) => {
-  const actual = await importOriginal<typeof import('util')>();
-  return {
-    ...actual,
-    promisify: (fn: unknown) => fn,
-  };
-});
-
-import { execFile } from 'child_process';
+import { performSwitchBranch } from '@/services/worktree-branch-service.js';
 import { createSwitchBranchHandler } from '@/routes/worktree/routes/switch-branch.js';
 
-const mockExecFile = execFile as Mock;
+const mockPerformSwitchBranch = vi.mocked(performSwitchBranch);
 
 describe('switch-branch route', () => {
   let req: Request;
@@ -34,42 +22,77 @@ describe('switch-branch route', () => {
     res = context.res;
   });
 
+  it('should return 400 when branchName is missing', async () => {
+    req.body = { worktreePath: '/repo/path' };
+
+    const handler = createSwitchBranchHandler();
+    await handler(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith({
+      success: false,
+      error: 'branchName required',
+    });
+    expect(mockPerformSwitchBranch).not.toHaveBeenCalled();
+  });
+
+  it('should return 400 when branchName starts with a dash', async () => {
+    req.body = { worktreePath: '/repo/path', branchName: '-flag' };
+
+    const handler = createSwitchBranchHandler();
+    await handler(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith({
+      success: false,
+      error: 'Invalid branch name',
+    });
+    expect(mockPerformSwitchBranch).not.toHaveBeenCalled();
+  });
+
+  it('should return 400 when branchName starts with double dash', async () => {
+    req.body = { worktreePath: '/repo/path', branchName: '--option' };
+
+    const handler = createSwitchBranchHandler();
+    await handler(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith({
+      success: false,
+      error: 'Invalid branch name',
+    });
+    expect(mockPerformSwitchBranch).not.toHaveBeenCalled();
+  });
+
+  it('should return 400 when branchName contains invalid characters', async () => {
+    req.body = { worktreePath: '/repo/path', branchName: 'branch name with spaces' };
+
+    const handler = createSwitchBranchHandler();
+    await handler(req, res);
+
+    expect(res.status).toHaveBeenCalledWith(400);
+    expect(res.json).toHaveBeenCalledWith({
+      success: false,
+      error: 'Invalid branch name',
+    });
+    expect(mockPerformSwitchBranch).not.toHaveBeenCalled();
+  });
+
   it('should allow switching when only untracked files exist', async () => {
     req.body = {
       worktreePath: '/repo/path',
       branchName: 'feature/test',
     };
 
-    mockExecFile.mockImplementation(async (file: string, args: string[]) => {
-      const command = `${file} ${args.join(' ')}`;
-      if (command === 'git rev-parse --abbrev-ref HEAD') {
-        return { stdout: 'main\n', stderr: '' };
-      }
-      if (command === 'git rev-parse --verify feature/test') {
-        return { stdout: 'abc123\n', stderr: '' };
-      }
-      if (command === 'git branch -r --format=%(refname:short)') {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git status --porcelain') {
-        return { stdout: '?? .automaker/\n?? notes.txt\n', stderr: '' };
-      }
-      if (command === 'git checkout feature/test') {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git fetch --all --quiet') {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git stash list') {
-        return { stdout: '', stderr: '' };
-      }
-      if (command.startsWith('git stash push')) {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git stash pop') {
-        return { stdout: '', stderr: '' };
-      }
-      return { stdout: '', stderr: '' };
+    mockPerformSwitchBranch.mockResolvedValue({
+      success: true,
+      result: {
+        previousBranch: 'main',
+        currentBranch: 'feature/test',
+        message: "Switched to branch 'feature/test'",
+        hasConflicts: false,
+        stashedChanges: false,
+      },
     });
 
     const handler = createSwitchBranchHandler();
@@ -85,11 +108,7 @@ describe('switch-branch route', () => {
         stashedChanges: false,
       },
     });
-    expect(mockExecFile).toHaveBeenCalledWith(
-      'git',
-      ['checkout', 'feature/test'],
-      expect.objectContaining({ cwd: '/repo/path' })
-    );
+    expect(mockPerformSwitchBranch).toHaveBeenCalledWith('/repo/path', 'feature/test', undefined);
   });
 
   it('should stash changes and switch when tracked files are modified', async () => {
@@ -98,42 +117,15 @@ describe('switch-branch route', () => {
       branchName: 'feature/test',
     };
 
-    let stashListCallCount = 0;
-
-    mockExecFile.mockImplementation(async (file: string, args: string[]) => {
-      const command = `${file} ${args.join(' ')}`;
-      if (command === 'git rev-parse --abbrev-ref HEAD') {
-        return { stdout: 'main\n', stderr: '' };
-      }
-      if (command === 'git rev-parse --verify feature/test') {
-        return { stdout: 'abc123\n', stderr: '' };
-      }
-      if (command === 'git status --porcelain') {
-        return { stdout: ' M src/index.ts\n?? notes.txt\n', stderr: '' };
-      }
-      if (command === 'git branch -r --format=%(refname:short)') {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git stash list') {
-        stashListCallCount++;
-        if (stashListCallCount === 1) {
-          return { stdout: '', stderr: '' };
-        }
-        return { stdout: 'stash@{0}: automaker-branch-switch\n', stderr: '' };
-      }
-      if (command.startsWith('git stash push')) {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git checkout feature/test') {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git fetch --all --quiet') {
-        return { stdout: '', stderr: '' };
-      }
-      if (command === 'git stash pop') {
-        return { stdout: 'Already applied.\n', stderr: '' };
-      }
-      return { stdout: '', stderr: '' };
+    mockPerformSwitchBranch.mockResolvedValue({
+      success: true,
+      result: {
+        previousBranch: 'main',
+        currentBranch: 'feature/test',
+        message: "Switched to branch 'feature/test' (local changes stashed and reapplied)",
+        hasConflicts: false,
+        stashedChanges: true,
+      },
     });
 
     const handler = createSwitchBranchHandler();