Show agent-provided session names in the sidebar

[gordonwoodhull]

I use the /rename feature pretty consistently in Claude Code, and wanted those names to show up in the left pane of Agentsview.

It looks like there may have been previous work in this area but it was never complete.

Summary

Adds an opt-in "Use session names" Appearance toggle (default off) that shows agent-provided session names in the sidebar: Claude Code /rename names plus the native conversation titles seven other parsers already extract (ChatGPT, Claude.ai, Forge, Hermes, Kiro, Piebald, Cortex).

A new name_source column (user | agent) records provenance. A manual in-app rename is marked user and always shows, and is never overwritten by an agent name; agent names are marked agent, update live (including a full-reparse fallback when a /rename is appended mid-session), and are shown only when the toggle is on. Parser-extracted names are now carried through toDBSession (previously dropped on the live disk-sync path — only the importer persisted them), with matching PostgreSQL push/read parity, a dataVersion bump so existing sessions re-parse to backfill the column, and resync metadata-copy logic that preserves user-owned names while letting agent names refresh.

Because the names live in the skinny sidebar index, agent-named rows render immediately on load without per-row hydration.

Other agents/chat apps with a session name

The feature, if enabled, should also work for these coding agents. (Untested.)

• Cortex Code
• Forge
• Hermes Agent
• Kiro IDE
• Piebald

And these imported chats (2):

• ChatGPT (conversation title)
• Claude.ai (conversation name)

[roborev-ci]

roborev: Combined Review (70ff277)

Medium findings remain; no Critical or High issues were reported.

Medium

• frontend/src/lib/components/sidebar/SessionList.svelte:203
  Agent display_name rows are treated as already hydrated, but the default toggle hides those names. Sidebar index rows have first_message: null, so default-off agent-named sessions render the project fallback instead of the first-message preview.
  Fix: Treat index-only rows with name_source === "agent" and hidden session names as needing visible hydration, or include the fallback preview in the sidebar index.

• internal/db/sessions.go:78
  sessionFullCols still omits name_source, so GetSessionFull and ListSessionsModifiedBetween return sessions with NameSource == nil. The normal PostgreSQL push path reads sessions through ListSessionsModifiedBetween, so pushed rows lose name_source despite the PG insert/update changes.
  Fix: Add name_source to sessionFullCols and update all matching scan calls, including tests that cover the real push path rather than only direct pushSession.

• internal/postgres/push.go:642
  sessionPushFingerprint does not include NameSource. A session whose only metadata change is name_source can match the previous fingerprint and be skipped, leaving PostgreSQL stale.
  Fix: Add stringValue(sess.NameSource) to the fingerprint fields and cover it in the fingerprint tests.

---

Panel: ci_default_security | Synthesis: codex, 14s | Members: codex_default (codex/default, done, 5m6s), codex_security (codex/security, done, 2m24s) | Total: 7m44s

[gordonwoodhull]

Thanks for the review — addressed all three Medium findings, each in its own commit with a test that fails on the unpatched code:

1. Sidebar hydration vs. the toggle (SessionList.svelte) — needsVisibleHydration treated any display_name row as hydrated, so default-off agent-named rows fell through to the project-name fallback instead of the first-message preview. It now hydrates a row unless its name will actually render. (b3e5b8a)

2. name_source dropped on the real push path (sessions.go) — sessionFullCols omitted name_source, so ListSessionsModifiedBetween (and GetSessionFull) returned NameSource == nil and the PG push stored NULL despite the insert/update changes. Added name_source to sessionFullCols and both scanners, plus a test that exercises the real Push path rather than only direct pushSession. (6c51e6c)

3. Push fingerprint omitted name_source (push.go) — a session whose only change was name_source matched the previous fingerprint and was skipped, leaving PG stale. Added stringValue(sess.NameSource) to the fingerprint fields and covered it in the fingerprint test. (8717d6b)

---

Note: the failing frontend check job is unrelated to this PR. On a clean main (no changes from this branch), npm run check already fails with Unused '@ts-expect-error' directive in src/lib/virtual/createVirtualizer.svelte.ts:100 — it appears to be a pre-existing interaction between #589 (svelte-check 4.6) and #593 (dependency bump) that surfaces only once both are on main. I didn't want to fix this without understanding it.

[roborev-ci]

roborev: Combined Review (8717d6b)

Medium issue found: PostgreSQL schema compatibility can miss a required column and fail later at runtime.

Medium

• internal/postgres/schema.go:1290 - CheckSchemaCompat does not probe sessions.name_source, while PostgreSQL session/sidebar read paths now select it. With a legacy schema and a read-only or non-migrating role, compatibility can pass but pg serve can later fail on session queries.
  • Fix: Add name_source to the sessions compatibility probe and cover it with a regression test similar to the existing secrets_rules_version check.

---

Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 7m38s), codex_security (codex/security, done, 1m32s) | Total: 9m18s

[gordonwoodhull]

Addressed the follow-up finding (8717d6b review):

