diff --git a/packages/app/src/components/sidebar-agent-list.tsx b/packages/app/src/components/sidebar-agent-list.tsx index 04c34dd10..91bc3f733 100644 --- a/packages/app/src/components/sidebar-agent-list.tsx +++ b/packages/app/src/components/sidebar-agent-list.tsx @@ -185,6 +185,7 @@ function SidebarAgentRow({ const relativeCreatedAt = formatTimeAgo(entry.agent.createdAt); const isActive = isSidebarActiveAgent({ status: entry.agent.status, + pendingPermissionCount: entry.agent.pendingPermissionCount, requiresAttention: entry.agent.requiresAttention, attentionReason: entry.agent.attentionReason, }); diff --git a/packages/app/src/hooks/use-aggregated-agents.ts b/packages/app/src/hooks/use-aggregated-agents.ts index fda5c2dbf..f7b4a170a 100644 --- a/packages/app/src/hooks/use-aggregated-agents.ts +++ b/packages/app/src/hooks/use-aggregated-agents.ts @@ -64,6 +64,7 @@ export function useAggregatedAgents(): AggregatedAgentsResult { lastActivityAt: agent.lastActivityAt, cwd: agent.cwd, provider: agent.provider, + pendingPermissionCount: agent.pendingPermissions.length, requiresAttention: agent.requiresAttention, attentionReason: agent.attentionReason, attentionTimestamp: agent.attentionTimestamp, diff --git a/packages/app/src/hooks/use-all-agents-list.ts b/packages/app/src/hooks/use-all-agents-list.ts index f533453d4..c9508a244 100644 --- a/packages/app/src/hooks/use-all-agents-list.ts +++ b/packages/app/src/hooks/use-all-agents-list.ts @@ -26,6 +26,7 @@ function toAggregatedAgent(params: { lastActivityAt: source.lastActivityAt, cwd: source.cwd, provider: source.provider, + pendingPermissionCount: source.pendingPermissions.length, requiresAttention: source.requiresAttention, attentionReason: source.attentionReason, attentionTimestamp: source.attentionTimestamp ?? null, diff --git a/packages/app/src/hooks/use-command-center.ts b/packages/app/src/hooks/use-command-center.ts index be4ab751c..286e6917a 100644 --- a/packages/app/src/hooks/use-command-center.ts +++ b/packages/app/src/hooks/use-command-center.ts @@ -40,6 +40,10 @@ function isMatch(agent: AggregatedAgent, query: string): boolean { } function sortAgents(left: AggregatedAgent, right: AggregatedAgent): number { + const leftNeedsInput = (left.pendingPermissionCount ?? 0) > 0 ? 1 : 0; + const rightNeedsInput = (right.pendingPermissionCount ?? 0) > 0 ? 1 : 0; + if (leftNeedsInput !== rightNeedsInput) return rightNeedsInput - leftNeedsInput; + const leftAttention = left.requiresAttention ? 1 : 0; const rightAttention = right.requiresAttention ? 1 : 0; if (leftAttention !== rightAttention) return rightAttention - leftAttention; diff --git a/packages/app/src/hooks/use-favicon-status.ts b/packages/app/src/hooks/use-favicon-status.ts index 812b04491..006867216 100644 --- a/packages/app/src/hooks/use-favicon-status.ts +++ b/packages/app/src/hooks/use-favicon-status.ts @@ -30,7 +30,8 @@ function deriveFaviconStatus( return "running"; } const hasAttention = agents.some((agent) => agent.requiresAttention); - if (hasAttention) { + const hasNeedsInput = agents.some((agent) => (agent.pendingPermissionCount ?? 0) > 0); + if (hasAttention || hasNeedsInput) { return "attention"; } return "none"; @@ -41,8 +42,8 @@ function deriveMacDockBadgeCount( ): number | undefined { const attentionCount = agents.filter( (agent) => - agent.requiresAttention && - (agent.attentionReason === "permission" || agent.attentionReason === "finished") + (agent.pendingPermissionCount ?? 0) > 0 || + (agent.requiresAttention && agent.attentionReason === "finished") ).length; if (attentionCount > 0) { return attentionCount; diff --git a/packages/app/src/hooks/use-sidebar-agents-list.ts b/packages/app/src/hooks/use-sidebar-agents-list.ts index c67372834..36d7ef7d8 100644 --- a/packages/app/src/hooks/use-sidebar-agents-list.ts +++ b/packages/app/src/hooks/use-sidebar-agents-list.ts @@ -77,6 +77,7 @@ function applySidebarDefaultOrdering( for (const entry of entries) { const bucket = deriveSidebarStateBucket({ status: entry.agent.status, + pendingPermissionCount: entry.agent.pendingPermissionCount, requiresAttention: entry.agent.requiresAttention, attentionReason: entry.agent.attentionReason, }); @@ -135,6 +136,7 @@ function toAggregatedAgent(params: { lastActivityAt: params.lastActivityAt, cwd: source.cwd, provider: source.provider, + pendingPermissionCount: source.pendingPermissions.length, requiresAttention: source.requiresAttention, attentionReason: source.attentionReason, attentionTimestamp: source.attentionTimestamp ?? null, @@ -239,6 +241,7 @@ export function useSidebarAgentsList(options?: { const existing = byProject.get(entry.project.projectKey); const isActive = isSidebarActiveAgent({ status: entry.agent.status, + pendingPermissionCount: entry.agent.pendingPermissionCount, requiresAttention: entry.agent.requiresAttention, attentionReason: entry.agent.attentionReason, }); diff --git a/packages/app/src/screens/agent/agent-ready-screen.tsx b/packages/app/src/screens/agent/agent-ready-screen.tsx index 8e2f2a456..74f2dd575 100644 --- a/packages/app/src/screens/agent/agent-ready-screen.tsx +++ b/packages/app/src/screens/agent/agent-ready-screen.tsx @@ -924,6 +924,7 @@ function AgentScreenContent({ focusedAgentId, isConnected, requiresAttention: agent?.requiresAttention, + attentionReason: agent?.attentionReason, }) ) { return; diff --git a/packages/app/src/screens/agents-screen.tsx b/packages/app/src/screens/agents-screen.tsx index 90d779be4..39c97e73e 100644 --- a/packages/app/src/screens/agents-screen.tsx +++ b/packages/app/src/screens/agents-screen.tsx @@ -29,6 +29,11 @@ export function AgentsScreen({ serverId }: { serverId: string }) { const sortedAgents = useMemo(() => { return [...agents].sort((a, b) => { + const aNeedsInput = (a.pendingPermissionCount ?? 0) > 0; + const bNeedsInput = (b.pendingPermissionCount ?? 0) > 0; + if (aNeedsInput && !bNeedsInput) return -1; + if (!aNeedsInput && bNeedsInput) return 1; + if (a.requiresAttention && !b.requiresAttention) return -1; if (!a.requiresAttention && b.requiresAttention) return 1; return 0; diff --git a/packages/app/src/stores/session-store.ts b/packages/app/src/stores/session-store.ts index 5046fc10d..89a797864 100644 --- a/packages/app/src/stores/session-store.ts +++ b/packages/app/src/stores/session-store.ts @@ -995,6 +995,7 @@ export const useSessionStore = create()( lastActivityAt, cwd: agent.cwd, provider: agent.provider, + pendingPermissionCount: agent.pendingPermissions.length, requiresAttention: agent.requiresAttention ?? false, attentionReason: agent.attentionReason ?? null, attentionTimestamp: agent.attentionTimestamp ?? null, diff --git a/packages/app/src/types/agent-directory.ts b/packages/app/src/types/agent-directory.ts index 3fb61f586..18bd68c56 100644 --- a/packages/app/src/types/agent-directory.ts +++ b/packages/app/src/types/agent-directory.ts @@ -14,4 +14,6 @@ export type AgentDirectoryEntry = Pick< | "attentionTimestamp" | "archivedAt" | "labels" ->; +> & { + pendingPermissionCount?: number; +}; diff --git a/packages/app/src/utils/agent-attention.test.ts b/packages/app/src/utils/agent-attention.test.ts index 8b7b4e674..d0246ed1f 100644 --- a/packages/app/src/utils/agent-attention.test.ts +++ b/packages/app/src/utils/agent-attention.test.ts @@ -10,6 +10,7 @@ describe("shouldClearAgentAttentionOnView", () => { focusedAgentId: "agent-1", isConnected: true, requiresAttention: true, + attentionReason: "finished", }) ).toBe(true); }); @@ -21,6 +22,7 @@ describe("shouldClearAgentAttentionOnView", () => { focusedAgentId: "agent-1", isConnected: false, requiresAttention: true, + attentionReason: "finished", }) ).toBe(false); }); @@ -32,6 +34,7 @@ describe("shouldClearAgentAttentionOnView", () => { focusedAgentId: "agent-2", isConnected: true, requiresAttention: true, + attentionReason: "finished", }) ).toBe(false); }); @@ -43,6 +46,7 @@ describe("shouldClearAgentAttentionOnView", () => { focusedAgentId: "agent-1", isConnected: true, requiresAttention: false, + attentionReason: null, }) ).toBe(false); }); @@ -54,6 +58,19 @@ describe("shouldClearAgentAttentionOnView", () => { focusedAgentId: "agent-1", isConnected: true, requiresAttention: true, + attentionReason: "finished", + }) + ).toBe(false); + }); + + it("returns false for permission-shaped attention", () => { + expect( + shouldClearAgentAttentionOnView({ + agentId: "agent-1", + focusedAgentId: "agent-1", + isConnected: true, + requiresAttention: true, + attentionReason: "permission", }) ).toBe(false); }); diff --git a/packages/app/src/utils/agent-attention.ts b/packages/app/src/utils/agent-attention.ts index ea102faa2..664563753 100644 --- a/packages/app/src/utils/agent-attention.ts +++ b/packages/app/src/utils/agent-attention.ts @@ -3,6 +3,7 @@ interface ShouldClearAgentAttentionOnViewInput { focusedAgentId: string | null | undefined; isConnected: boolean; requiresAttention: boolean | null | undefined; + attentionReason?: "finished" | "error" | "permission" | null | undefined; } export function shouldClearAgentAttentionOnView( @@ -18,5 +19,8 @@ export function shouldClearAgentAttentionOnView( if (!input.requiresAttention) { return false; } + if (input.attentionReason === "permission") { + return false; + } return input.focusedAgentId === agentId; } diff --git a/packages/app/src/utils/agent-grouping.ts b/packages/app/src/utils/agent-grouping.ts index c57e061c1..067a20193 100644 --- a/packages/app/src/utils/agent-grouping.ts +++ b/packages/app/src/utils/agent-grouping.ts @@ -192,7 +192,7 @@ export interface ProjectGroup { projectKey: string; projectName: string; agents: AggregatedAgent[]; - /** Number of truly active agents (running or requires attention) */ + /** Number of truly active agents (running, needs input, or requires attention) */ activeCount: number; /** Total agents before any limit was applied */ totalCount: number; @@ -222,11 +222,11 @@ const MAX_INACTIVE_PER_PROJECT = 5; /** * Groups agents into active (by project) and inactive (by date) sections. - * Active = running, requires attention, or had activity within the grace period (24 hours). + * Active = running, needs input, requires attention, or had activity within the grace period. * * Within each project group: - * - All truly active agents (running/requires attention) are always shown - * - Recently active (within grace period but not running) are limited to MAX_INACTIVE_PER_PROJECT + * - All truly active agents (running/needs input/requires attention) are always shown + * - Recently active (within grace period but not truly active) are limited to MAX_INACTIVE_PER_PROJECT */ export function groupAgents( agents: AggregatedAgent[], @@ -244,7 +244,9 @@ export function groupAgents( } const isRunningOrAttention = - agent.status === "running" || agent.requiresAttention; + agent.status === "running" || + agent.requiresAttention || + (agent.pendingPermissionCount ?? 0) > 0; const ageDiff = now - agent.lastActivityAt.getTime(); const isRecentlyActive = ageDiff < ACTIVE_GRACE_PERIOD_MS; const isActive = isRunningOrAttention || isRecentlyActive; @@ -272,7 +274,9 @@ export function groupAgents( }; const isTrulyActive = - agent.status === "running" || agent.requiresAttention; + agent.status === "running" || + agent.requiresAttention || + (agent.pendingPermissionCount ?? 0) > 0; if (isTrulyActive) { existing.trulyActive.push(agent); } else { diff --git a/packages/app/src/utils/sidebar-agent-state.test.ts b/packages/app/src/utils/sidebar-agent-state.test.ts new file mode 100644 index 000000000..1377aa1c0 --- /dev/null +++ b/packages/app/src/utils/sidebar-agent-state.test.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from "vitest"; +import { deriveSidebarStateBucket } from "./sidebar-agent-state"; + +describe("deriveSidebarStateBucket", () => { + it("prioritizes pending permissions as needs_input", () => { + expect( + deriveSidebarStateBucket({ + status: "idle", + pendingPermissionCount: 1, + requiresAttention: false, + attentionReason: null, + }) + ).toBe("needs_input"); + }); + + it("keeps legacy permission attention in needs_input", () => { + expect( + deriveSidebarStateBucket({ + status: "idle", + pendingPermissionCount: 0, + requiresAttention: true, + attentionReason: "permission", + }) + ).toBe("needs_input"); + }); + + it("treats unread finished agents as attention", () => { + expect( + deriveSidebarStateBucket({ + status: "idle", + pendingPermissionCount: 0, + requiresAttention: true, + attentionReason: "finished", + }) + ).toBe("attention"); + }); +}); diff --git a/packages/app/src/utils/sidebar-agent-state.ts b/packages/app/src/utils/sidebar-agent-state.ts index ff2c23fde..627e63339 100644 --- a/packages/app/src/utils/sidebar-agent-state.ts +++ b/packages/app/src/utils/sidebar-agent-state.ts @@ -16,10 +16,16 @@ export type SidebarStateBucket = export function deriveSidebarStateBucket(input: { status: AgentLifecycleStatus; + pendingPermissionCount?: number; requiresAttention?: boolean; attentionReason?: SidebarAttentionReason; }): SidebarStateBucket { - if (input.requiresAttention && input.attentionReason === "permission") { + if ((input.pendingPermissionCount ?? 0) > 0) { + return "needs_input"; + } + // Legacy fallback for snapshots persisted before permission state was decoupled + // from unread attention. + if (input.attentionReason === "permission") { return "needs_input"; } if (input.status === "error" || input.attentionReason === "error") { @@ -37,6 +43,7 @@ export function deriveSidebarStateBucket(input: { export function isSidebarActiveAgent(input: { status: AgentLifecycleStatus; + pendingPermissionCount?: number; requiresAttention?: boolean; attentionReason?: SidebarAttentionReason; }): boolean { diff --git a/packages/server/src/server/agent/agent-manager.test.ts b/packages/server/src/server/agent/agent-manager.test.ts index da014e936..6461e9554 100644 --- a/packages/server/src/server/agent/agent-manager.test.ts +++ b/packages/server/src/server/agent/agent-manager.test.ts @@ -1699,6 +1699,171 @@ describe("AgentManager", () => { expect(attentionCalls).toHaveLength(0); }); + test("clearAgentAttention on errored agent stays cleared until a new error transition", async () => { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-attention-error-")); + const storagePath = join(workdir, "agents"); + const storage = new AgentStorage(storagePath, logger); + + class FailingSession extends TestAgentSession { + private attempt = 0; + + async *stream(): AsyncGenerator { + this.attempt += 1; + yield { type: "turn_started", provider: this.provider }; + throw new Error(`boom-${this.attempt}`); + } + } + + class FailingClient implements AgentClient { + readonly provider = "codex" as const; + readonly capabilities = TEST_CAPABILITIES; + + async isAvailable(): Promise { + return true; + } + + async createSession(config: AgentSessionConfig): Promise { + return new FailingSession(config); + } + + async resumeSession(config?: Partial): Promise { + return new FailingSession({ + provider: "codex", + cwd: config?.cwd ?? process.cwd(), + }); + } + } + + const attentionReasons: Array<"finished" | "error" | "permission"> = []; + const manager = new AgentManager({ + clients: { + codex: new FailingClient(), + }, + registry: storage, + logger, + idFactory: () => "00000000-0000-4000-8000-000000000130", + onAgentAttention: ({ reason }) => { + attentionReasons.push(reason); + }, + }); + + const agent = await manager.createAgent({ + provider: "codex", + cwd: workdir, + title: "Attention transition test", + }); + + await expect(manager.runAgent(agent.id, "fail once")).rejects.toThrow("boom-1"); + + const afterFirstFailure = manager.getAgent(agent.id); + expect(afterFirstFailure?.lifecycle).toBe("error"); + expect(afterFirstFailure?.attention.requiresAttention).toBe(true); + expect(afterFirstFailure?.attention).toMatchObject({ + requiresAttention: true, + attentionReason: "error", + }); + + await manager.clearAgentAttention(agent.id); + manager.notifyAgentState(agent.id); + + const afterClear = manager.getAgent(agent.id); + expect(afterClear?.lifecycle).toBe("error"); + expect(afterClear?.attention).toEqual({ requiresAttention: false }); + + await expect(manager.runAgent(agent.id, "fail again")).rejects.toThrow("boom-2"); + + const afterSecondFailure = manager.getAgent(agent.id); + expect(afterSecondFailure?.lifecycle).toBe("error"); + expect(afterSecondFailure?.attention).toMatchObject({ + requiresAttention: true, + attentionReason: "error", + }); + expect(attentionReasons).toEqual(["error", "error"]); + }); + + test("permission request notifies once without forcing unread attention state", async () => { + const workdir = mkdtempSync(join(tmpdir(), "agent-manager-attention-permission-")); + const storagePath = join(workdir, "agents"); + const storage = new AgentStorage(storagePath, logger); + + class PermissionSession extends TestAgentSession { + async *stream(): AsyncGenerator { + yield { type: "turn_started", provider: this.provider }; + yield { + type: "permission_requested", + provider: this.provider, + request: { + id: "perm-1", + provider: this.provider, + kind: "tool", + name: "Read file", + }, + }; + yield { + type: "permission_resolved", + provider: this.provider, + requestId: "perm-1", + resolution: { behavior: "allow" }, + }; + yield { type: "turn_completed", provider: this.provider }; + } + } + + class PermissionClient implements AgentClient { + readonly provider = "codex" as const; + readonly capabilities = TEST_CAPABILITIES; + + async isAvailable(): Promise { + return true; + } + + async createSession(config: AgentSessionConfig): Promise { + return new PermissionSession(config); + } + + async resumeSession(config?: Partial): Promise { + return new PermissionSession({ + provider: "codex", + cwd: config?.cwd ?? process.cwd(), + }); + } + } + + const attentionReasons: Array<"finished" | "error" | "permission"> = []; + const manager = new AgentManager({ + clients: { + codex: new PermissionClient(), + }, + registry: storage, + logger, + idFactory: () => "00000000-0000-4000-8000-000000000131", + onAgentAttention: ({ reason }) => { + attentionReasons.push(reason); + }, + }); + + const agent = await manager.createAgent({ + provider: "codex", + cwd: workdir, + title: "Permission transition test", + }); + + const stream = manager.streamAgent(agent.id, "permission flow"); + await stream.next(); // turn_started + await stream.next(); // permission_requested + + const withPermissionPending = manager.getAgent(agent.id); + expect(withPermissionPending?.pendingPermissions.size).toBe(1); + expect(withPermissionPending?.attention).toEqual({ requiresAttention: false }); + + // Drain the rest of the stream to close cleanly. + while (!(await stream.next()).done) { + // no-op + } + + expect(attentionReasons).toContain("permission"); + }); + test("respondToPermission updates currentModeId after plan approval", async () => { const workdir = mkdtempSync(join(tmpdir(), "agent-manager-test-")); const storagePath = join(workdir, "agents"); diff --git a/packages/server/src/server/agent/agent-manager.ts b/packages/server/src/server/agent/agent-manager.ts index 5b3ee6ea9..690d96777 100644 --- a/packages/server/src/server/agent/agent-manager.ts +++ b/packages/server/src/server/agent/agent-manager.ts @@ -1776,7 +1776,13 @@ export class AgentManager { } break; case "permission_requested": - agent.pendingPermissions.set(event.request.id, event.request); + { + const hadPendingPermissions = agent.pendingPermissions.size > 0; + agent.pendingPermissions.set(event.request.id, event.request); + if (!hadPendingPermissions && !agent.internal) { + this.broadcastAgentAttention(agent, "permission"); + } + } this.emitState(agent); break; case "permission_resolved": @@ -1849,7 +1855,7 @@ export class AgentManager { } private emitState(agent: ManagedAgent): void { - // Check if attention should be set based on status change + // Keep attention as an edge-triggered unread signal, not a level signal. this.checkAndSetAttention(agent); this.dispatch({ @@ -1888,7 +1894,7 @@ export class AgentManager { } // Check if agent entered error state - if (currentStatus === "error") { + if (previousStatus !== "error" && currentStatus === "error") { agent.attention = { requiresAttention: true, attentionReason: "error", @@ -1898,18 +1904,6 @@ export class AgentManager { this.enqueueBackgroundPersist(agent); return; } - - // Check if agent has pending permissions - if (agent.pendingPermissions.size > 0) { - agent.attention = { - requiresAttention: true, - attentionReason: "permission", - attentionTimestamp: new Date(), - }; - this.broadcastAgentAttention(agent, "permission"); - this.enqueueBackgroundPersist(agent); - return; - } } private enqueueBackgroundPersist(agent: ManagedAgent): void { diff --git a/packages/server/src/server/session.ts b/packages/server/src/server/session.ts index 84c7a4758..cb067ddd5 100644 --- a/packages/server/src/server/session.ts +++ b/packages/server/src/server/session.ts @@ -4894,9 +4894,9 @@ export class Session { } private getStatusPriority(agent: AgentSnapshotPayload): number { - const requiresAttention = agent.requiresAttention ?? false const attentionReason = agent.attentionReason ?? null - if (requiresAttention && attentionReason === 'permission') { + const hasPendingPermission = (agent.pendingPermissions?.length ?? 0) > 0 + if (hasPendingPermission || attentionReason === 'permission') { return 0 } if (agent.status === 'error' || attentionReason === 'error') {