feat: Expose opencode's auto generated session titles#569
Conversation
📝 WalkthroughWalkthroughCapture OpenCode-generated session titles at sandbox runtime, emit Changes
Sequence DiagramsequenceDiagram
participant SR as Sandbox Runtime
participant CP as Control Plane
participant SI as Session Index
participant WC as Web Client
SR->>SR: detect session idle
SR->>SR: fetch OpenCode /session/{id}
SR-->>CP: emit sandbox event: session_title (title, sandboxId, timestamp)
CP->>CP: processSandboxEvent -> applyGeneratedSessionTitle
CP->>CP: updateSessionTitleIfUnset(sessionId, title, updatedAt)
CP->>SI: updateTitleIfNewer(id, title, updatedAt) (async)
SI-->>CP: success/failure
CP->>CP: broadcast WebSocket {type:"session_title", title}
CP-->>WC: WebSocket delivers session_title
WC->>WC: update sessionState.title
WC->>WC: mutate(SIDEBAR_SESSIONS_KEY) -> revalidate sidebar list
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/control-plane/src/session/durable-object.ts`:
- Around line 1507-1519: The background write in syncSessionIndexTitle is using
SessionIndexStore.updateTitle unconditionally which can race and overwrite a
newer manual rename; change the background call to a conditional index update
such as SessionIndexStore.updateTitleIfUnset(sessionId, title) or
SessionIndexStore.updateTitleIfNewer(sessionId, title, updatedAt) so the index
write only applies when the index is empty or older, and keep the ctx.waitUntil
wrapper and existing error logging (log.error
"session_index.update_title.background_error") intact; locate the
syncSessionIndexTitle method and replace the updateTitle invocation with the
appropriate IfUnset/IfNewer variant and pass any required timestamp or sentinel
so manual renames are preserved.
In `@packages/control-plane/src/session/repository.test.ts`:
- Around line 155-162: The test claims to exercise updateSessionTitleIfUnset but
seeds a non-empty title; change the mock fixture in the test to return a row
with title null (or empty string) so the "unset" branch is exercised, then call
repo.updateSessionTitleIfUnset("sess-1", "Generated title", 4000) and assert it
returns true; keep the existing assertions that mock.calls[0].query contains the
WHERE clause and that mock.calls[0].params equals ["Generated title", 4000,
"sess-1"] so the SQL and params are still validated for the unset-title path in
updateSessionTitleIfUnset.
In `@packages/control-plane/src/session/repository.ts`:
- Around line 286-296: The method updateSessionTitleIfUnset can return true even
if no row was updated because it compares this.getSession()?.title to title;
instead use the database update result to determine success: capture the
result/metadata from this.sql.exec (e.g., affectedRows/changes/rowCount
depending on the DB driver) when calling UPDATE in updateSessionTitleIfUnset and
return true only if the exec result indicates at least one row was updated; do
not rely on getSession() for success detection. Ensure you reference
updateSessionTitleIfUnset, this.sql.exec, and getSession in the change so
reviewers can find the logic to adjust.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0169aab2-b442-43d6-92e7-3a0560c7cbed
📒 Files selected for processing (8)
packages/control-plane/src/session/durable-object.tspackages/control-plane/src/session/repository.test.tspackages/control-plane/src/session/repository.tspackages/sandbox-runtime/src/sandbox_runtime/bridge.pypackages/sandbox-runtime/tests/test_bridge_sse.pypackages/shared/src/types/index.tspackages/web/src/hooks/use-session-socket.test.tsxpackages/web/src/hooks/use-session-socket.ts
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/control-plane/src/session/durable-object.ts (1)
1551-1575: Optional:notifyParentOfStatusChangeis being reused for title-only updates.At Line 1567 the call passes the unchanged
session.status, so the parent fan-out is really a "child updated" notification rather than a status change. The method/payload still works, but the name is misleading and conflates two intents. Consider extracting a smallnotifyParentOfChildUpdate(childSessionId, { status, title })helper (or renaming the existing one) so callers don't have to read the implementation to understand they can also propagate title changes through it.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/control-plane/src/session/durable-object.ts` around lines 1551 - 1575, The call in applyGeneratedSessionTitle uses notifyParentOfStatusChange to propagate a title-only update which is misleading; add a new helper (e.g., notifyParentOfChildUpdate or notifyParentOfTitleChange) that accepts the child session (or child session id), publicSessionId, and the updated fields (at minimum title and optionally status) and use that from applyGeneratedSessionTitle instead of notifyParentOfStatusChange; keep notifyParentOfStatusChange semantics for real status transitions and update callers to use the new notifyParentOfChildUpdate helper where they only need to propagate child metadata changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/control-plane/src/session/repository.ts`:
- Around line 214-215: The code reads result.rowsWritten immediately after
calling this.sql.exec(...) without consuming the returned cursor; consume the
cursor (e.g., call .toArray() or iterate it) before checking rowsWritten to
ensure the final count is accurate. Update the code around the
this.sql.exec(...) call (the variable named result or similar) to await
result.toArray() or iterate the cursor first, then read result.rowsWritten > 0
and return based on that consumed value.
---
Nitpick comments:
In `@packages/control-plane/src/session/durable-object.ts`:
- Around line 1551-1575: The call in applyGeneratedSessionTitle uses
notifyParentOfStatusChange to propagate a title-only update which is misleading;
add a new helper (e.g., notifyParentOfChildUpdate or notifyParentOfTitleChange)
that accepts the child session (or child session id), publicSessionId, and the
updated fields (at minimum title and optionally status) and use that from
applyGeneratedSessionTitle instead of notifyParentOfStatusChange; keep
notifyParentOfStatusChange semantics for real status transitions and update
callers to use the new notifyParentOfChildUpdate helper where they only need to
propagate child metadata changes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 812ae65e-3e6e-4390-83c0-cb92d9032eb6
📒 Files selected for processing (5)
packages/control-plane/src/db/session-index.test.tspackages/control-plane/src/db/session-index.tspackages/control-plane/src/session/durable-object.tspackages/control-plane/src/session/repository.test.tspackages/control-plane/src/session/repository.ts
| readonly rowsRead?: number; | ||
| readonly rowsWritten?: number; |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Cloudflare Workers Durable Object SqlStorageCursor rowsWritten per-query semantics for UPDATE statement
💡 Result:
For Cloudflare Durable Objects SQLite storage, SqlStorageCursor.rowsWritten is “per SQL query” (per sql.exec(...) call). It starts at the number of rows written so far as part of that query, can increase as you iterate the cursor, and the final value is what’s used for SQL billing. So for an UPDATE statement, the rowsWritten you observe will correspond to the rows written by that UPDATE statement within that single sql.exec("UPDATE ...") query—if you read/advance the cursor further, rowsWritten may increase, and you should treat the final rowsWritten value (after the cursor is fully consumed) as the definitive count for billing. Notes from the docs: - rowsWritten “may increase as you iterate the cursor” and “the final value is used for SQL billing.” - This applies to the cursor’s SQL query overall (not globally across multiple sql.exec calls). - When writing data, index updates also count as additional rows (so UPDATEs that change indexed columns can increase rowsWritten beyond the number of rows matched by the UPDATE).
Citations:
- 1: https://developers.cloudflare.com/durable-objects/api/sql-storage/
- 2: https://developers.cloudflare.com/durable-objects/api/sqlite-storage-api/
- 3: https://cloudflare-docs-7ou.pages.dev/durable-objects/api/sql-storage/
- 4: https://github.com/cloudflare/cloudflare-docs/blob/production/src/content/docs/durable-objects/api/sqlite-storage-api.mdx
🏁 Script executed:
sed -n '280,305p' packages/control-plane/src/session/repository.tsRepository: ColeMurray/background-agents
Length of output: 782
🌐 Web query:
Cloudflare Durable Objects sql.exec UPDATE return value rowsWritten consumption
💡 Result:
In Cloudflare Durable Objects (SQLite-backed), this.ctx.storage.sql.exec(...) returns a cursor (SqlStorageCursor) with accounting fields: 1) rowsWritten - Meaning: “The number of rows written so far as part of this SQL query.” It “may increase as you iterate the cursor.” The “final value is used for SQL billing.” - So for an UPDATE ... statement executed via sql.exec, the relevant billing/usage counter is the cursor’s rowsWritten (not the return value from .one / .toArray). 2) “Consumption” / billing relationship - Cloudflare’s Durable Objects SQL billing is based on row reads/writes usage counts, where writes are tracked via rowsWritten (final value after the cursor is fully consumed). 3) Return value vs rowsWritten - The SQL execution API is synchronous: you call exec(...) immediately and then read results from the cursor (e.g., via cursor.one / iterating / cursor.toArray). - For UPDATE queries that change rows, the cursor’s rowsWritten increases as the cursor is processed; if you don’t fully consume the cursor, the “final” rowsWritten used for billing may not be reached. Practical notes - If your UPDATE does not return rows (common), prefer relying on the cursor fields (e.g., cursor.rowsWritten) rather than expecting an .one-style row object. - Cloudflare also notes that index updates count as additional rows written (“every row update of an index counts as an additional row”). Grounding details (what to look at) - SqlStorageCursor properties include rowsRead and rowsWritten, and both specify that the final values are used for SQL billing. If you share an exact snippet (your UPDATE SQL plus how you consume the cursor), I can tell you exactly where to read rowsWritten and whether you need to fully consume the cursor.
Citations:
- 1: https://developers.cloudflare.com/durable-objects/api/sql-storage/
- 2: https://cloudflare-docs-7ou.pages.dev/durable-objects/api/sql-storage/
- 3: https://developer.cloudflare.com/durable-objects/platform/pricing/
🌐 Web query:
Cloudflare Durable Objects sql.exec UPDATE statement cursor rowsWritten immediately available no cursor iteration
💡 Result:
In Cloudflare Durable Objects’ SQLite SQL API (ctx.storage.sql.exec), the sql.exec call returns a SqlStorageCursor. That cursor can be iterated (for...of or cursor.next) and it tracks billing counters like rowsRead and rowsWritten. Regarding your specific requirement: - You generally should NOT rely on “UPDATE … and rowsWritten is immediately available without cursor iteration”. The official docs only specify that rowsRead/rowsWritten may increase as you iterate the cursor and that the final value is used for SQL billing. That implies the final rowsWritten value is only known after the cursor has been consumed (e.g., by iterating, toArray, etc.). Practical implication / recommended pattern: - Execute the UPDATE and then fully consume the returned cursor so you don’t miss the final rowsWritten value (and to ensure the statement is fully processed). For example: - this.ctx.storage.sql.exec("UPDATE ...", ...).toArray; - or iterate/consume via for (let row of cursor) {} / cursor.toArray / cursor.raw.toArray depending on your API needs. If your goal is “fire-and-forget” updates, you should still be aware that rowsWritten accounting is tied to cursor consumption per the documented billing behavior.
Citations:
- 1: https://developers.cloudflare.com/durable-objects/api/sql-storage/
- 2: https://developer.cloudflare.com/durable-objects/api/sqlite-storage-api/
- 3: https://developers.cloudflare.com/durable-objects/api/sqlite-storage-api/
Cursor must be consumed before reading rowsWritten.
The code reads result.rowsWritten immediately after sql.exec() without consuming the cursor. According to Cloudflare's documentation, rowsWritten may increase as you iterate the cursor, and the final value is only guaranteed after the cursor is fully consumed (e.g., via .toArray() or iteration). Reading rowsWritten before consumption could yield an incomplete count and cause the function to return an incorrect boolean.
Add cursor consumption (e.g., this.sql.exec(...).toArray()) before checking rowsWritten > 0, or store the cursor and consume it before the return statement.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/control-plane/src/session/repository.ts` around lines 214 - 215, The
code reads result.rowsWritten immediately after calling this.sql.exec(...)
without consuming the returned cursor; consume the cursor (e.g., call .toArray()
or iterate it) before checking rowsWritten to ensure the final count is
accurate. Update the code around the this.sql.exec(...) call (the variable named
result or similar) to await result.toArray() or iterate the cursor first, then
read result.rowsWritten > 0 and return based on that consumed value.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/control-plane/src/session/repository.ts (1)
288-299: Conditional update +rowsWritten-based success reporting looks good.The
WHERE id = ? AND (title IS NULL OR TRIM(title) = '')predicate correctly scopes the update to "unset" titles, parameter order matches the SQL placeholders, and consuming the cursor viaresult.toArray()before readingrowsWrittenavoids the prior race where the count could be incomplete. Returning(result.rowsWritten ?? 0) > 0is also resilient to Cloudflare's index-update accounting (whererowsWrittenmay exceed the matched row count).One small observation (non-blocking): the
result.toArray()return value is intentionally discarded purely to force cursor consumption. A short comment there would help future readers understand it isn't dead code, e.g.:💡 Optional readability nit
- result.toArray(); + // Consume the cursor so rowsWritten reflects the final count (Cloudflare SqlStorageCursor semantics). + result.toArray(); return (result.rowsWritten ?? 0) > 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/control-plane/src/session/repository.ts` around lines 288 - 299, The call to result.toArray() in updateSessionTitleIfUnset is intentionally invoked only to fully consume the DB cursor before checking result.rowsWritten, but that intent isn't documented; add a short one-line comment immediately above the result.toArray() call (referencing result and rowsWritten in the comment) stating that the return value is intentionally ignored and the call forces cursor consumption so rowsWritten is reliable, leaving the rest of the logic (the SQL, parameters, and final return using (result.rowsWritten ?? 0) > 0) unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/control-plane/src/session/repository.ts`:
- Around line 288-299: The call to result.toArray() in updateSessionTitleIfUnset
is intentionally invoked only to fully consume the DB cursor before checking
result.rowsWritten, but that intent isn't documented; add a short one-line
comment immediately above the result.toArray() call (referencing result and
rowsWritten in the comment) stating that the return value is intentionally
ignored and the call forces cursor consumption so rowsWritten is reliable,
leaving the rest of the logic (the SQL, parameters, and final return using
(result.rowsWritten ?? 0) > 0) unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 766433f2-58b9-4164-b5db-22dc1c7548a8
📒 Files selected for processing (2)
packages/control-plane/src/session/repository.test.tspackages/control-plane/src/session/repository.ts
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sandbox-runtime/src/sandbox_runtime/bridge.py (1)
1186-1194:⚠️ Potential issue | 🟠 MajorMove the title lookup off the completion hot path.
session_titleis best-effort, but these awaits run inside the same timeout scope as the final-state fetch. If OpenCode is slow or the session is near its inactivity deadline, the extra HTTP round-trip can delay or cancel the actual prompt completion. Please fetch the title after final-state emission, or dispatch it separately with its own short timeout.Suggested adjustment
- await self._emit_session_title_event() async for final_event in self._fetch_final_message_state( message_id, opencode_message_id, cumulative_text, allowed_assistant_msg_ids, compaction_occurred=compaction_occurred, ): yield final_event + await self._emit_session_title_event()Also applies to: 1211-1219
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sandbox-runtime/src/sandbox_runtime/bridge.py` around lines 1186 - 1194, The current code awaits self._emit_session_title_event() before yielding final events inside the same timeout as self._fetch_final_message_state (called with message_id, opencode_message_id, etc.), which can delay or cancel completion; change the flow so that _fetch_final_message_state yields all final events first and only then invoke _emit_session_title_event() (or schedule it in a separate task with its own short timeout) so the title lookup is off the completion hot path; apply the same change where _emit_session_title_event() is awaited around lines handling the same pattern (e.g., the block using _fetch_final_message_state at the other location).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@packages/sandbox-runtime/src/sandbox_runtime/bridge.py`:
- Around line 1186-1194: The current code awaits
self._emit_session_title_event() before yielding final events inside the same
timeout as self._fetch_final_message_state (called with message_id,
opencode_message_id, etc.), which can delay or cancel completion; change the
flow so that _fetch_final_message_state yields all final events first and only
then invoke _emit_session_title_event() (or schedule it in a separate task with
its own short timeout) so the title lookup is off the completion hot path; apply
the same change where _emit_session_title_event() is awaited around lines
handling the same pattern (e.g., the block using _fetch_final_message_state at
the other location).
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5ef968c1-bb5c-43ec-a508-1c47b5fd2f98
📒 Files selected for processing (1)
packages/sandbox-runtime/src/sandbox_runtime/bridge.py
1 participant
Expose opencode's autogenerated session titles to open inspect. It will get the title after the first turn and rename the session if a session has not been manually named.
Summary by CodeRabbit
New Features
Bug Fixes
Tests