• CheckSchemaCompat didn't probe sessions.name_source (internal/postgres/schema.go). The PG session/sidebar read paths now select name_source, so a legacy schema under a read-only/non-migrating role could pass the compat check and then fail at runtime on the first session query. Added name_source to the probe, with a regression test mirroring the existing secrets_rules_version check (verified test-first: it fails when the column is dropped, passes after the probe change). (63fbdea)

I also swept the rest of the codebase for the same class of omission (every display_name/read-path that should parallel name_source). The parser/agent-label/rename-handler references are correct as-is; the only other place worth a mention is the import/export format, which doesn't carry name_source (a re-import leaves an agent-named row ungated but still shown).

I didn't think it belonged in the export format, but happy to add if needed.

[roborev-ci]

roborev: Combined Review (63fbdea)

Summary verdict: One medium issue found; no high or critical findings.

Medium

• frontend/src/lib/components/layout/SessionBreadcrumb.svelte:427: The new showSessionNames preference only gates sidebar labels; the active session breadcrumb still renders session.display_name unconditionally. Agent-sourced names hidden with the default Off setting can reappear when the session is opened.
  • Fix: Use a shared display-name helper that suppresses name_source === "agent" when ui.showSessionNames is false, and apply it to breadcrumb, modal, trash, and pinned display paths.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 6m31s), codex_security (codex/security, done, 17s) | Total: 6m55s

[gordonwoodhull]

Addressed the breadcrumb/consistency finding (42126be):

• Extracted a shared visibleSessionName(session) helper (frontend/src/lib/utils/sessionName.ts) that returns the name unless it's name_source === "agent" and the "use session names" toggle is off. Manual renames ("user") and legacy/imported (null) names always show.
• Routed every session-name display path through it: the sidebar (SessionItem/SessionList, refactored from the old inline check), the active-session breadcrumb, the delete-confirmation modal, the trash list, the pinned page's store-backed path, and the rename input prefill (so double-click-to-rename prefills the preview, not a hidden agent name). Each keeps its existing fallback.
• The agent-visibility decision is isolated in a single predicate so a future "always-on" migration (dropping the toggle) is a one-line change.
• Added a unit test matrix for the helper; all 1214 frontend tests pass.

One remaining ungated path — question for @roborev-ci: the Pinned page's common code path renders pin.session_display_name straight from the pinned-messages API (PinnedMessage), which carries session_display_name but not session_name_source — so it can't be gated client-side. Closing it would mean threading session_name_source through the pins API (internal/db/pins.go, internal/postgres/curation.go, the API response, and the PinnedMessage TS type), parallel to the existing session_display_name.

@roborev-ci — should we thread session_name_source through the pins API to gate that last path, or is gating the pinned list (vs. the sidebar) out of proportion to the benefit? Want to confirm the direction before adding cross-layer plumbing.

[roborev-ci]

roborev: Combined Review (42126be)

Summary verdict: Two medium issues remain; no critical or high findings were reported.

Medium

• internal/server/upload.go:233
  Uploaded Claude sessions drop ParsedSession.DisplayName, so /rename names parsed by the new Claude parser are not persisted through the upload path.
  Fix: Mirror toDBSession: set DisplayName and NameSource: "agent" when sess.DisplayName is non-empty, and add an upload-path regression test.

• frontend/src/lib/components/command-palette/CommandPalette.svelte:199
  Search results still display backend-coalesced result.name, which can be an agent-provided display_name; the search API does not return name_source, so the new “Use session names” gate is bypassed in the command palette.
  Fix: Include enough session-name metadata in SQLite and PostgreSQL search results to gate with visibleSessionName, falling back to first-message/project when agent names are hidden.

---

Panel: ci_default_security | Synthesis: codex, 8s | Members: codex_default (codex/default, done, 7m27s), codex_security (codex/security, done, 1m26s) | Total: 9m1s

[gordonwoodhull]

Hmm, this seems to be sprawling. I'll convert to draft and see if there is any interest from human maintainers, and what the scope should be.

Just changing the titles in the sidebar was easy, but it would probably be better to use consistent names everywhere.

An alternative design would be having session_name as a separate field from display_name, instead of name_source. Either way, it's an extra field which tends to propagate.

[mariusvniekerk]

I'm not entirely sure if we want to have the complexity of the feature flagging? Presumably if a user went to the effor of naming their sessions using /rename etc there is no reason to not show those.

I would hazard a guess that removing the use-session-names toggle removes the need for a frontend name-source field etc. Ultimately having some coalesce based cascade feels like the correct implementation that users would expect.

[gordonwoodhull]

Hi @mariusvniekerk. Thanks for the review!

Sure, I was being cautious with the feature flag but I agree it’s probably not worth the complexity.

I think an extra field would be required for the behavior that user-entered name is preferred to session name, and goes back to session name if the entered name is removed. It could be a flag (as implemented) or a string.

However, maybe if we relax it to last-one-wins or letting the session name win over the user-entered name, it could be just one field.

