feat(web): allow renaming worker sessions in the sidebar#1748
Conversation
Closes ComposioHQ#1647 Adds an inline rename UX to each worker session row in the sidebar. A small pencil button appears on row 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 clears the field — reverting the session to its default title. The rename writes to the existing displayName metadata field, which is now the highest-priority signal in getSessionTitle so a user-chosen label always beats PR/issue titles. The session ID (ao-N) remains canonical — only display surfaces are affected. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
Greptile SummaryThis PR adds an inline rename UX to worker session rows in the sidebar, with a pencil button on hover that opens an editable input. Renames persist via a new
Confidence Score: 4/5Safe to merge with awareness that the collapsed sidebar won't show optimistic rename updates, and the Enter-blur double-submit path from the prior review remains unresolved. The core rename feature is well-implemented; the main concerns are the collapsed sidebar not reflecting in-flight optimistic renames, and the unresolved blur-after-Enter double-submit path from the prior review. packages/web/src/components/ProjectSidebar.tsx — both the collapsed-sidebar optimistic-update gap and the blur/Enter interaction live here.
|
| Filename | Overview |
|---|---|
| packages/web/src/components/ProjectSidebar.tsx | Adds inline rename UX with optimistic pendingRenames state; expanded sidebar correctly gates on displayNameUserSet, but collapsed sidebar ignores pending renames; blur-after-Enter double-submit guard relies on stale closure (flagged in prior review). |
| packages/web/src/app/api/sessions/[id]/route.ts | New PATCH handler with thorough input validation (control-char strip, whitespace collapse, 80-char cap, identifier validation); correctly uses empty-string convention to remove keys from metadata and clear the displayNameUserSet flag on revert. |
| packages/web/src/lib/format.ts | User-set displayName correctly promoted to top of fallback chain; minor comment numbering error (two "5" labels in the chain). |
| packages/core/src/metadata.ts | Adds displayNameUserSet parsing in readMetadata with consistent on/off/true/false handling that mirrors the prAutoDetect pattern; also included in unflattenFromStringRecord's booleanFields set for correct round-trip storage. |
| packages/web/src/lib/serialize.ts | Correctly serializes displayNameUserSet from session metadata string to boolean in sessionToDashboard; no issues. |
| packages/core/src/types.ts | Adds optional displayNameUserSet?: boolean to SessionMetadata with clear doc-comment explaining the flag's semantics; clean extension with no breaking changes. |
| packages/web/src/lib/types.ts | Adds non-optional displayNameUserSet: boolean to DashboardSession with accurate doc-comment; defaults to false in serialization which is correct for sessions that have never been explicitly renamed. |
Sequence Diagram
sequenceDiagram
participant User
participant Sidebar as ProjectSidebar
participant API as PATCH /api/sessions/:id
participant Meta as metadata.ts (disk)
participant SM as SessionManager
participant SSE as SSE refresh
User->>Sidebar: Click pencil button
Sidebar->>Sidebar: startRename() → setEditingSessionId + setEditingValue
User->>Sidebar: Type new name, press Enter (or blur)
Sidebar->>Sidebar: submitRename() — guard editingSessionId check
Sidebar->>Sidebar: "setEditingSessionId(null), setPendingRenames({id: newName})"
Sidebar->>API: "PATCH /api/sessions/:id {displayName}"
API->>API: validateIdentifier(id)
API->>API: stripControlChars + trim + slice(0,80)
API->>SM: sessionManager.get(id)
API->>Meta: updateMetadata(displayName, displayNameUserSet)
API->>SM: invalidateCache() [OpenCode only]
API->>SM: sessionManager.get(id) [read updated]
API-->>Sidebar: 200 OK / error
alt PATCH failed
Sidebar->>Sidebar: setPendingRenames → delete(id) [rollback]
end
SSE-->>Sidebar: sessions refresh with new displayName
Sidebar->>Sidebar: cleanup effect: pendingRenames.delete(id) when displayName matches
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
packages/web/src/lib/format.ts:96-97
The fallback chain comment numbering skips from step 5 to another step 5 — the humanized-branch block should be labeled 6, matching the doc-comment at the top of the function. As-is it conflicts with the "5. User prompt" block above it, making the chain hard to read and audit.
```suggestion
// 6. Humanized branch — stable semantic fallback.
// humanizeBranch returns "" when the branch is just the session ID
```
### Issue 2 of 2
packages/web/src/components/ProjectSidebar.tsx:460-467
**Collapsed sidebar ignores `pendingRenames` optimistic updates**
The collapsed sidebar computes `rawTitle` directly from `session.branch ?? getSessionTitle(session)` without consulting `pendingRenames`. If a user renames a session and immediately collapses the sidebar (or the sidebar starts collapsed), the three-letter abbreviation and tooltip still reflect the pre-rename label until the next SSE refresh. The expanded sidebar correctly applies the `pendingRenames` mask; the collapsed view should use the same `effectiveDisplayName` logic so the two views stay in sync during an in-flight rename.
Reviews (3): Last reviewed commit: "fix(web): address review feedback on ses..." | Re-trigger Greptile
PR review flagged that promoting `displayName` to the top of `getSessionTitle` regressed every existing session: spawn-time auto-derived `displayName` would shadow live PR/issue titles for sessions the user never explicitly renamed. Adds a `displayNameUserSet` boolean flag to SessionMetadata and DashboardSession. The dashboard fallback chain promotes `displayName` above PR/issue titles only when this flag is true; auto-derived spawn-time values stay at their original position (below PR/issue, above userPrompt). PATCH /api/sessions/:id sets `displayNameUserSet=true` when the user types a name, and clears it when they revert. Sidebar gates its displayName preference on the flag too, so non-renamed rows keep the existing branch-first behavior. Also addresses review #3 (rename-while-pending pre-fill) and #4 (double-submit guard on Enter+blur). Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Pushed eee8d56 addressing the review: 🔴 Critical #1 —
💡 CSS reformatting noise: ✅ fixed. Reverted the Test counts: 1109 core tests pass, 28 sidebar, 34 format, 8 PATCH. The 2 pre-existing api-routes failures (cache-first PR enrichment, killed-session PR refresh) are present on |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Pull request overview
Adds a user-facing “rename session” capability to the web dashboard by introducing an inline rename control in the ProjectSidebar and a new PATCH /api/sessions/:id API route that persists the chosen label into session metadata (with sanitization and length limits).
Changes:
- Add inline rename UX in the sidebar with optimistic UI updates.
- Add
PATCH /api/sessions/[id]to persistdisplayName+ provenance flag (displayNameUserSet) into metadata. - Update session title selection logic and tests to prioritize user-set display names over PR/issue titles.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/web/src/lib/types.ts | Adds displayNameUserSet to the dashboard session model. |
| packages/web/src/lib/serialize.ts | Serializes displayNameUserSet from session metadata into DashboardSession. |
| packages/web/src/lib/format.ts | Updates getSessionTitle fallback chain to prefer user-set display names. |
| packages/web/src/lib/tests/format.test.ts | Adds/updates tests for the new title precedence behavior. |
| packages/web/src/components/ProjectSidebar.tsx | Implements inline rename UI, optimistic updates, and PATCH calls from the sidebar. |
| packages/web/src/components/tests/ProjectSidebar.test.tsx | Adds rename interaction tests (Enter/Escape/blur rollback). |
| packages/web/src/app/globals.css | Adds styling for the rename button and inline input; reformats unrelated CSS block. |
| packages/web/src/app/api/sessions/[id]/route.ts | Introduces PATCH handler to update session metadata for display name renames. |
| packages/web/src/tests/helpers.ts | Updates test session factory to include displayNameUserSet. |
| packages/web/src/tests/api-routes.test.ts | Adds tests and core mocks for the new PATCH route behavior. |
| packages/core/src/types.ts | Extends SessionMetadata with displayNameUserSet. |
| packages/core/src/metadata.ts | Reads/writes/parses displayNameUserSet in metadata serialization. |
| packages/core/src/tests/metadata.test.ts | Adds coverage for displayNameUserSet persistence semantics. |
| .changeset/rename-worker-sessions.md | Publishes the web package change as a minor release with user-facing notes. |
- ProjectSidebar: gate effective displayName on displayNameUserSet so auto-derived spawn-time names no longer shadow live PR/issue titles in the sidebar (mirrors the gate already in format.ts:getSessionTitle). Adds a regression test. - ProjectSidebar: drop unreachable `?? currentTitle` from startRename initial value — the right side of the nullish-coalescing always returns a string, so the fallback is already handled by the `|| currentTitle` on the next line. - ProjectSidebar: reveal rename pencil on `group-focus-within` so keyboard users tabbing through the session links discover the affordance, not just pointer users. - globals.css: change rename button + input border-radius from 3px to 0 to match the repo's --radius-base: 0 design rule for UI controls. - core/metadata: accept legacy "on"/"off" strings for displayNameUserSet in readMetadata for parity with prAutoDetect (defensive — the storage write path already converts to boolean via unflattenFromStringRecord). Adds coverage for all six accepted forms. - web/serialize: drop dead `=== "on"` check on displayNameUserSet — Session.metadata is Record<string, string> and the value can only ever be "true" / "false" after flattenToStringRecord. Refs ComposioHQ#1647. Co-Authored-By: Claude Opus 4.7 (1M context) <[email protected]>
|
Pushed 1131c33 addressing the latest round of review feedback. Greptile
Copilot
Verification
|
i-trytoohard
left a comment
i-trytoohard
left a comment
There was a problem hiding this comment.
Re-Review — All Previous Criticals Fixed
Both criticals from the initial review are resolved:
-
updateMetadatais synchronous (returnsvoid, usesatomicWriteFileSync) — no missing await. Handler order is correct: write → invalidate cache → re-read. -
displayNameUserSetflag cleanly separates user renames from auto-derived names. Two-tier precedence: user-set name wins over everything; auto-derived stays below PR/issue titles. Atomic writes via file lock +atomicWriteFileSync. Boolean parsing covers all 6 forms.
Copilot/Greptile feedback also addressed: group-focus-within for keyboard accessibility, border-radius: 0 matching design spec, on/off parity with prAutoDetect.
CI: 12/12 green.
Approved.
Summary
PATCH /api/sessions/:idendpoint that writes the existingdisplayNamemetadata field (sanitized, max 80 chars, control-char-stripped).displayNameto the top ofgetSessionTitle's fallback chain so a user-chosen rename beats PR/issue titles. The session ID (ao-N) stays canonical — only display surfaces are affected.Closes #1647.
Test plan
pnpm --filter @aoagents/ao-web typecheck— cleanpnpm lint— 0 errors (49 pre-existing warnings)pnpm --filter @aoagents/ao-web test -- ProjectSidebar— 27 pass (incl. 6 new rename tests)pnpm --filter @aoagents/ao-web test -- api-routes— 73 pass (incl. 8 new PATCH tests; 2 pre-existing failures verified onmainvia stash round-trip)pnpm --filter @aoagents/ao-web test -- format— 33 pass (incl. updated precedence tests)🤖 Generated with Claude Code