diff --git a/.changeset/rename-worker-sessions.md b/.changeset/rename-worker-sessions.md new file mode 100644 index 000000000..528c43ff1 --- /dev/null +++ b/.changeset/rename-worker-sessions.md @@ -0,0 +1,5 @@ +--- +"@aoagents/ao-web": minor +--- + +Add inline rename for worker sessions in the sidebar. Each worker row now shows a small pencil button on hover; clicking it swaps the label for an input pre-filled with the current title. Enter persists via `PATCH /api/sessions/:id`, Escape cancels, and an empty value reverts the session to its default title. The rename is written to the existing `displayName` metadata field and is now the highest-priority signal in `getSessionTitle`, so a user-chosen label always beats PR/issue titles. The session ID (`ao-N`) remains the canonical identifier — only display surfaces change. (#1647) diff --git a/packages/core/src/__tests__/metadata.test.ts b/packages/core/src/__tests__/metadata.test.ts index 0ff20a4ef..03ac1c16b 100644 --- a/packages/core/src/__tests__/metadata.test.ts +++ b/packages/core/src/__tests__/metadata.test.ts @@ -150,6 +150,66 @@ describe("writeMetadata + readMetadata", () => { const meta = readMetadata(dataDir, "app-6"); expect(meta?.displayName).toBe("Refactor session manager"); }); + + it("serializes and reads back displayNameUserSet flag", () => { + writeMetadata(dataDir, "app-7", { + worktree: "/tmp/w", + branch: "feat/test", + status: "working", + displayName: "PR 1466 review", + displayNameUserSet: true, + }); + + const content = readFileSync(join(dataDir, "app-7.json"), "utf-8"); + const parsed = JSON.parse(content); + expect(parsed.displayNameUserSet).toBe(true); + + const meta = readMetadata(dataDir, "app-7"); + expect(meta?.displayNameUserSet).toBe(true); + }); + + it("accepts on/off and true/false for displayNameUserSet (matches prAutoDetect)", () => { + // Defensive: storage paths that flow through unflattenFromStringRecord + // already convert "on"/"off" → boolean before write, but readMetadata + // should still tolerate the legacy string forms for parity with prAutoDetect. + for (const [stored, expected] of [ + ["on", true], + ["off", false], + ["true", true], + ["false", false], + [true, true], + [false, false], + ] as const) { + writeFileSync( + join(dataDir, `flag-${String(stored)}.json`), + JSON.stringify({ + worktree: "/tmp/w", + branch: "feat/test", + status: "working", + displayNameUserSet: stored, + }), + "utf-8", + ); + const meta = readMetadata(dataDir, `flag-${String(stored)}` as never); + expect(meta?.displayNameUserSet).toBe(expected); + } + }); + + it("omits displayNameUserSet when undefined and does not flag auto-derived sessions", () => { + writeMetadata(dataDir, "app-8", { + worktree: "/tmp/w", + branch: "feat/test", + status: "working", + displayName: "Auto-derived at spawn", + }); + + const content = readFileSync(join(dataDir, "app-8.json"), "utf-8"); + const parsed = JSON.parse(content); + expect(parsed.displayNameUserSet).toBeUndefined(); + + const meta = readMetadata(dataDir, "app-8"); + expect(meta?.displayNameUserSet).toBeUndefined(); + }); }); describe("readMetadataRaw", () => { diff --git a/packages/core/src/metadata.ts b/packages/core/src/metadata.ts index 95d354f77..a4f3a19c5 100644 --- a/packages/core/src/metadata.ts +++ b/packages/core/src/metadata.ts @@ -174,6 +174,16 @@ export function readMetadata(dataDir: string, sessionId: SessionId): SessionMeta pinnedSummary: raw["pinnedSummary"] as string | undefined, userPrompt: raw["userPrompt"] as string | undefined, displayName: raw["displayName"] as string | undefined, + displayNameUserSet: + raw["displayNameUserSet"] === "off" || + raw["displayNameUserSet"] === "false" || + raw["displayNameUserSet"] === false + ? false + : raw["displayNameUserSet"] === "on" || + raw["displayNameUserSet"] === "true" || + raw["displayNameUserSet"] === true + ? true + : undefined, }; } @@ -218,7 +228,7 @@ const jsonFields = new Set([ function unflattenFromStringRecord(data: Record): Record { const result: Record = {}; const numberFields = new Set(["dashboardPort", "terminalWsPort", "directTerminalWsPort"]); - const booleanFields = new Set(["prAutoDetect"]); + const booleanFields = new Set(["prAutoDetect", "displayNameUserSet"]); for (const [key, value] of Object.entries(data)) { if (value === undefined || value === "") continue; @@ -281,6 +291,8 @@ export function writeMetadata( if (metadata.pinnedSummary) data["pinnedSummary"] = metadata.pinnedSummary; if (metadata.userPrompt) data["userPrompt"] = metadata.userPrompt; if (metadata.displayName) data["displayName"] = metadata.displayName; + if (metadata.displayNameUserSet !== undefined) + data["displayNameUserSet"] = metadata.displayNameUserSet; atomicWriteFileSync(path, serializeMetadata(data)); } diff --git a/packages/core/src/types.ts b/packages/core/src/types.ts index 47dbabba7..7d10d10c4 100644 --- a/packages/core/src/types.ts +++ b/packages/core/src/types.ts @@ -1764,13 +1764,26 @@ export interface SessionMetadata { pinnedSummary?: string; // First quality summary, pinned for display stability userPrompt?: string; // Prompt used when spawning without a tracker issue /** - * Stable human-readable display name derived from task context at spawn time. - * Populated from issue title, user prompt, or orchestrator system prompt — - * whichever was available when the session was created. Used by the dashboard - * as a fallback above humanized branch names so sessions are identifiable - * even when PR/issue enrichment is unavailable. + * Human-readable display name for the session. + * + * Populated automatically at spawn time from the best available task context + * (issue title, user prompt, or orchestrator system prompt). Can be + * overwritten later via the dashboard rename UI — the session ID (`ao-N`) + * remains the canonical identifier; only display surfaces are affected. + * + * Whether this value should beat PR/issue titles in the dashboard depends + * on `displayNameUserSet` — auto-derived values stay below live tracker + * signals, user-set values win over them. */ displayName?: string; + /** + * Set to `true` when the user explicitly renamed the session via the + * dashboard. The dashboard fallback chain promotes `displayName` above + * PR/issue titles only when this flag is true, so an auto-derived spawn-time + * `displayName` doesn't shadow a live PR title for sessions the user never + * touched. + */ + displayNameUserSet?: boolean; } // ============================================================================= diff --git a/packages/web/src/__tests__/api-routes.test.ts b/packages/web/src/__tests__/api-routes.test.ts index 5b8b4f6d3..60ff21c15 100644 --- a/packages/web/src/__tests__/api-routes.test.ts +++ b/packages/web/src/__tests__/api-routes.test.ts @@ -5,6 +5,7 @@ import { SessionNotRestorableError, createInitialCanonicalLifecycle, createActivitySignal, + updateMetadata, type Session, type SessionManager, type OrchestratorConfig, @@ -209,10 +210,28 @@ vi.mock("@/lib/services", () => ({ getSCM: vi.fn(() => mockSCM), })); +// Mock filesystem-touching core helpers so PATCH /api/sessions/:id doesn't +// write to the user's actual ~/.agent-orchestrator dir during tests. Spread +// the real module first so the rest of the test file (types, errors, etc.) +// keeps working. Factory must self-contain its mocks because vi.mock is +// hoisted above any module-level declarations. +vi.mock("@aoagents/ao-core", async (importOriginal) => { + const actual = (await importOriginal()) as Record; + return { + ...actual, + updateMetadata: vi.fn(), + getProjectSessionsDir: vi.fn(() => "/tmp/ao-test/sessions"), + readAgentReportAuditTrailAsync: vi.fn(async () => []), + }; +}); + // ── Import routes after mocking ─────────────────────────────────────── import { GET as sessionsGET } from "@/app/api/sessions/route"; -import { GET as sessionDetailGET } from "@/app/api/sessions/[id]/route"; +import { + GET as sessionDetailGET, + PATCH as sessionDetailPATCH, +} from "@/app/api/sessions/[id]/route"; import { POST as orchestratorsPOST, GET as orchestratorsGET } from "@/app/api/orchestrators/route"; import { POST as spawnPOST } from "@/app/api/spawn/route"; import { POST as sendPOST } from "@/app/api/sessions/[id]/send/route"; @@ -356,7 +375,10 @@ describe("API Routes", () => { }); it("prefers the most recently active live orchestrator for project-scoped worker navigation", async () => { - const deadLifecycle = createInitialCanonicalLifecycle("orchestrator", new Date("2026-04-19T11:00:00.000Z")); + const deadLifecycle = createInitialCanonicalLifecycle( + "orchestrator", + new Date("2026-04-19T11:00:00.000Z"), + ); deadLifecycle.session.state = "terminated"; deadLifecycle.session.reason = "runtime_missing"; deadLifecycle.session.terminatedAt = "2026-04-19T11:00:00.000Z"; @@ -414,7 +436,10 @@ describe("API Routes", () => { }); it("keeps dead orchestrators as the fallback project-scoped payload when none are live", async () => { - const deadLifecycle = createInitialCanonicalLifecycle("orchestrator", new Date("2026-04-19T11:00:00.000Z")); + const deadLifecycle = createInitialCanonicalLifecycle( + "orchestrator", + new Date("2026-04-19T11:00:00.000Z"), + ); deadLifecycle.session.state = "terminated"; deadLifecycle.session.reason = "runtime_missing"; deadLifecycle.session.terminatedAt = "2026-04-19T11:00:00.000Z"; @@ -449,7 +474,10 @@ describe("API Routes", () => { }); it("prefers the most recently active dead orchestrator when no live project orchestrator exists", async () => { - const olderDeadLifecycle = createInitialCanonicalLifecycle("orchestrator", new Date("2026-04-19T10:00:00.000Z")); + const olderDeadLifecycle = createInitialCanonicalLifecycle( + "orchestrator", + new Date("2026-04-19T10:00:00.000Z"), + ); olderDeadLifecycle.session.state = "terminated"; olderDeadLifecycle.session.reason = "runtime_missing"; olderDeadLifecycle.session.terminatedAt = "2026-04-19T10:00:00.000Z"; @@ -458,7 +486,10 @@ describe("API Routes", () => { olderDeadLifecycle.runtime.reason = "process_missing"; olderDeadLifecycle.runtime.lastObservedAt = "2026-04-19T10:00:00.000Z"; - const newerDeadLifecycle = createInitialCanonicalLifecycle("orchestrator", new Date("2026-04-19T11:00:00.000Z")); + const newerDeadLifecycle = createInitialCanonicalLifecycle( + "orchestrator", + new Date("2026-04-19T11:00:00.000Z"), + ); newerDeadLifecycle.session.state = "terminated"; newerDeadLifecycle.session.reason = "runtime_missing"; newerDeadLifecycle.session.terminatedAt = "2026-04-19T11:00:00.000Z"; @@ -522,17 +553,20 @@ describe("API Routes", () => { }, }), ); - (mockSessionManager.listCached as ReturnType).mockResolvedValue(sessionsWithPRs); + (mockSessionManager.listCached as ReturnType).mockResolvedValue( + sessionsWithPRs, + ); const metadataSpy = vi .spyOn(serialize, "enrichSessionsMetadata") .mockResolvedValue(undefined); - const enrichSpy = vi - .spyOn(serialize, "enrichSessionPR") - .mockImplementation( - () => new Promise((resolve) => { setTimeout(resolve, 1_000); }), - ); + const enrichSpy = vi.spyOn(serialize, "enrichSessionPR").mockImplementation( + () => + new Promise((resolve) => { + setTimeout(resolve, 1_000); + }), + ); const responsePromise = sessionsGET(makeRequest("http://localhost:3000/api/sessions")); @@ -598,7 +632,9 @@ describe("API Routes", () => { }, }), ]; - (mockSessionManager.listCached as ReturnType).mockResolvedValue(sessionsWithPRs); + (mockSessionManager.listCached as ReturnType).mockResolvedValue( + sessionsWithPRs, + ); const metadataSpy = vi .spyOn(serialize, "enrichSessionsMetadata") @@ -642,7 +678,8 @@ describe("API Routes", () => { const runtimeTerminalLifecycle = createInitialCanonicalLifecycle("worker", new Date()); runtimeTerminalLifecycle.session.state = "terminated"; runtimeTerminalLifecycle.session.reason = "user_killed"; - runtimeTerminalLifecycle.session.terminatedAt = runtimeTerminalLifecycle.session.lastTransitionAt; + runtimeTerminalLifecycle.session.terminatedAt = + runtimeTerminalLifecycle.session.lastTransitionAt; runtimeTerminalLifecycle.runtime.state = "missing"; runtimeTerminalLifecycle.runtime.reason = "process_missing"; runtimeTerminalLifecycle.pr.state = "open"; @@ -666,15 +703,15 @@ describe("API Routes", () => { }, }), ]; - (mockSessionManager.listCached as ReturnType).mockResolvedValue(sessionWithOpenPR); + (mockSessionManager.listCached as ReturnType).mockResolvedValue( + sessionWithOpenPR, + ); const metadataSpy = vi .spyOn(serialize, "enrichSessionsMetadata") .mockResolvedValue(undefined); - const enrichSpy = vi - .spyOn(serialize, "enrichSessionPR") - .mockResolvedValue(true); + const enrichSpy = vi.spyOn(serialize, "enrichSessionPR").mockResolvedValue(true); const res = await sessionsGET(makeRequest("http://localhost:3000/api/sessions")); @@ -717,6 +754,107 @@ describe("API Routes", () => { }, 10_000); }); + // ── PATCH /api/sessions/[id] ─────────────────────────────────────── + + describe("PATCH /api/sessions/[id]", () => { + function patchRequest(id: string, body: unknown): NextRequest { + return makeRequest(`http://localhost:3000/api/sessions/${id}`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify(body), + }); + } + + beforeEach(() => { + vi.mocked(updateMetadata).mockReset(); + }); + + it("persists a sanitized displayName and sets the user-set flag", async () => { + const res = await sessionDetailPATCH( + patchRequest("backend-7", { displayName: " PR 432 review " }), + { + params: Promise.resolve({ id: "backend-7" }), + }, + ); + + expect(res.status).toBe(200); + expect(vi.mocked(updateMetadata)).toHaveBeenCalledTimes(1); + const [, sessionId, updates] = vi.mocked(updateMetadata).mock.calls[0]; + expect(sessionId).toBe("backend-7"); + // Whitespace is collapsed and trimmed before persist; flag is set so the + // dashboard knows to promote this name above PR/issue titles. + expect(updates).toEqual({ displayName: "PR 432 review", displayNameUserSet: "true" }); + expect(mockSessionManager.invalidateCache).toHaveBeenCalled(); + }); + + it("treats null displayName as a clear and unsets the user-set flag", async () => { + const res = await sessionDetailPATCH(patchRequest("backend-7", { displayName: null }), { + params: Promise.resolve({ id: "backend-7" }), + }); + + expect(res.status).toBe(200); + expect(vi.mocked(updateMetadata)).toHaveBeenCalledWith(expect.any(String), "backend-7", { + displayName: "", + displayNameUserSet: "", + }); + }); + + it("strips control characters", async () => { + const res = await sessionDetailPATCH(patchRequest("backend-7", { displayName: "FooBar" }), { + params: Promise.resolve({ id: "backend-7" }), + }); + expect(res.status).toBe(200); + expect(vi.mocked(updateMetadata)).toHaveBeenCalledWith(expect.any(String), "backend-7", { + displayName: "FooBar", + displayNameUserSet: "true", + }); + }); + + it("truncates names longer than 80 characters", async () => { + const longName = "a".repeat(120); + const res = await sessionDetailPATCH(patchRequest("backend-7", { displayName: longName }), { + params: Promise.resolve({ id: "backend-7" }), + }); + expect(res.status).toBe(200); + const [, , updates] = vi.mocked(updateMetadata).mock.calls[0]; + expect((updates as { displayName: string }).displayName.length).toBe(80); + }); + + it("returns 400 when displayName field is missing", async () => { + const res = await sessionDetailPATCH(patchRequest("backend-7", {}), { + params: Promise.resolve({ id: "backend-7" }), + }); + expect(res.status).toBe(400); + expect(vi.mocked(updateMetadata)).not.toHaveBeenCalled(); + }); + + it("returns 400 when displayName is not a string or null", async () => { + const res = await sessionDetailPATCH(patchRequest("backend-7", { displayName: 42 }), { + params: Promise.resolve({ id: "backend-7" }), + }); + expect(res.status).toBe(400); + expect(vi.mocked(updateMetadata)).not.toHaveBeenCalled(); + }); + + it("returns 400 on invalid JSON", async () => { + const req = makeRequest("http://localhost:3000/api/sessions/backend-7", { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: "{not json", + }); + const res = await sessionDetailPATCH(req, { params: Promise.resolve({ id: "backend-7" }) }); + expect(res.status).toBe(400); + }); + + it("returns 404 when the session is unknown", async () => { + const res = await sessionDetailPATCH(patchRequest("does-not-exist", { displayName: "x" }), { + params: Promise.resolve({ id: "does-not-exist" }), + }); + expect(res.status).toBe(404); + expect(vi.mocked(updateMetadata)).not.toHaveBeenCalled(); + }); + }); + describe("GET /api/runtime/terminal", () => { function withEnv(overrides: Record, fn: () => Promise) { const saved: Record = {}; @@ -1042,7 +1180,9 @@ describe("API Routes", () => { }); it("returns 500 when list fails", async () => { - (mockSessionManager.list as ReturnType).mockRejectedValueOnce(new Error("boom")); + (mockSessionManager.list as ReturnType).mockRejectedValueOnce( + new Error("boom"), + ); const res = await orchestratorsGET( makeRequest("http://localhost:3000/api/orchestrators?project=my-app"), ); diff --git a/packages/web/src/__tests__/helpers.ts b/packages/web/src/__tests__/helpers.ts index 9226da4ce..38d9544eb 100644 --- a/packages/web/src/__tests__/helpers.ts +++ b/packages/web/src/__tests__/helpers.ts @@ -205,6 +205,7 @@ export function makeSession(overrides: Partial = {}): Dashboar issueTitle: null, userPrompt: null, displayName: null, + displayNameUserSet: false, summary: "Test session", summaryIsFallback: false, createdAt: new Date().toISOString(), diff --git a/packages/web/src/app/api/sessions/[id]/route.ts b/packages/web/src/app/api/sessions/[id]/route.ts index db0ce8128..da7dd77bf 100644 --- a/packages/web/src/app/api/sessions/[id]/route.ts +++ b/packages/web/src/app/api/sessions/[id]/route.ts @@ -1,5 +1,10 @@ import { type NextRequest } from "next/server"; -import { getProjectSessionsDir, readAgentReportAuditTrailAsync } from "@aoagents/ao-core"; +import { + getProjectSessionsDir, + isOpenCodeSessionManager, + readAgentReportAuditTrailAsync, + updateMetadata, +} from "@aoagents/ao-core"; import { getServices } from "@/lib/services"; import { sessionToDashboard, @@ -8,10 +13,13 @@ import { enrichSessionsMetadata, } from "@/lib/serialize"; import { settlesWithin } from "@/lib/async-utils"; +import { stripControlChars, validateIdentifier } from "@/lib/validation"; import { getCorrelationId, jsonWithCorrelation, recordApiObservation } from "@/lib/observability"; const AGENT_REPORT_AUDIT_TIMEOUT_MS = 1000; const METADATA_ENRICH_TIMEOUT_MS = 3000; +/** Max length of the user-set display name. Matches the spawn-time derivation cap. */ +const DISPLAY_NAME_MAX_LENGTH = 80; export async function GET(_request: NextRequest, { params }: { params: Promise<{ id: string }> }) { const correlationId = getCorrelationId(_request); @@ -29,9 +37,11 @@ export async function GET(_request: NextRequest, { params }: { params: Promise<{ const project = resolveProject(coreSession, config.projects); if (project) { const sessionsDir = getProjectSessionsDir(coreSession.projectId); - const auditPromise = readAgentReportAuditTrailAsync(sessionsDir, coreSession.id).then((audit) => { - dashboardSession.agentReportAudit = audit; - }); + const auditPromise = readAgentReportAuditTrailAsync(sessionsDir, coreSession.id).then( + (audit) => { + dashboardSession.agentReportAudit = audit; + }, + ); await settlesWithin(auditPromise, AGENT_REPORT_AUDIT_TIMEOUT_MS); } @@ -83,3 +93,118 @@ export async function GET(_request: NextRequest, { params }: { params: Promise<{ return jsonWithCorrelation({ error: "Internal server error" }, { status: 500 }, correlationId); } } + +/** + * PATCH /api/sessions/:id — update mutable fields on a session. + * + * Currently supports: + * - `displayName` (string | null): user-set rename. Empty string or `null` + * clears the field, reverting the session to its default title. + */ +export async function PATCH(request: NextRequest, { params }: { params: Promise<{ id: string }> }) { + const correlationId = getCorrelationId(request); + const startedAt = Date.now(); + const { id } = await params; + + const idErr = validateIdentifier(id, "id"); + if (idErr) { + return jsonWithCorrelation({ error: idErr }, { status: 400 }, correlationId); + } + + let body: Record | null; + try { + body = (await request.json()) as Record; + } catch { + return jsonWithCorrelation( + { error: "Invalid JSON in request body" }, + { status: 400 }, + correlationId, + ); + } + if (!body || typeof body !== "object") { + return jsonWithCorrelation({ error: "Invalid request body" }, { status: 400 }, correlationId); + } + + // Only one mutable field for now — keep validation explicit so adding + // future fields here is a deliberate edit. + if (!Object.prototype.hasOwnProperty.call(body, "displayName")) { + return jsonWithCorrelation( + { error: "displayName is required" }, + { status: 400 }, + correlationId, + ); + } + const raw = body["displayName"]; + if (raw !== null && typeof raw !== "string") { + return jsonWithCorrelation( + { error: "displayName must be a string or null" }, + { status: 400 }, + correlationId, + ); + } + + // Empty / null / whitespace-only ⇒ clear the field (revert to default). + // Otherwise sanitize: strip control chars, collapse whitespace, trim, cap length. + const cleaned = + raw === null + ? "" + : stripControlChars(raw).replace(/\s+/g, " ").trim().slice(0, DISPLAY_NAME_MAX_LENGTH); + + try { + const { config, sessionManager } = await getServices(); + const coreSession = await sessionManager.get(id); + if (!coreSession) { + return jsonWithCorrelation({ error: "Session not found" }, { status: 404 }, correlationId); + } + + const sessionsDir = getProjectSessionsDir(coreSession.projectId); + // Empty string in updateMetadata removes the key — exactly the "revert to + // default" semantic. The user-set flag tracks provenance: we set it when + // the user types a name, clear it when they revert, so the dashboard's + // fallback chain knows whether to promote `displayName` over PR/issue + // titles or treat it as an auto-derived spawn artifact. + updateMetadata(sessionsDir, id, { + displayName: cleaned, + displayNameUserSet: cleaned === "" ? "" : "true", + }); + + if (isOpenCodeSessionManager(sessionManager)) { + sessionManager.invalidateCache(); + } + + const updated = await sessionManager.get(id); + const dashboardSession = updated + ? sessionToDashboard(updated) + : sessionToDashboard(coreSession); + + recordApiObservation({ + config, + method: "PATCH", + path: "/api/sessions/[id]", + correlationId, + startedAt, + outcome: "success", + statusCode: 200, + projectId: coreSession.projectId, + sessionId: id, + }); + + return jsonWithCorrelation(dashboardSession, { status: 200 }, correlationId); + } catch (error) { + const { config } = await getServices().catch(() => ({ config: undefined })); + if (config) { + recordApiObservation({ + config, + method: "PATCH", + path: "/api/sessions/[id]", + correlationId, + startedAt, + outcome: "failure", + statusCode: 500, + sessionId: id, + reason: error instanceof Error ? error.message : "Internal server error", + }); + } + return jsonWithCorrelation({ error: "Internal server error" }, { status: 500 }, correlationId); + } +} diff --git a/packages/web/src/app/globals.css b/packages/web/src/app/globals.css index 0d07719cf..38a300e88 100644 --- a/packages/web/src/app/globals.css +++ b/packages/web/src/app/globals.css @@ -4536,6 +4536,53 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { text-transform: lowercase; } +/* Rename pencil button — sits at the rightmost edge of a session row. + Hidden by default; revealed on row hover (via Tailwind `group-hover`) + and on focus so keyboard users can reach it. */ +.project-sidebar__sess-rename-btn { + flex-shrink: 0; + display: inline-flex; + align-items: center; + justify-content: center; + width: 18px; + height: 18px; + padding: 0; + margin-left: 4px; + border: 0; + background: transparent; + cursor: pointer; + color: var(--color-text-muted); + border-radius: 0; + transition: + opacity 80ms, + color 80ms, + background 80ms; +} + +.project-sidebar__sess-rename-btn:hover { + color: var(--color-text-primary); + background: var(--sidebar-row-hover); +} + +/* Inline rename input — replaces the label while editing. Inherits the + row's font sizing so the input visually swaps in without layout shift. */ +.project-sidebar__sess-rename-input { + flex: 1; + min-width: 0; + font-size: 11.5px; + font-family: inherit; + color: var(--color-text-primary); + background: var(--color-bg-primary); + border: 1px solid var(--color-border-strong); + border-radius: 0; + padding: 1px 6px; + outline: none; +} + +.project-sidebar__sess-rename-input:focus { + border-color: var(--color-accent-amber); +} + /* Sidebar settings popover */ .project-sidebar__settings-wrap { position: relative; @@ -7644,11 +7691,34 @@ html.light .xterm .xterm-viewport:hover::-webkit-scrollbar-thumb:active { white-space: nowrap; color: var(--color-text-secondary); } -.topbar-zone-pill__value { font-weight: 600; font-variant-numeric: tabular-nums; } -.topbar-zone-pill__label { opacity: 0.85; } -.topbar-zone-pill--merge { background: color-mix(in srgb, var(--color-status-ready) 12%, transparent); color: var(--color-status-ready); } -.topbar-zone-pill--respond { background: color-mix(in srgb, var(--color-status-error) 12%, transparent); color: var(--color-status-error); } -.topbar-zone-pill--review { background: color-mix(in srgb, var(--color-accent-orange) 12%, transparent); color: var(--color-accent-orange); } -.topbar-zone-pill--working { background: color-mix(in srgb, var(--color-accent-blue) 12%, transparent); color: var(--color-accent-blue); } -.topbar-zone-pill--pending { background: color-mix(in srgb, var(--color-status-attention) 12%, transparent); color: var(--color-status-attention); } -.topbar-zone-pill--done { background: color-mix(in srgb, var(--color-text-tertiary) 14%, transparent); color: var(--color-text-tertiary); } +.topbar-zone-pill__value { + font-weight: 600; + font-variant-numeric: tabular-nums; +} +.topbar-zone-pill__label { + opacity: 0.85; +} +.topbar-zone-pill--merge { + background: color-mix(in srgb, var(--color-status-ready) 12%, transparent); + color: var(--color-status-ready); +} +.topbar-zone-pill--respond { + background: color-mix(in srgb, var(--color-status-error) 12%, transparent); + color: var(--color-status-error); +} +.topbar-zone-pill--review { + background: color-mix(in srgb, var(--color-accent-orange) 12%, transparent); + color: var(--color-accent-orange); +} +.topbar-zone-pill--working { + background: color-mix(in srgb, var(--color-accent-blue) 12%, transparent); + color: var(--color-accent-blue); +} +.topbar-zone-pill--pending { + background: color-mix(in srgb, var(--color-status-attention) 12%, transparent); + color: var(--color-status-attention); +} +.topbar-zone-pill--done { + background: color-mix(in srgb, var(--color-text-tertiary) 14%, transparent); + color: var(--color-text-tertiary); +} diff --git a/packages/web/src/components/ProjectSidebar.tsx b/packages/web/src/components/ProjectSidebar.tsx index 85c66bd89..cc8bb51f2 100644 --- a/packages/web/src/components/ProjectSidebar.tsx +++ b/packages/web/src/components/ProjectSidebar.tsx @@ -170,6 +170,12 @@ function ProjectSidebarInner({ const [showKilled, setShowKilled] = useState(false); const [showDone, setShowDone] = useState(false); const [showSessionId, setShowSessionId] = useState(loadShowSessionId); + // Inline session-rename state. Only one row is edited at a time. `pendingRenames` + // mirrors the in-flight / just-saved value so the new label appears immediately + // without waiting for the next SSE refresh. + const [editingSessionId, setEditingSessionId] = useState(null); + const [editingValue, setEditingValue] = useState(""); + const [pendingRenames, setPendingRenames] = useState>(new Map()); const [settingsOpen, setSettingsOpen] = useState(false); const [projectMenuOpenId, setProjectMenuOpenId] = useState(null); const [projectSettingsProjectId, setProjectSettingsProjectId] = useState(null); @@ -291,6 +297,76 @@ function ProjectSidebarInner({ return map; }, [sessions, prefixByProject, allPrefixes, visibleProjects, showKilled, showDone]); + // Clear an optimistic rename once the prop session.displayName catches up. + // Without this, we'd keep masking the server value forever after a save. + useEffect(() => { + if (pendingRenames.size === 0 || !sessions) return; + const next = new Map(pendingRenames); + let changed = false; + for (const session of sessions) { + const pending = next.get(session.id); + if (pending !== undefined && (session.displayName ?? "") === pending) { + next.delete(session.id); + changed = true; + } + } + if (changed) setPendingRenames(next); + }, [sessions, pendingRenames]); + + const startRename = (session: DashboardSession, currentTitle: string) => { + // Prefer the in-flight optimistic value over the prop — if the user opens + // rename while a previous PATCH is still propagating, the prop still shows + // the pre-rename value but we want the input to start from the latest. + // Auto-derived displayName isn't pre-filled (user-set flag absent) — start + // from the live title so the user types over the visible label. + const pending = pendingRenames.get(session.id); + const initial = + pending ?? (session.displayNameUserSet ? (session.displayName ?? "") : ""); + setEditingSessionId(session.id); + setEditingValue(initial || currentTitle); + }; + + const cancelRename = () => { + setEditingSessionId(null); + setEditingValue(""); + }; + + const submitRename = async (sessionId: string) => { + // Guard against double-submit. submitRename is wired to both Enter (which + // unmounts the input) and onBlur (which can fire during that unmount in + // some browsers); without this, both paths would fire a PATCH. + if (editingSessionId !== sessionId) return; + // Trim, but allow empty — empty means "revert to default" on the server. + const next = editingValue.trim(); + setEditingSessionId(null); + setEditingValue(""); + setPendingRenames((prev) => { + const map = new Map(prev); + map.set(sessionId, next); + return map; + }); + try { + const res = await fetch(`/api/sessions/${encodeURIComponent(sessionId)}`, { + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ displayName: next === "" ? null : next }), + }); + if (!res.ok) { + const body = (await res.json().catch(() => null)) as { error?: string } | null; + throw new Error(body?.error ?? "Failed to rename session"); + } + } catch { + // Roll back the optimistic update so the row reverts to the prop value. + // The user sees the original name return — no further notification is + // needed for this niche failure path. + setPendingRenames((prev) => { + const map = new Map(prev); + map.delete(sessionId); + return map; + }); + } + }; + const navigate = (url: string, session?: DashboardSession) => { if (session) { try { @@ -707,49 +783,130 @@ function ProjectSidebarInner({ visibleSessions.map((session) => { const level = getAttentionLevel(session); const isSessionActive = activeSessionId === session.id; - const title = session.branch ?? getSessionTitle(session); + // Display precedence: optimistic rename (just-saved value) + // → user-set displayName → branch → fallback chain. + // Auto-derived displayName (displayNameUserSet=false) is + // intentionally skipped here so PR/issue titles surfaced + // by getSessionTitle aren't shadowed — mirrors the gate in + // format.ts:getSessionTitle. + const pending = pendingRenames.get(session.id); + const effectiveDisplayName = + pending !== undefined + ? pending + : session.displayNameUserSet + ? (session.displayName ?? "") + : ""; + const title = + effectiveDisplayName !== "" + ? effectiveDisplayName + : (session.branch ?? getSessionTitle(session)); const sessionHref = projectSessionPath(project.id, session.id); + const isEditing = editingSessionId === session.id; + if (isEditing) { + return ( +
+ + setEditingValue(e.target.value)} + onKeyDown={(e) => { + if (e.key === "Enter") { + e.preventDefault(); + void submitRename(session.id); + } else if (e.key === "Escape") { + e.preventDefault(); + cancelRename(); + } + }} + onFocus={(e) => e.currentTarget.select()} + onBlur={() => void submitRename(session.id)} + maxLength={80} + aria-label={`Rename ${session.id}`} + className="project-sidebar__sess-rename-input" + /> +
+ ); + } return ( - { - if (e.metaKey || e.ctrlKey || e.shiftKey || e.button === 1) return; - e.preventDefault(); - navigate(sessionHref, session); - }} className={cn( - "project-sidebar__sess-row", + "project-sidebar__sess-row group", isSessionActive && "project-sidebar__sess-row--active", )} - aria-current={isSessionActive ? "page" : undefined} - aria-label={`Open ${title}`} > - - - {!showSessionId ? ( - - {LEVEL_LABELS[level]} - - ) : null} - + + + ); }) ) : error ? ( diff --git a/packages/web/src/components/__tests__/ProjectSidebar.test.tsx b/packages/web/src/components/__tests__/ProjectSidebar.test.tsx index 4c5f69c20..13536ee04 100644 --- a/packages/web/src/components/__tests__/ProjectSidebar.test.tsx +++ b/packages/web/src/components/__tests__/ProjectSidebar.test.tsx @@ -80,7 +80,9 @@ describe("ProjectSidebar", () => { />, ); - expect(screen.getByRole("button", { name: /switch to (dark|light) mode/i })).toBeInTheDocument(); + expect( + screen.getByRole("button", { name: /switch to (dark|light) mode/i }), + ).toBeInTheDocument(); }); it("renders a collapsed empty rail when collapsed with no projects", () => { @@ -477,9 +479,7 @@ describe("ProjectSidebar", () => { fireEvent.click(screen.getByRole("button", { name: /Project actions for Project Two/i })); - expect( - await screen.findByRole("menuitem", { name: "Open orchestrator" }), - ).toBeInTheDocument(); + expect(await screen.findByRole("menuitem", { name: "Open orchestrator" })).toBeInTheDocument(); }); it("omits 'Open orchestrator' from the menu when no orchestrator entry exists for the project", async () => { @@ -548,4 +548,178 @@ describe("ProjectSidebar", () => { expect(screen.getByLabelText("Loading sessions")).toBeInTheDocument(); expect(screen.queryByText("No sessions shown")).not.toBeInTheDocument(); }); + + // ── Rename worker sessions ─────────────────────────────────────────── + + describe("session rename", () => { + function renderWithSession(displayName: string | null = null) { + // When a displayName is supplied, treat it as a user-set rename so the + // sidebar renders it as the row label (auto-derived names are gated). + return render( + , + ); + } + + it("renders a pencil button for each worker session row", () => { + renderWithSession(); + expect(screen.getByRole("button", { name: /rename worker-1/i })).toBeInTheDocument(); + }); + + it("opens an inline input prefilled with the displayed title on pencil click", () => { + renderWithSession(); + fireEvent.click(screen.getByRole("button", { name: /rename worker-1/i })); + const input = screen.getByRole("textbox", { name: /rename worker-1/i }) as HTMLInputElement; + expect(input.value).toBe("Review API changes"); + }); + + it("PATCHes the new name and shows it immediately on Enter", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ id: "worker-1", displayName: "PR 1466 review" }), + }); + vi.stubGlobal("fetch", fetchMock); + renderWithSession(); + + fireEvent.click(screen.getByRole("button", { name: /rename worker-1/i })); + const input = screen.getByRole("textbox", { name: /rename worker-1/i }); + fireEvent.change(input, { target: { value: "PR 1466 review" } }); + fireEvent.keyDown(input, { key: "Enter" }); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + "/api/sessions/worker-1", + expect.objectContaining({ + method: "PATCH", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ displayName: "PR 1466 review" }), + }), + ); + }); + + // Optimistic update: the new name appears without waiting for SSE. + expect(screen.getByRole("link", { name: "Open PR 1466 review" })).toBeInTheDocument(); + }); + + it("sends null to clear when the input is empty (revert to default)", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ id: "worker-1", displayName: null }), + }); + vi.stubGlobal("fetch", fetchMock); + renderWithSession("Existing rename"); + + fireEvent.click(screen.getByRole("button", { name: /rename worker-1/i })); + const input = screen.getByRole("textbox", { name: /rename worker-1/i }); + fireEvent.change(input, { target: { value: " " } }); + fireEvent.keyDown(input, { key: "Enter" }); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledWith( + "/api/sessions/worker-1", + expect.objectContaining({ + body: JSON.stringify({ displayName: null }), + }), + ); + }); + }); + + it("cancels on Escape without firing PATCH", () => { + const fetchMock = vi.fn(); + vi.stubGlobal("fetch", fetchMock); + renderWithSession("Existing rename"); + + fireEvent.click(screen.getByRole("button", { name: /rename worker-1/i })); + const input = screen.getByRole("textbox", { name: /rename worker-1/i }); + fireEvent.change(input, { target: { value: "Half-typed change" } }); + fireEvent.keyDown(input, { key: "Escape" }); + + expect(fetchMock).not.toHaveBeenCalled(); + // Input is gone; original name is back. + expect(screen.queryByRole("textbox", { name: /rename worker-1/i })).not.toBeInTheDocument(); + expect(screen.getByRole("link", { name: "Open Existing rename" })).toBeInTheDocument(); + }); + + it("does not fire a duplicate PATCH if Enter and onBlur both trigger", async () => { + // Some browsers fire blur during input unmount after the Enter handler + // already cleared editing state. The submitRename guard should swallow + // the second call. + const fetchMock = vi.fn().mockResolvedValue({ + ok: true, + json: async () => ({ id: "worker-1", displayName: "Renamed" }), + }); + vi.stubGlobal("fetch", fetchMock); + renderWithSession(); + + fireEvent.click(screen.getByRole("button", { name: /rename worker-1/i })); + const input = screen.getByRole("textbox", { name: /rename worker-1/i }); + fireEvent.change(input, { target: { value: "Renamed" } }); + fireEvent.keyDown(input, { key: "Enter" }); + // Simulate the post-unmount blur cascade. + fireEvent.blur(input); + + await waitFor(() => { + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + }); + + it("does not shadow PR title with auto-derived displayName (displayNameUserSet=false)", () => { + // Regression: an auto-derived displayName captured at spawn time must not + // beat a live PR title in the sidebar. Mirrors the gate in getSessionTitle. + render( + , + ); + expect(screen.getByRole("link", { name: "Open feat: live PR title" })).toBeInTheDocument(); + expect(screen.queryByText("Stale spawn-time label")).not.toBeInTheDocument(); + }); + + it("rolls back the optimistic name when the PATCH fails", async () => { + const fetchMock = vi.fn().mockResolvedValue({ + ok: false, + status: 500, + json: async () => ({ error: "Failed to rename session" }), + }); + vi.stubGlobal("fetch", fetchMock); + renderWithSession("Original"); + + fireEvent.click(screen.getByRole("button", { name: /rename worker-1/i })); + const input = screen.getByRole("textbox", { name: /rename worker-1/i }); + fireEvent.change(input, { target: { value: "Optimistic name" } }); + fireEvent.keyDown(input, { key: "Enter" }); + + // Settles back to the prop value once the failed PATCH resolves. + await waitFor(() => { + expect(screen.getByRole("link", { name: "Open Original" })).toBeInTheDocument(); + }); + }); + }); }); diff --git a/packages/web/src/lib/__tests__/format.test.ts b/packages/web/src/lib/__tests__/format.test.ts index cc078e2ed..58b47c829 100644 --- a/packages/web/src/lib/__tests__/format.test.ts +++ b/packages/web/src/lib/__tests__/format.test.ts @@ -29,6 +29,7 @@ function makeSession(overrides?: Partial): DashboardSession { issueTitle: null, userPrompt: null, displayName: null, + displayNameUserSet: false, summary: null, summaryIsFallback: false, createdAt: new Date().toISOString(), @@ -113,7 +114,7 @@ describe("humanizeBranch", () => { // --------------------------------------------------------------------------- describe("getSessionTitle", () => { - it("returns PR title when available (highest priority)", () => { + it("returns PR title when available and no displayName is set", () => { const session = makeSession({ summary: "Agent summary", issueTitle: "Issue title", @@ -195,6 +196,52 @@ describe("getSessionTitle", () => { expect(getSessionTitle(session)).toBe("Infer Project Id"); }); + it("prefers user-set displayName above every other signal (rename wins)", () => { + // When displayNameUserSet is true, the user has explicitly renamed this + // session and their label must beat live PR / issue titles. + const session = makeSession({ + displayName: "PR 1466 review", + displayNameUserSet: true, + issueTitle: "Add user authentication", + branch: "feat/auth", + pr: { + number: 1466, + title: "feat: add auth", + url: "https://github.com/x/y/pull/1466", + state: "open", + } as DashboardSession["pr"], + }); + expect(getSessionTitle(session)).toBe("PR 1466 review"); + }); + + it("does NOT promote auto-derived displayName above PR title", () => { + // Spawn-time auto-derived displayName must not shadow a live PR title — + // a stale spawn label would make sessions hard to identify days later. + const session = makeSession({ + displayName: "Auto-derived at spawn", + displayNameUserSet: false, + issueTitle: "Add user authentication", + branch: "feat/auth", + pr: { + number: 1466, + title: "feat: add auth", + url: "https://github.com/x/y/pull/1466", + state: "open", + } as DashboardSession["pr"], + }); + expect(getSessionTitle(session)).toBe("feat: add auth"); + }); + + it("does NOT promote auto-derived displayName above issue title", () => { + const session = makeSession({ + displayName: "Auto-derived at spawn", + displayNameUserSet: false, + issueTitle: "Add user authentication", + branch: "feat/auth", + }); + expect(getSessionTitle(session)).toBe("Add user authentication"); + }); + it("returns displayName when no PR / issue title / user prompt", () => { const session = makeSession({ id: "ao-5", @@ -218,8 +265,7 @@ describe("getSessionTitle", () => { id: "ao-42", summary: null, issueTitle: null, - userPrompt: - "Add rate limiting to /api/upload\n\nUse a sliding-window counter keyed by IP.", + userPrompt: "Add rate limiting to /api/upload\n\nUse a sliding-window counter keyed by IP.", displayName: "Add rate limiting to /api/upload", branch: "session/ao-42", }); @@ -239,10 +285,13 @@ describe("getSessionTitle", () => { expect(getSessionTitle(session)).toBe("Fix the race condition"); }); - it("prefers issue title over displayName when both are present", () => { + it("prefers issue title over auto-derived displayName when both are present", () => { + // Auto-derived displayName must stay below live tracker titles so a stale + // spawn-time value doesn't shadow the current issue title. const session = makeSession({ issueTitle: "Live issue title", displayName: "Stale captured display name", + displayNameUserSet: false, branch: "feat/auth", }); expect(getSessionTitle(session)).toBe("Live issue title"); diff --git a/packages/web/src/lib/format.ts b/packages/web/src/lib/format.ts index 9e829b4be..38e64e824 100644 --- a/packages/web/src/lib/format.ts +++ b/packages/web/src/lib/format.ts @@ -50,41 +50,45 @@ export function humanizeBranch(branch: string, sessionId?: string): string { * Compute the best display title for a session card. * * Fallback chain (ordered by signal quality): - * 1. PR title — human-visible deliverable name - * 2. Issue title — human-written task description (live from tracker) - * 3. Display name — cleaned, single-line, 80-char-truncated task - * context captured at spawn time. Sits above - * `userPrompt` because for prompt-only sessions both - * are populated from the same `spawnConfig.prompt` — - * `displayName` is the cleaned version and should - * win so kanban cards don't show raw multi-line - * prompts. - * 4. User prompt — raw freeform spawn instructions (fallback for - * sessions spawned before `displayName` existed) - * 5. Humanized branch — stable task identifier when no explicit title exists - * (skipped when it collapses to just the session ID) - * 6. Pinned summary — first quality summary, stable across agent updates - * 7. Quality summary — live summary, but can drift as the session evolves - * 8. Any summary — even a fallback excerpt is better than nothing - * 9. Status text — absolute fallback + * 1. User-set display name — only when `displayNameUserSet` is true. + * An explicit rename always wins so the user's + * chosen label isn't shadowed by tracker signals. + * 2. PR title — human-visible deliverable name + * 3. Issue title — human-written task description (live from tracker) + * 4. Auto-derived display — captured at spawn from issue title / user prompt / + * name orchestrator system prompt. Stays BELOW PR/issue + * titles so a stale spawn-time value doesn't + * shadow the live deliverable name. + * 5. User prompt — raw freeform spawn instructions (fallback for + * sessions spawned before `displayName` existed) + * 6. Humanized branch — stable task identifier when no explicit title + * exists (skipped when it collapses to just the + * session ID) + * 7. Pinned summary — first quality summary, stable across agent updates + * 8. Quality summary — live summary, but can drift as the session evolves + * 9. Any summary — even a fallback excerpt is better than nothing + * 10. Status text — absolute fallback */ export function getSessionTitle(session: DashboardSession): string { - // 1. PR title — always best + // 1. User-set rename — wins over everything when explicitly flagged. + if (session.displayName && session.displayNameUserSet) { + return session.displayName; + } + + // 2. PR title if (session.pr?.title) return session.pr.title; - // 2. Issue title — human-written task description + // 3. Issue title — human-written task description if (session.issueTitle) return session.issueTitle; - // 3. Display name — persisted at spawn time from issue title / user prompt / - // orchestrator system prompt. Intentionally ordered ABOVE `userPrompt` - // because for prompt-only sessions both are derived from the same - // `spawnConfig.prompt`: `displayName` is the single-line, 80-char-truncated - // version and `userPrompt` is the raw multi-line original. If `userPrompt` - // were checked first, the raw prompt would always shadow the cleaned one on - // kanban cards and session tabs. + // 4. Auto-derived displayName — captured at spawn time. Sits below PR/issue + // but above userPrompt: for prompt-only sessions both are populated from the + // same `spawnConfig.prompt`; `displayName` is the cleaned single-line, + // 80-char-truncated version and should win so kanban cards don't show raw + // multi-line prompts. if (session.displayName) return session.displayName; - // 4. User prompt — raw freeform spawn instructions. Only reached when + // 5. User prompt — raw freeform spawn instructions. Only reached when // `displayName` is absent (older sessions spawned before this field existed, // or sessions where derivation failed). if (session.userPrompt) return session.userPrompt; diff --git a/packages/web/src/lib/serialize.ts b/packages/web/src/lib/serialize.ts index b2231a7a1..2f8b34b6e 100644 --- a/packages/web/src/lib/serialize.ts +++ b/packages/web/src/lib/serialize.ts @@ -181,6 +181,7 @@ export function sessionToDashboard(session: Session): DashboardSession { issueTitle: null, // Will be enriched by enrichSessionIssueTitle() userPrompt: session.metadata["userPrompt"] ?? null, displayName: session.metadata["displayName"] ?? null, + displayNameUserSet: session.metadata["displayNameUserSet"] === "true", summary, summaryIsFallback: agentSummary ? (session.agentInfo?.summaryIsFallback ?? false) : false, createdAt: session.createdAt.toISOString(), diff --git a/packages/web/src/lib/types.ts b/packages/web/src/lib/types.ts index 86c46b660..27bd197f7 100644 --- a/packages/web/src/lib/types.ts +++ b/packages/web/src/lib/types.ts @@ -106,6 +106,12 @@ export interface DashboardSession { * remain identifiable even when PR/issue enrichment is unavailable. */ displayName: string | null; + /** + * True when `displayName` was explicitly set by the user via the rename UI. + * Auto-derived spawn-time values leave this false. {@link getSessionTitle} + * uses it to decide whether `displayName` should beat live PR/issue titles. + */ + displayNameUserSet: boolean; summary: string | null; /** True when the summary is a low-quality fallback (e.g. truncated spawn prompt) */ summaryIsFallback: boolean;