I’ll investigate.

Result: still two fields, but db-only; see below.

[mariusvniekerk]

@gordonwoodhull on the db model we should store all the names we can, and just send the correct computed field to the frontend from the golang side, keeps this a concern for the backend only and the frontend remains blissfully unaware

[gordonwoodhull]

Thanks @mariusvniekerk — fully agree. I've reworked the branch to implement the coalesce-based cascade with name_source as a backend-only concern:

What changed

The "Use session names" toggle and all frontend name_source threading are gone. Instead, the Go layer resolves the name before it reaches the frontend:

• name_source stays in SQLite and PG schemas as a write-time guard: the upsert CASE WHEN name_source = 'user' protects user renames from being overwritten on re-sync, and RenameSession stamps name_source = 'user' / clears to NULL.
• name_source is stripped from all JSON responses (json:"-" on both Session and SidebarSessionIndexRow, removed from the sidebar query and PG read path). The frontend never sees it.
• Every display surface just reads display_name, falling back to first_message preview → project name. No helper, no conditional.

Cascade behavior

• Agent /rename or native conversation title → display_name = "name", name_source = "agent" on parse
• User renames in-app → display_name = "name", name_source = "user"; upsert CASE preserves this on all subsequent re-syncs
• User clears their rename → display_name = NULL, name_source = NULL; the server immediately re-parses the session file and restores the agent name in the same HTTP response (no 15-minute wait)

Bugs fixed along the way

• upload.go was silently dropping ParsedSession.DisplayName — fixed
• importer.go had a hand-rolled "preserve existing name" guard that compared names by value instead of checking name_source — replaced with the upsert CASE
• ImportChatGPT wasn't setting name_source — fixed
• All three parse→DB converters now go through a shared ParsedSessionNameFields helper so a fourth can't drift

What's kept

• dataVersion = 34 bump + BackfillNameSource migration (stamps name_source = 'user' on pre-feature non-NULL display_name rows so existing renames survive)
• name_source in PG DDL, CheckSchemaCompat, push fingerprint, and push INSERT … ON CONFLICT — all write paths intact
• All parser extraction (Claude /rename, 7 native-title parsers)

[roborev-ci]

roborev: Combined Review (80485cd)

Summary verdict: The PR needs changes before merge due to medium-severity session name persistence and PostgreSQL read parity issues.

Medium

• Location: internal/postgres/sessions.go:32, internal/postgres/sessions.go:137, internal/postgres/sessions.go:601
  Problem: PostgreSQL read paths still omit name_source, so pushed values do not round-trip through Store.GetSession/sidebar reads and the new PG name-source tests will fail.
  Fix: Add name_source to pgSessionCols and scan it into s.NameSource; also align the PG sidebar SELECT/scan with the intended hidden-field contract.

• Location: internal/db/sessions.go:1879
  Problem: RenameSession overwrites the only stored display_name with the user override and clears it on nil. File-backed sessions can recover by reparsing, but imported sessions now marked name_source=agent have no file path, so clearing a user rename loses the imported agent title until re-import.
  Fix: Store user override and agent/imported title separately, then compute the exposed display_name from user override first and agent title second.

---

Panel: ci_default_security | Synthesis: codex, 12s | Members: codex_default (codex/default, done, 8m54s), codex_security (codex/security, done, 1m2s) | Total: 10m8s

[gordonwoodhull]

(Response written by AI for concision and accuracy; opinions are mine.)

Finding 1 (PG name_source read path): Removing name_source from the PG read path was intentional — it's the mechanism that keeps name_source a backend-only concern and out of the frontend. Adding it back to pgSessionCols/scanPGSession would restore exactly the kind of threading across layers that the simplification was meant to eliminate. The PG write path (push INSERT, fingerprint, CheckSchemaCompat) still carries it correctly. Happy to add a comment to pgSessionCols making the omission explicit, but I'd rather not add the field back unless there's a concrete reason it's needed on the read side.

Finding 2 (imported sessions): This one is real. File-backed sessions (Claude Code, Copilot, etc.) work correctly — clearing a user rename immediately re-parses the session file and restores the agent name. But imported sessions (Claude AI export, ChatGPT export) have no backing file, so clearing a rename falls back to the first-message preview rather than the imported conversation title. The fix would require storing the user override and the agent title in separate fields, which is the more complex two-field design we moved away from. What would you like to do here?

[roborev-ci]

roborev: Combined Review (00b168d)

Synthesis unavailable. Showing individual review outputs.

codex — default (done)

Review Findings

• Severity: Medium
• Location: internal/server/huma_routes_sessions.go:551
• Problem: Clearing a user rename can leave the old name visible in the SPA when no agent-provided name is restored. The endpoint returns db.Session, whose nil display_name is omitted from JSON, and the frontend merge keeps the previous display_name when the response lacks that property.
• Fix: Return an explicit display_name: null from the rename response, or update the frontend rename merge to clear display_name when the request cleared it.

---

