feat(polish): multi-provider UX improvements

__tests__/electron/services/api-routes/agent-routes.test.ts

@@ -25,6 +25,8 @@ vi.mock('../../../../electron/core/agent-manager', () => ({
   agents: new Map(),
   saveAgents: vi.fn(),
   initAgentPty: vi.fn(),
+  killStalePty: vi.fn(),
+  ensureProjectTrusted: vi.fn(),
 }));
 
 vi.mock('../../../../electron/core/pty-manager', () => ({
@@ -37,7 +39,7 @@ vi.mock('../../../../electron/utils/path-builder', () => ({
 }));
 
 import { registerAgentRoutes } from '../../../../electron/services/api-routes/agent-routes';
-import { agents, saveAgents } from '../../../../electron/core/agent-manager';
+import { agents, saveAgents, killStalePty } from '../../../../electron/core/agent-manager';
 import { ptyProcesses, writeProgrammaticInput } from '../../../../electron/core/pty-manager';
 import { RouteApp, RouteContext, RouteRequest, SendJson } from '../../../../electron/services/api-routes/types';
 import { AgentStatus, AppSettings } from '../../../../electron/types';
@@ -85,13 +87,15 @@ beforeEach(() => {
   agents.clear();
   ptyProcesses.clear();
   vi.mocked(saveAgents).mockClear();
+  vi.mocked(killStalePty).mockClear();
   mockPtyProcess.onData.mockClear();
   mockPtyProcess.onExit.mockClear();
   mockPtyProcess.kill.mockClear();
 
   ctx = {
     mainWindow: { isDestroyed: () => false, webContents: { send: vi.fn() } } as any,
     appSettings: {} as AppSettings,
+    getAppSettings: () => ({} as AppSettings),
     getTelegramBot: () => null,
     getSlackApp: () => null,
     slackResponseChannel: null,
@@ -215,6 +219,49 @@ describe('agent-routes', () => {
     });
   });
 
+  describe('POST /api/agents/:id/start', () => {
+    it('kills existing PTY before spawning a new one', async () => {
+      const existingPty = { kill: vi.fn() };
+      ptyProcesses.set('existing-pty', existingPty as any);
+      const agent = makeAgent({ id: 'a1', status: 'idle', ptyId: 'existing-pty' });
+      agents.set('a1', agent);
+
+      const app = makeRouteApp();
+      registerAgentRoutes(app, ctx);
+      const handler = findHandler(app, 'POST', 'start');
+
+      const sendJson = vi.fn();
+      await handler(makeReq({ params: { id: 'a1' }, body: { prompt: 'Do something' } }), sendJson, ctx);
+
+      expect(existingPty.kill).toHaveBeenCalled();
+      expect(ptyProcesses.has('existing-pty')).toBe(false);
+      expect(sendJson).toHaveBeenCalledWith({ success: true, agent: { id: 'a1', status: 'running' } });
+    });
+
+    it('transitions waiting→error when PTY exits while agent is waiting', async () => {
+      const agent = makeAgent({ id: 'a1', status: 'idle' });
+      agents.set('a1', agent);
+
+      const app = makeRouteApp();
+      registerAgentRoutes(app, ctx);
+      const handler = findHandler(app, 'POST', 'start');
+
+      const sendJson = vi.fn();
+      await handler(makeReq({ params: { id: 'a1' }, body: { prompt: 'Do something' } }), sendJson, ctx);
+
+      // Simulate status being set to 'waiting' by a hook while the PTY is running
+      agent.status = 'waiting';
+
+      // Simulate PTY exit
+      const exitHandler = mockPtyProcess.onExit.mock.calls[0][0] as (args: { exitCode: number }) => void;
+      exitHandler({ exitCode: 1 });
+
+      // After the 1500ms delay, status should become 'error' not stay 'waiting'
+      await new Promise(r => setTimeout(r, 1600));
+      expect(agent.status).toBe('error');
+    });
+  });
+
   describe('POST /api/agents/:id/message', () => {
     it('sends message to agent PTY', async () => {
       const mockPty = { write: vi.fn() };
@@ -233,23 +280,106 @@ describe('agent-routes', () => {
       expect(sendJson).toHaveBeenCalledWith({ success: true });
     });
 
-    it('initializes PTY if not present', async () => {
+    it('auto-respawns PTY with message as prompt when PTY is missing', async () => {
+      const agent = makeAgent({ id: 'a1', status: 'waiting', projectPath: '/test/project' });
+      agents.set('a1', agent);
+
+      const app = makeRouteApp();
+      registerAgentRoutes(app, ctx);
+      const handler = findHandler(app, 'POST', 'message');
+
+      const sendJson = vi.fn();
+      await handler(makeReq({ params: { id: 'a1' }, body: { message: 'continue the task' } }), sendJson, ctx);
+
+      // Should NOT call the legacy initAgentPtyCallback (bare bash shell)
+      expect(ctx.initAgentPtyCallback).not.toHaveBeenCalled();
+      // Should have spawned a new one-shot PTY
+      const pty = await import('node-pty');
+      expect(pty.spawn).toHaveBeenCalled();
+      // Agent should be running and PTY registered
+      expect(agent.status).toBe('running');
+      expect(agent.ptyId).toBe('test-uuid');
+      expect(ptyProcesses.has('test-uuid')).toBe(true);
+      expect(sendJson).toHaveBeenCalledWith({ success: true });
+    });
+
+    it('BUG 4: calls killStalePty before reusing existing PTY', async () => {
       const mockPty = { write: vi.fn() };
-      const agent = makeAgent({ id: 'a1', status: 'waiting' });
+      ptyProcesses.set('pty-1', mockPty as any);
+      const agent = makeAgent({
+        id: 'a1',
+        status: 'running',
+        ptyId: 'pty-1',
+        projectPath: '/test/project',
+        worktreePath: '/test/project/.worktrees/feat/backend',
+        ptyCwd: '/test/project/.worktrees/feat/backend',
+      });
       agents.set('a1', agent);
 
-      // initAgentPtyCallback returns 'new-pty-id', so set up that PTY
-      ptyProcesses.set('new-pty-id', mockPty as any);
+      const app = makeRouteApp();
+      registerAgentRoutes(app, ctx);
+      const handler = findHandler(app, 'POST', 'message');
+
+      await handler(makeReq({ params: { id: 'a1' }, body: { message: 'hi' } }), vi.fn(), ctx);
+
+      // killStalePty must be called on every /message so stale cwd is caught
+      expect(killStalePty).toHaveBeenCalledWith(agent);
+    });
+
+    it('BUG 4: reconnect path records worktreePath as ptyCwd', async () => {
+      const agent = makeAgent({
+        id: 'a1',
+        status: 'waiting',
+        projectPath: '/test/project',
+        worktreePath: '/test/project/.worktrees/feat/backend',
+      });
+      agents.set('a1', agent);
 
       const app = makeRouteApp();
       registerAgentRoutes(app, ctx);
       const handler = findHandler(app, 'POST', 'message');
 
-      const sendJson = vi.fn();
-      await handler(makeReq({ params: { id: 'a1' }, body: { message: 'hello' } }), sendJson, ctx);
+      await handler(makeReq({ params: { id: 'a1' }, body: { message: 'go' } }), vi.fn(), ctx);
 
-      expect(ctx.initAgentPtyCallback).toHaveBeenCalledWith(agent);
-      expect(sendJson).toHaveBeenCalledWith({ success: true });
+      // ptyCwd must match the worktree path — this is the cwd bash/claude
+      // actually inherits from pty.spawn. Without it, killStalePty can't
+      // detect a later worktree change.
+      expect(agent.ptyCwd).toBe('/test/project/.worktrees/feat/backend');
+
+      // pty.spawn must receive the raw path (not the shell-escaped version)
+      // so it works for paths that legitimately contain a single quote.
+      const pty = await import('node-pty');
+      expect(pty.spawn).toHaveBeenCalled();
+      const spawnOpts = (pty.spawn as any).mock.calls.at(-1)[2];
+      expect(spawnOpts.cwd).toBe('/test/project/.worktrees/feat/backend');
+    });
+  });
+
+  describe('BUG 4 cwd invariants', () => {
+    it('POST /start uses worktreePath as raw spawn cwd and records ptyCwd', async () => {
+      const agent = makeAgent({
+        id: 'a1',
+        status: 'idle',
+        projectPath: '/test/project',
+        worktreePath: '/test/project/.worktrees/feat/backend',
+      });
+      agents.set('a1', agent);
+
+      const app = makeRouteApp();
+      registerAgentRoutes(app, ctx);
+      const handler = findHandler(app, 'POST', 'start');
+
+      await handler(makeReq({ params: { id: 'a1' }, body: { prompt: 'work' } }), vi.fn(), ctx);
+
+      // ptyCwd must match the logical worktree path so killStalePty has
+      // ground truth to compare against when worktreePath later changes.
+      expect(agent.ptyCwd).toBe('/test/project/.worktrees/feat/backend');
+
+      // pty.spawn must receive the raw (non-shell-escaped) worktree path.
+      // Passing the escaped form would break paths containing single quotes.
+      const pty = await import('node-pty');
+      const spawnOpts = (pty.spawn as any).mock.calls.at(-1)[2];
+      expect(spawnOpts.cwd).toBe('/test/project/.worktrees/feat/backend');
     });
   });
 

electron/core/agent-manager.ts

@@ -16,6 +16,63 @@ import { scheduleTick } from '../utils/agents-tick';
 
 export const agents: Map<string, AgentStatus> = new Map();
 
+/**
+ * Pre-populate Claude Code's workspace trust record for a given directory.
+ *
+ * BUG 6 fix: `--dangerously-skip-permissions` skips *runtime* permission
+ * prompts (Edit/Write/Bash confirmations), but Claude Code has a SEPARATE
+ * "workspace trust" dialog that fires on first launch in an unknown directory.
+ * That dialog is gated by `~/.claude.json`'s
+ * `projects[<absolute-path>].hasTrustDialogAccepted` flag — NOT by the
+ * runtime permission mode. So even a bypass-mode agent hits the trust prompt
+ * on first launch in a new project.
+ *
+ * Writing the flag ourselves before we spawn the claude process makes the
+ * trust dialog never appear. Safe to call repeatedly and idempotent.
+ */
+export function ensureProjectTrusted(projectPath: string): void {
+  if (!projectPath) return;
+  const claudeJsonPath = path.join(os.homedir(), '.claude.json');
+  type ClaudeConfig = {
+    projects?: Record<string, {
+      hasTrustDialogAccepted?: boolean;
+      projectOnboardingSeenCount?: number;
+      [key: string]: unknown;
+    }>;
+    [key: string]: unknown;
+  };
+  let config: ClaudeConfig = {};
+  try {
+    if (fs.existsSync(claudeJsonPath)) {
+      const raw = fs.readFileSync(claudeJsonPath, 'utf-8');
+      if (raw.trim()) {
+        config = JSON.parse(raw) as ClaudeConfig;
+      }
+    }
+  } catch (err) {
+    console.warn(`ensureProjectTrusted: failed to read ${claudeJsonPath}:`, err);
+    // If the file exists but is unreadable/corrupt, don't overwrite it.
+    if (fs.existsSync(claudeJsonPath)) return;
+  }
+
+  if (!config.projects) config.projects = {};
+  const existing = config.projects[projectPath] ?? {};
+  if (existing.hasTrustDialogAccepted === true) return;
+
+  config.projects[projectPath] = {
+    ...existing,
+    hasTrustDialogAccepted: true,
+    projectOnboardingSeenCount: existing.projectOnboardingSeenCount ?? 1,
+  };
+
+  try {
+    fs.writeFileSync(claudeJsonPath, JSON.stringify(config, null, 2));
+    console.log(`ensureProjectTrusted: marked ${projectPath} trusted in ~/.claude.json`);
+  } catch (err) {
+    console.warn(`ensureProjectTrusted: failed to write ${claudeJsonPath}:`, err);
+  }
+}
+
 export let agentsLoaded = false;
 export let superAgentTelegramTask = false;
 export let superAgentOutputBuffer: string[] = [];
@@ -32,6 +89,36 @@ export function clearSuperAgentOutputBuffer() {
   superAgentOutputBuffer = [];
 }
 
+/**
+ * Kill the agent's PTY if its recorded cwd no longer matches the agent's
+ * current logical working directory (worktreePath ?? projectPath). Returns
+ * true if the PTY was killed (caller should respawn before writing to it).
+ *
+ * This prevents the BUG 4 scenario where an agent has a worktree added
+ * after its PTY was created: the running PTY keeps its old cwd, so
+ * subsequent messages land in the main workspace instead of the worktree.
+ */
+export function killStalePty(agent: AgentStatus): boolean {
+  if (!agent.ptyId) return false;
+  const expectedCwd = agent.worktreePath || agent.projectPath;
+  if (agent.ptyCwd === expectedCwd) return false;
+  const existing = ptyProcesses.get(agent.ptyId);
+  if (existing) {
+    try {
+      existing.kill();
+    } catch (err) {
+      console.warn(`Failed to kill stale PTY for agent ${agent.id}:`, err);
+    }
+    ptyProcesses.delete(agent.ptyId);
+  }
+  console.log(
+    `Killed stale PTY for agent ${agent.id}: ptyCwd=${agent.ptyCwd} expected=${expectedCwd}`
+  );
+  agent.ptyId = undefined;
+  agent.ptyCwd = undefined;
+  return true;
+}
+
 const previousAgentStatus: Map<string, string> = new Map();
 
 const pendingStatusChanges: Map<string, {
@@ -211,6 +298,7 @@ export function loadAgents() {
 
       agent.status = 'idle';
       agent.ptyId = undefined;
+      agent.ptyCwd = undefined;
 
       // Migrate legacy skipPermissions boolean → permissionMode
       if (!agent.permissionMode) {
@@ -242,6 +330,10 @@ export async function initAgentPty(
     cwd = os.homedir();
   }
 
+  // BUG 6: pre-accept Claude Code's workspace trust dialog for this cwd so
+  // bypass-mode agents never see the first-launch prompt.
+  ensureProjectTrusted(cwd);
+
   console.log(`Initializing PTY for restored agent ${agent.id} in ${cwd}`);
 
   // Build PATH that includes user-configured paths, nvm, and other common locations for claude
@@ -291,9 +383,16 @@ export async function initAgentPty(
     }
   }
 
-  // Get provider-specific env vars
+  // Get provider-specific env vars. Pass loaded savedSettings so alt
+  // providers (OpenRouter, DeepSeek, etc.) can inject ANTHROPIC_BASE_URL
+  // and ANTHROPIC_API_KEY from the configured API keys.
   const agentProvider = getProvider(agent.provider);
-  const providerEnvVars = agentProvider.getPtyEnvVars(agent.id, agent.projectPath, agent.skills);
+  const providerEnvVars = agentProvider.getPtyEnvVars(
+    agent.id,
+    agent.projectPath,
+    agent.skills,
+    savedSettings as unknown as AppSettings,
+  );
 
   const ptyProcess = pty.spawn(shell, ['-l'], {
     name: 'xterm-256color',
@@ -312,6 +411,7 @@ export async function initAgentPty(
 
   const ptyId = uuidv4();
   ptyProcesses.set(ptyId, ptyProcess);
+  agent.ptyCwd = cwd;
 
   ptyProcess.onData((data) => {
     const agentData = agents.get(agent.id);