refactor: decouple permission state from attention signal
This commit is contained in:
@@ -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,
|
||||
});
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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,
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
});
|
||||
|
||||
@@ -924,6 +924,7 @@ function AgentScreenContent({
|
||||
focusedAgentId,
|
||||
isConnected,
|
||||
requiresAttention: agent?.requiresAttention,
|
||||
attentionReason: agent?.attentionReason,
|
||||
})
|
||||
) {
|
||||
return;
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -995,6 +995,7 @@ export const useSessionStore = create<SessionStore>()(
|
||||
lastActivityAt,
|
||||
cwd: agent.cwd,
|
||||
provider: agent.provider,
|
||||
pendingPermissionCount: agent.pendingPermissions.length,
|
||||
requiresAttention: agent.requiresAttention ?? false,
|
||||
attentionReason: agent.attentionReason ?? null,
|
||||
attentionTimestamp: agent.attentionTimestamp ?? null,
|
||||
|
||||
@@ -14,4 +14,6 @@ export type AgentDirectoryEntry = Pick<
|
||||
| "attentionTimestamp"
|
||||
| "archivedAt"
|
||||
| "labels"
|
||||
>;
|
||||
> & {
|
||||
pendingPermissionCount?: number;
|
||||
};
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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 {
|
||||
|
||||
@@ -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<AgentStreamEvent> {
|
||||
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<boolean> {
|
||||
return true;
|
||||
}
|
||||
|
||||
async createSession(config: AgentSessionConfig): Promise<AgentSession> {
|
||||
return new FailingSession(config);
|
||||
}
|
||||
|
||||
async resumeSession(config?: Partial<AgentSessionConfig>): Promise<AgentSession> {
|
||||
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<AgentStreamEvent> {
|
||||
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<boolean> {
|
||||
return true;
|
||||
}
|
||||
|
||||
async createSession(config: AgentSessionConfig): Promise<AgentSession> {
|
||||
return new PermissionSession(config);
|
||||
}
|
||||
|
||||
async resumeSession(config?: Partial<AgentSessionConfig>): Promise<AgentSession> {
|
||||
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");
|
||||
|
||||
@@ -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 {
|
||||
|
||||
@@ -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') {
|
||||
|
||||
Reference in New Issue
Block a user