• Severity: Low
• Location: frontend/src/lib/components/modals/ConfirmDeleteModal.svelte:15, frontend/src/lib/components/pinned/PinnedPage.svelte:59
• Problem: normalizeMessagePreview() always returns a string, so replacing || with ?? makes the project/default fallback unreachable for empty or missing first messages, producing blank labels.
• Fix: Treat empty previews as absent, e.g. s.display_name ?? (normalizeMessagePreview(s.first_message) || s.project || "this session").

Summary

The change adds backend-tracked session name ownership, parser/import/sync persistence, PostgreSQL push parity, and frontend display-name cleanup.

---

codex — security (done)

Summary: Reviewed the session name/name_source changes across parser, DB/PG, upload/import, rename handling, and frontend rendering. I did not find a plausible security exploit introduced by this diff.

No issues found.

[roborev-ci]

roborev: Combined Review (90d586d)

Summary verdict: Changes need revision due to medium-risk session naming data loss/migration issues.

Medium

• Location: internal/db/sessions.go:1878
  Problem: User renames overwrite the only stored display_name, so clearing a rename cannot restore the previous agent/import title unless the session has a source file that can be re-parsed. Imported sessions have no file_path, so clearing a custom rename permanently falls back to the first message instead of the imported title.
  Fix: Store agent-provided and user-provided names separately, or preserve the last agent name when setting name_source='user', then compute the exposed display name from that cascade.

• Location: internal/db/db.go:783
  Problem: The migration backfills every legacy non-NULL display_name as name_source='user', but legacy imports already stored agent/import titles in display_name. Those rows become pinned as user names, so future imports/resyncs will not update their titles even when the user never renamed them.
  Fix: Backfill only rows with evidence of a manual rename, or use a safer migration heuristic for known imported/agent-titled rows and add coverage for legacy imported sessions.

---

Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 4m59s), codex_security (codex/security, done, 2m3s) | Total: 7m12s

[gordonwoodhull]

Previous round of fixes (90d586d, not yet commented on):

roborev's review of 90d586d found two issues, both fixed in 00b168d:

• Medium — clearing a user rename left the old name visible in the SPA when no agent name was available, because display_name: null is omitted from the JSON response (Go's omitempty). Fixed in sessions.svelte.ts: renameSession now explicitly sets display_name = null in the store when the caller passed null and the response omits the field.

• Low — normalizeMessagePreview() returns "" for null/empty input, making ?? the wrong operator before the project/default fallback in ConfirmDeleteModal and PinnedPage. Fixed to use || (falsy check) instead, matching the original intent.

---

Current round (b099eba):

Finding 1 (imported sessions): Already raised with the human reviewers here — still waiting on direction.

Finding 2 (BackfillNameSource pins imported titles as user-owned): Fixed. The migration comment said "pre-feature databases stored only manual renames in display_name" — but the importers also wrote conversation titles directly into display_name. Restricting the WHERE clause to file_path IS NOT NULL correctly identifies genuine user renames: file-backed sessions always have a path; imported sessions never do. Imported rows are left with name_source = NULL, so future re-imports can update their titles via the normal upsert CASE.

[roborev-ci]

roborev: Combined Review (b099eba)

Synthesis unavailable. Showing individual review outputs.

codex — default (done)

Review Findings

• Severity: Medium
• Location: internal/db/sessions.go:1879
• Problem: RenameSession overwrites the only stored display_name, so imported/uploaded sessions lose their agent/imported title when a user rename is set. The clear path can only recover file-backed sessions via reparse, so sessions with no file_path come back with no title after clearing.
• Fix: Store the agent/imported title separately from the user override, or otherwise retain a restorable agent title for non-file-backed sessions and use it when clearing a rename.

---

• Severity: Medium
• Location: internal/server/huma_routes_sessions.go:545
• Problem: The forced reparse after clearing a rename resets file state only for the requested session ID. Claude fork sessions share a file, but processClaude checks skip state using the file-derived root session ID, so clearing a fork’s rename can still skip the file and fail to restore the agent name.
• Fix: Reset file state for all non-deleted sessions with the same file_path, or add a path-based force-reparse API that bypasses the ID-based skip check.

---

• Severity: Low
• Location: internal/postgres/sessions.go:32
• Problem: PostgreSQL read paths still omit name_source from pgSessionCols and scanPGSession, so the PG db.Session model no longer mirrors SQLite even though the column is created and pushed.
• Fix: Include name_source in pgSessionCols and scan it into db.Session.NameSource; json:"-" will keep it out of API responses.

Summary

The change adds backend-owned session-name tracking and parser/import propagation, but the single-column storage model and partial PG/read-path parity leave several restore and migration gaps.

---

codex — security (done)

Summary: No plausible security vulnerabilities were identified in the diff.

No issues found.

[gordonwoodhull]

Finding 1 (imported sessions): Covered here — still waiting on direction from the human reviewers.

Finding 3 (PG name_source read path): Covered here — omitting it from pgSessionCols is intentional. Including it and relying on json:"-" would work too, but the effect is the same: the field never reaches the frontend. Happy to add either way if there's a preference.

Finding 2 (Claude fork sessions, db69e7c): Fixed. Claude fork sessions share a .jsonl file but have separate rows in the DB. The previous fix called ResetSessionFileState(id) which cleared file_size/file_mtime for the one session being renamed — but shouldSkipFile looks up the skip state using the root session ID derived from the filename, not the fork ID. So the root's stored state remained intact and the re-parse was still skipped. Replaced with ResetFileStateByPath(path) which clears all sessions sharing the file, so shouldSkipFile returns false for root and fork alike.

[roborev-ci]

roborev: Combined Review (db69e7c)

Synthesis unavailable. Showing individual review outputs.

claude-code — default (done)

Based on my review of the diff and verification against the repository, here are my findings.

Review Findings

Severity: Medium
Location: internal/postgres/schema.go:1288-1293 (CheckSchemaCompat), with a stale comment at internal/postgres/sync_test.go:180-181
Problem: CheckSchemaCompat now probes name_source, but no PG read path actually selects it. pgSessionCols/scanPGSession were intentionally not updated (the feature is backend-only — confirmed by the tests asserting found.NameSource is nil and the round-trip tests reading via direct SQL). pg serve skips EnsureSchema when the PG role is read-only (the explicitly supported read-only-role path, cmd/agentsview/pg.go:250-256) and then runs CheckSchemaCompat. Against a schema populated by a pre-name_source pusher, serve will now fatal on startup for a column it never reads — a regression from the prior behavior where it served fine. The test comment claiming "the PG session and sidebar read paths now select name_source" is factually incorrect after the backend-only refactor.
Fix: Remove name_source from the CheckSchemaCompat SELECT (it is a write-only guard, not a read column, per guideline 17's "any column required for reads"), and drop/replace the stale test and comment. If schema uniformity is desired instead, document that read-only serve requires the column.

---

Severity: Low
Location: internal/db/sessions.go:1124-1135 (ResetSessionFileState)
Problem: ResetSessionFileState(id string) is added but never called; the rename-clear path uses ResetFileStateByPath instead (to also re-parse Claude fork sessions sharing the file). It's dead code with no test.
Fix: Remove ResetSessionFileState.

---

Severity: Low
Location: internal/importer/importer.go:159-194 (upsertConversation)
Problem: The explicit "preserve user-renamed display_name on re-import" logic was removed in favor of the name_source='user' upsert guard. Post-feature UI renames of imported (file_path NULL) sessions are correctly preserved via the upsert CASE, but pre-feature renames are not backfilled to name_source='user' (BackfillNameSource excludes file_path IS NULL), so re-importing via ImportClaudeAI (which overwrites, unlike ImportChatGPT which skips existing) will replace such a rename with the imported title. This appears to be an accepted, documented tradeoff (a pre-feature imported-session rename is indistinguishable from an imported title), but it is a behavior change with possible one-time data loss at the migration boundary.
Fix: Confirm this is intended; if so, no code change — otherwise consider treating file-backed exclusion separately from imported titles, or note the tradeoff in release notes.

---

Severity: Low
Location: internal/server/huma_routes_sessions.go:536-551 (humaRenameSession)
Problem: The end-to-end server flow for the headline fix (clearing a user rename triggers a synchronous re-parse so the agent name is restored in the same response) has no integration test. The constituent pieces are tested (ResetFileStateByPath, parser rename extraction, the upsert CASE, the frontend store merge), but the handler behavior tying them together is not.
Fix: Add a server-level test: seed a Claude session whose file contains /rename X, rename via the API, clear the rename, and assert the readback returns the agent name X.

Summary

Adds a name_source column that distinguishes user renames from agent-provided session names, preserving user renames through upserts/resyncs/imports/PG-push while letting agent names update freely; the core SQLite upsert CASE, parser /rename extraction, resync overlay, and PG push parity are correct, but CheckSchemaCompat over-probes a non-read column and there is some dead code and a test gap.

---

claude-code — security (done)

I've reviewed the diff and verified the key potential sinks in the codebase.

Summary

This change set implements session display-name provenance tracking: a new name_source column ('user' vs 'agent') that distinguishes manual renames from agent-parsed names, with parity across SQLite and PostgreSQL, a Claude /rename parser, importer integration, and frontend display/cascade logic. I evaluated it against the project's documented single-user/loopback threat model.

Verification performed:

• XSS sinks — Confirmed display_name/name never reaches an unsanitized HTML sink. Every {@html ...} usage in the components tree feeds escapeHTML(), renderMarkdown() (DOMPurify-sanitized per guideline build(deps-dev): Bump svelte-check from 4.1.5 to 4.4.1 in /frontend #4), or sanitizeSnippet(). The display name is rendered only via Svelte text interpolation ({session?.display_name ?? ...}, {truncate(info.name, 60)}), which auto-escapes. The /rename argument extracted by extractRenameName therefore cannot inject markup even though session files are treated as non-adversarial.
• SQL injection — All new/modified queries (BackfillNameSource, RenameSession, ResetFileStateByPath, ResetSessionFileState, the upsert CASE blocks, PG pushSession INSERT/ON CONFLICT) use static SQL or bound parameters. No string interpolation of user data.
• Re-parse on rename clear (humaRenameSession) — The re-parsed path comes from GetSessionFilePath(in.ID) (a value the sync engine stored from discovered agent directories), not from request input. The rename API only writes display_name; an attacker cannot inject an arbitrary file_path to coerce reading a sensitive file. ResetFileStateByPath is scoped by file_path = ? AND deleted_at IS NULL and intentionally resets co-located Claude fork sessions. No path-traversal or arbitrary-read vector.
• Auth/CORS posture — Unchanged; no new endpoints. The rename mutation is gated by the existing CORS origin check (mutating requests) and bearer token in remote mode.
• Concurrency — New DB methods consistently take db.mu.Lock(), matching the surrounding pattern; no new race introduced.

The placeholder renumbering in internal/postgres/push.go and the upsert CASE ownership logic are data-integrity concerns rather than security ones, and are covered by passing round-trip tests (TestPushNameSourceViaPushPath).

No issues found.

[gordonwoodhull]

Finding 1 (imported sessions): Covered here — still waiting on direction.

Finding 3 (PG name_source read path): Covered here — intentional omission. name_source stays in sessionBaseCols/sessionFullCols so the Go struct is populated for the PG push path, but json:"-" keeps it out of API responses. Including it in pgSessionCols and relying on json:"-" alone would work equally well — happy to do that if preferred.

Finding 2 (dead ResetSessionFileState, stale CheckSchemaCompat probe, 910c576): All three fixed:

• ResetSessionFileState removed — superseded by ResetFileStateByPath
• name_source removed from CheckSchemaCompat — it's a write-only column and pg serve with a read-only role against a pre-name_source schema would have crashed on startup checking for a column it doesn't actually read
• TestGetSessionReturnsNameSource renamed to TestGetSessionPopulatesNameSourceInternally with updated comments — the test was verifying that the internal Go struct field is populated (correct and needed for the push path), not that name_source appears in API responses (it doesn't)

The CheckSchemaCompat probe was added before the simplification, when the PG read path did select name_source. It should have been pruned in the same pass that removed it from pgSessionCols, but was kept on the assumption it was still needed. That was wrong — removing it now.

[roborev-ci]

roborev: Combined Review (910c576)

Medium finding: clearing a user rename can lose the original imported title.

• Medium: [internal/db/sessions.go:1881] display_name is used for both backend/imported titles and user overrides. RenameSession overwrites that single stored value and clears it to NULL, so imported or DB-backed sessions without a classifiable path can lose their original title and fall back to first message/project.
  • Fix: Store backend/imported names separately from user-provided overrides, compute the returned display name on reads, and add a rename-clear test for an imported or DB-backed titled session.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 10m16s), codex_security (codex/security, done, 2m19s) | Total: 12m42s

[gordonwoodhull]

This is the same "imported sessions" item raised many times by roborev and addressed here. Is it worth the complication of an extra field for this case?

[mariusvniekerk]

having the agentsview-owned field (ie not inferred) does feel like a good thing, since we do occasionally reparse sessions from the storage, so that field could get lost otherwise.

[gordonwoodhull]

Following up on @mariusvniekerk's guidance to store all names and send the correct computed field from the Go side — I've implemented the two-field design:

What changed

display_name and session_name are now separate columns with distinct owners:

• session_name TEXT — parser/importer-provided; freely overwritten on every re-parse (Claude /rename, native conversation titles from 7 other agents, importer titles)
• display_name TEXT — user override only; touched exclusively by RenameSession

All read queries return COALESCE(display_name, session_name) AS display_name. The frontend is unchanged — it still reads one display_name field and remains unaware of the distinction.

What this fixes

The long-standing "imported sessions lose their title when you clear a custom rename" issue: previously display_name was the only stored value, so clearing a user rename left nothing to fall back to. Now session_name is always there independently. Clearing a rename immediately reveals the session name via the COALESCE — no file re-parse required.

What was deleted

• name_source column and all its threading (CASE expression in upsert, schema, PG DDL, push fingerprint, CheckSchemaCompat probe)
• BackfillNameSource migration function (no longer needed — display_name on upgrade from main is always user-owned by construction)
• ParsedSessionNameFields (two-return helper) → replaced by ParsedSessionName (single *string)
• The rename-clear re-parse machinery (ResetFileStateByPath, SyncPaths call in the rename handler) — the two-field design makes it unnecessary

What was renamed

• ParsedSession.DisplayName renamed to ParsedSession.SessionName throughout all parsers — the root cause of the original conflation

[roborev-ci]

roborev: Combined Review (0927a9a)

High-level verdict: changes need fixes before merge because parsed session names are dropped on write and hidden in several read paths.

High

• internal/sync/engine.go:5118, internal/server/upload.go:197
  • Parsed session names are written into Session.DisplayName, but upsertSessionSQL now only inserts/updates session_name from Session.SessionName. Sync reparses and uploads therefore drop /rename and native session titles instead of persisting them.
  • Fix: assign db.ParsedSessionName(...) to SessionName in both converters, and update the new converter tests to assert SessionName, not DisplayName.

Medium

• internal/postgres/sessions.go:33, internal/postgres/sessions.go:601, internal/postgres/messages.go:217, internal/postgres/usage.go:116

  • PostgreSQL read paths still use raw display_name, so session_name values pushed to PG are not shown in pg serve session lists/details/sidebar, search result names, name-only search matches, or usage top-session labels.
  • Fix: mirror the SQLite read semantics with COALESCE(display_name, session_name) AS display_name in PG session/sidebar/search/usage queries, and cover the session-name-only PG cases in tests.

• internal/db/pins.go:123, internal/postgres/curation.go:185

  • The all-pins metadata query still returns raw s.display_name. PinnedPage prefers this backend metadata when present, so sessions that only have session_name fall back to first message/project instead of showing the agent-provided name.
  • Fix: select COALESCE(s.display_name, s.session_name) for SessionDisplayName in both SQLite and PostgreSQL pinned-message metadata queries.

---

Panel: ci_default_security | Synthesis: codex, 11s | Members: codex_default (codex/default, done, 5m59s), codex_security (codex/security, done, 1m10s) | Total: 7m20s

[roborev-ci]

roborev: Combined Review (aed042d)

The PR has several medium-severity correctness issues around the new session_name model; no security issues were found.

Medium

• internal/postgres/session_name_pgtest_test.go:61
  The new PG integration test calls sync.Push(ctx, PushOptions{Full: true}), but PushOptions does not exist and (*Sync).Push has signature Push(ctx, full bool, onProgress func(PushProgress)) (PushResult, error). This makes the pgtest test package fail to compile.
  Fix: Call _, err := sync.Push(ctx, true, nil) and assert on err.

• internal/postgres/schema.go:1290
CheckSchemaCompat does not probe sessions.session_name, but multiple PG read paths now reference it via COALESCE(display_name, session_name). With a read-only PG role, pg serve skips EnsureSchema, passes this compatibility check, then fails at request time with column session_name does not exist.
  Fix: Include session_name in the sessions compatibility probe.

• internal/postgres/messages.go:252
  PostgreSQL search still matches only s.display_name and s.first_message in the name branch, even though result names now use COALESCE(s.display_name, s.session_name, ...). Sessions whose only visible title comes from session_name are displayed with that title but cannot be found by searching it.
  Fix: Mirror the SQLite query: use COALESCE(s.display_name, s.session_name) in the CASE and WHERE predicates, and include session_name in the fallback snippet.

• internal/db/db.go:461
  The SQLite migration only adds session_name; it does not migrate existing imported titles that were previously stored in display_name. Since UpsertSession no longer writes display_name, those legacy importer-owned values become permanent user overrides and mask future session_name updates.
  Fix: Add a migration/backfill path for legacy importer/parser-owned display names, or update import reprocessing to clear/move legacy display_name when it is not a user rename.

---

Panel: ci_default_security | Synthesis: codex, 14s | Members: codex_default (codex/default, done, 5m30s), codex_security (codex/security, done, 1m45s) | Total: 7m29s

[roborev-ci]

roborev: Combined Review (ee8399f)

Summary verdict: changes are mostly sound, but two Medium issues need attention before merge.

Medium

• internal/db/orphaned.go:392 - The v34 resync metadata copy treats every old non-NULL display_name as a user rename. Existing databases can already contain parser/importer-owned titles in display_name, so after the fresh parse writes session_name, this copy overlays the old title back into display_name; COALESCE(display_name, session_name) then keeps showing stale titles and shadows future session-name updates.

  • Fix: preserve only confirmed user renames, for example via name_source='user' when present, or define an explicit legacy migration policy before overlaying metadata.

• internal/postgres/usage.go:759 - PostgreSQL top-session metadata still falls back from display_name directly to first_message, so sessions with only session_name lose their agent-provided title in GetTopSessionsByCost, unlike SQLite.

  • Fix: match SQLite with COALESCE(NULLIF(COALESCE(display_name, session_name), ''), NULLIF(first_message, ''), NULLIF(project, ''), id).

---

Panel: ci_default_security | Synthesis: codex, 10s | Members: codex_default (codex/default, done, 6m30s), codex_security (codex/security, done, 3m21s) | Total: 10m1s

[roborev-ci]

roborev: Combined Review (d64101a)

Medium-risk issue found; no Critical or High findings.

Medium

• internal/db/orphaned.go:392 - Metadata copy treats every non-NULL old display_name as user-owned. Databases created by the previous name_source design can still contain agent-provided names in display_name; this overlay will copy those stale agent names into the new user-owned display_name, blocking freshly parsed session_name values.

  Fix: When old_db.sessions.name_source exists, copy display_name only for name_source = 'user'; for legacy DBs without name_source, keep the current non-NULL fallback.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 8m35s), codex_security (codex/security, done, 2m6s) | Total: 10m48s

[roborev-ci]

roborev: Combined Review (da42080)

Medium-risk issue found: one import error path can silently report success while leaving session metadata stale.

Medium

• internal/importer/importer.go:303
  Problem: ChatGPT re-import ignores RefreshSessionName errors, so a failed metadata write is reported as a successful skip while session_name and local_modified_at may remain stale.
  Fix: Handle the error like other import write failures: log it, increment stats.Errors, and avoid counting the row as skipped.

---

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 13m10s), codex_security (codex/security, done, 2m38s) | Total: 15m54s

[roborev-ci]

roborev: Combined Review (e57cc06)

Summary verdict: One medium issue remains; no critical or high-severity findings were reported.

Medium

• frontend/src/lib/components/command-palette/CommandPalette.svelte:227
  Recent-session rows still render and filter only on first_message/project. Because sidebar rows with resolved display_name no longer hydrate first_message, named sessions can show as just the project in the command palette and cannot match short client-side searches by name.
  Fix: Use session.display_name ?? normalizeMessagePreview(session.first_message) ?? session.project for the recent-session label, and include display_name in the short-query filter.

---

Panel: ci_default_security | Synthesis: codex, 6s | Members: codex_default (codex/default, done, 9m59s), codex_security (codex/security, done, 2m2s) | Total: 12m7s

[roborev-ci]

roborev: Combined Review (fe1e178)

Summary verdict: one medium correctness issue remains; no medium-or-higher security findings were reported.

Medium

• Location: internal/db/sessions.go:79
• Problem: SQLite GetSessionFull now returns raw display_name instead of the resolved cascade, while the PostgreSQL GetSessionFull path still returns COALESCE(display_name, session_name). GetSessionFull is part of the shared store interface, so agent-named sessions can appear unnamed on local full-session reads but named on PostgreSQL/detail/list reads.
• Fix: Split the raw full-row read needed for PostgreSQL push from public/store full-session reads, or add a separate raw field so DisplayName stays resolved consistently across backends.

---

Panel: ci_default_security | Synthesis: codex, 7s | Members: codex_default (codex/default, done, 11m17s), codex_security (codex/security, done, 1m30s) | Total: 12m54s

[wesm]

I'm looking at this now

[wesm]

This looks good to go, I'll leave the last roborev finding unaddressed (technically accurate, not worth fixing)

[roborev-ci]

roborev: Combined Review (7dceec9)

Summary verdict: one medium-severity upgrade-path issue should be fixed before merge.

Medium

• Location: internal/db/orphaned.go:401
• Problem: The upgrade resync copies every non-NULL old display_name into the new DB. In existing released databases, many of those values were agent/importer titles, not user renames. Since read paths now use COALESCE(display_name, session_name), the copied value can permanently shadow the freshly parsed session_name. Later ChatGPT/ClaudeAI title changes or Claude /rename reparses may update session_name but remain invisible for upgraded rows.
• Fix: Avoid unconditionally overlaying old display_name; only copy confirmed user-owned names, or skip the overlay when the fresh session_name already accounts for the old title. Add an upgrade-path test covering an old agent/import title followed by a re-import title change.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 7m42s), codex_security (codex/security, done, 1m21s) | Total: 9m12s

[roborev-ci]

roborev: Combined Review (3b09342)

Medium issues remain around backend parity and DuckDB incremental sync; no security findings were reported.

Medium

• internal/db/sessions.go:79 - SQLite GetSessionFull scans raw display_name and hides session_name behind json:"-", while PostgreSQL and DuckDB return the coalesced visible name. This creates backend parity drift and can make callers serializing full SQLite sessions, such as health --json, omit agent-provided session names.
  Fix: Keep GetSessionFull returning the coalesced visible DisplayName, and use a separate raw/full scan path for PG push or other internal ownership-sensitive code.

• internal/duckdb/sync.go:553 - DuckDB pushes now write session_name, but duckSessionFingerprintFields does not include sess.SessionName. Incremental DuckDB sync can classify a session-name-only change as unchanged and leave the mirror with a stale title.
  Fix: Add nilString(sess.SessionName) to the DuckDB session fingerprint fields and add a regression test that a SessionName change changes the fingerprint.

---

Panel: ci_default_security | Synthesis: codex, 9s | Members: codex_default (codex/default, done, 10m0s), codex_security (codex/security, done, 39s) | Total: 10m48s

[roborev-ci]

roborev: Combined Review (d818e5d)

No issues found.

---

Panel: ci_default_security | Synthesis: codex | Members: codex_default (codex/default, done, 8m39s), codex_security (codex/security, done, 1m4s) | Total: 9m43s