Repository navigation
Replace CLI in-memory store with Turso-backed @cyrus/database - #18
Merged
Merged
Conversation
Persist projects, threads, and conversations under CYRUS_HOME so worker restarts retain history. Add monotonic seq, afterSeq reads, persist-before-broadcast, and Result-based repository errors mapped to ORPC. Co-authored-by: Cursor <cursoragent@cursor.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughWalkthroughThe CLI's in-memory project/thread stores are replaced with a new shared Turso/Drizzle database package providing schema models and repository functions for projects, threads, and conversations. CLI handlers, coordinator, and chat streaming logic migrate to these async repositories with centralized error mapping. Web identity storage moves to a persisted Zustand store, streaming UI state is tracked per-thread, and server database setup, migration snapshots, and dependency versions are updated. ChangesTurso persistence migration
Server database config and snapshots
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant Runtime
participant ChatHandler
participant StreamUtils
participant ConversationsRepository
participant WebClient
Runtime->>ChatHandler: token/thought event
ChatHandler->>StreamUtils: trackDelta(event, buffers)
ChatHandler-->>WebClient: broadcast streaming chunk
Runtime->>ChatHandler: message_completed event
ChatHandler->>StreamUtils: resolvePersistEvent(event, buffers)
ChatHandler->>ConversationsRepository: appendConversation(threadId, chunk)
ConversationsRepository-->>ChatHandler: persisted entry with seq
ChatHandler-->>WebClient: broadcast persisted chunk
sequenceDiagram
participant CLIWorker
participant DatabaseConnection
participant Turso
participant ProjectsRepository
CLIWorker->>DatabaseConnection: initDatabase()
DatabaseConnection->>Turso: connect + push schema
DatabaseConnection->>Turso: configure pragmas
DatabaseConnection-->>CLIWorker: opened worker db
CLIWorker->>ProjectsRepository: resolveProjectCwd(projectId)
ProjectsRepository->>Turso: query projects
ProjectsRepository-->>CLIWorker: Result<cwd, RepositoryError>
Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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. 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: 6
🧹 Nitpick comments (3)
shared/database/src/repositories/conversations.ts (1)
27-46: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider wrapping the insert and thread update in a transaction.
appendConversationperforms an insert intoconversationsfollowed by an update tothreads.updatedAtas two independent operations. If the update fails, the conversation entry is already persisted but the thread'supdatedAtis stale, causing it to appear less recently active inlistThreadsordering. Wrapping both inconnection.db.transaction()would make them atomic.♻️ Proposed refactor
return tryRepo(async () => { const id = randomId(); const createdAt = nowISO(); - const [row] = await connection.db - .insert(conversations) - .values({ - id, - threadId, - chunk: JSON.stringify(chunk), - createdAt, - }) - .returning(); - if (!row) { - throw { - type: "persist_failed", - message: `failed to persist conversation entry for thread ${threadId}`, - } satisfies RepositoryError; - } - - await connection.db - .update(threads) - .set({ updatedAt: createdAt }) - .where(eq(threads.id, threadId)); - - return parseConversationEntry(row); + return await connection.db.transaction(async (tx) => { + const [row] = await tx + .insert(conversations) + .values({ + id, + threadId, + chunk: JSON.stringify(chunk), + createdAt, + }) + .returning(); + if (!row) { + throw { + type: "persist_failed", + message: `failed to persist conversation entry for thread ${threadId}`, + } satisfies RepositoryError; + } + + await tx + .update(threads) + .set({ updatedAt: createdAt }) + .where(eq(threads.id, threadId)); + + return parseConversationEntry(row); + }); });🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/database/src/repositories/conversations.ts` around lines 27 - 46, The appendConversation flow currently does the conversation insert and threads.updatedAt update as separate operations, so a failure in the second step leaves the data inconsistent. Update the appendConversation logic in conversations.ts to run both the insert into conversations and the update of threads inside connection.db.transaction() so they succeed or fail together; keep the existing persist_failed handling for the insert result and preserve the use of id, threadId, createdAt, and eq(threads.id, threadId) within the transactional block.shared/database/src/connection.ts (1)
10-68: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider adding a
close/disposemethod for resource cleanup.The singleton
DatabaseConnectionhas no way to release the native client. While the CLI process exit handles cleanup in practice, aclosemethod would improve testability and prevent resource leaks ifopenis ever called more than once.♻️ Suggested addition
get db(): DrizzleDb { if (!this.drizzleDb) throw new Error("DatabaseConnection is not initialized"); return this.drizzleDb; } + async close(): Promise<void> { + if (this.nativeClient) { + await this.nativeClient.close?.(); + this.nativeClient = null; + } + this.drizzleDb = null; + } + open(🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/database/src/connection.ts` around lines 10 - 68, The DatabaseConnection singleton has no cleanup path for the native client, which can leave resources open and makes repeated open/setup calls harder to manage. Add a close/dispose method to DatabaseConnection that safely releases nativeClient and resets drizzleDb, and have it guard against double-closing so callers can invoke it during tests or shutdown. Keep the change localized to the DatabaseConnection class and preserve the existing client/db getters’ initialization behavior.Source: Linters/SAST tools
shared/utils/src/time.ts (1)
14-16: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low valueConsider using
new Date().toISOString()for UTC consistency.
formatISO(new Date())returns a local-timezone string with offset (e.g.,2024-01-15T10:30:00+02:00) and no milliseconds, whilenew Date().toISOString()returns UTC (2024-01-15T08:30:00.000Z) with millisecond precision. For database timestamps, UTC is generally preferred for consistency — especially if the machine's timezone changes (DST transitions). Sincedate-fnsis already a dependency, this is a style/consistency choice rather than a correctness issue for a single-machine CLI.♻️ Optional refactor for UTC timestamps
export function nowISO(): string { - return formatISO(new Date()); + return new Date().toISOString(); }If you keep
formatISO, consider adding{ representation: "complete" }for millisecond precision to avoid identical timestamps on rapid sequential inserts.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/utils/src/time.ts` around lines 14 - 16, The nowISO helper in time.ts currently uses formatISO(new Date()), which produces a local-time offset string and omits milliseconds; update nowISO to return a UTC timestamp using new Date().toISOString(), or if you keep formatISO, adjust the formatting for complete precision. Keep the change scoped to the nowISO function so database timestamp generation stays consistent across timezone changes.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@apps/cli/src/commands/service/worker.ts`:
- Around line 30-32: Add explicit error handling around initDatabase() in
worker.ts so a database startup failure does not leave the worker running in a
broken state. Wrap the initDatabase() call near createWorkerRuntime() in a
try-catch, log a clear user-facing initialization failure message with the
underlying error, and then terminate cleanly by calling shutdown() and exiting
the process instead of relying on the unhandledRejection path. Keep the fix
localized to the worker startup flow so the worker never proceeds without a
usable database.
In `@apps/cli/src/utils/streams.ts`:
- Around line 20-23: The delta accumulation in trackDelta is reading from
messageBuffers even when the selected target is thoughtBuffers, so
thought/reasoning text gets overwritten instead of appended. Update trackDelta
to read the existing value from the same buffer it is writing to, using the
event.messageId key consistently, and keep resolvePersistEvent relying on the
fully accumulated thought text for reasoning_completed events.
In `@apps/web/src/stores/identity.ts`:
- Line 33: The controller identity store currently writes only
CONTROLLER_IDENTITY, so legacy users with CONTROLLER_ID/CONTROLLER_NAME lose
their previous controller identity on first load. Update the identity
initialization logic in the identity store to check for the old keys before
creating a new value, migrate the existing controller data into
CONTROLLER_IDENTITY when present, and then clear the legacy entries so the
migration runs only once. Use the existing identity store initialization path
and the CONTROLLER_IDENTITY constant as the anchor points for the change.
In `@package.json`:
- Around line 23-24: The catalog update in package.json should keep date-fns
because apps/web/package.json still resolves it through catalog:, and removing
it would break that workspace; update the catalog entry list to preserve
date-fns and remove only the unused random-word-slugs entry. Use the catalog
section in package.json as the target for this change.
In `@shared/database/src/repositories/threads.ts`:
- Around line 31-60: The ensureThread update path is matching threads only by id
and then returning the passed-in projectId, which can update or report a thread
from the wrong project. Update the lookup in ensureThread to filter by both
threads.id and projectId, and when building the ThreadSchema result use
existing.projectId instead of the input parameter so the returned object always
reflects the database row.
In `@shared/database/src/utils/error.ts`:
- Around line 19-21: `fromUnknown` and `repositoryErrorMessage` currently allow
unexpected objects with a `type` field to slip through, which can make the
default branch return a non-string value. Update `fromUnknown` in error.ts to
validate that `type` is one of the known RepositoryError variants before
casting, and make `repositoryErrorMessage`’s default path return a safe string
fallback instead of the raw `error` object. Use the existing `RepositoryError`,
`fromUnknown`, and `repositoryErrorMessage` symbols to keep the narrowing
consistent for callers like coordinator.ts.
---
Nitpick comments:
In `@shared/database/src/connection.ts`:
- Around line 10-68: The DatabaseConnection singleton has no cleanup path for
the native client, which can leave resources open and makes repeated open/setup
calls harder to manage. Add a close/dispose method to DatabaseConnection that
safely releases nativeClient and resets drizzleDb, and have it guard against
double-closing so callers can invoke it during tests or shutdown. Keep the
change localized to the DatabaseConnection class and preserve the existing
client/db getters’ initialization behavior.
In `@shared/database/src/repositories/conversations.ts`:
- Around line 27-46: The appendConversation flow currently does the conversation
insert and threads.updatedAt update as separate operations, so a failure in the
second step leaves the data inconsistent. Update the appendConversation logic in
conversations.ts to run both the insert into conversations and the update of
threads inside connection.db.transaction() so they succeed or fail together;
keep the existing persist_failed handling for the insert result and preserve the
use of id, threadId, createdAt, and eq(threads.id, threadId) within the
transactional block.
In `@shared/utils/src/time.ts`:
- Around line 14-16: The nowISO helper in time.ts currently uses formatISO(new
Date()), which produces a local-time offset string and omits milliseconds;
update nowISO to return a UTC timestamp using new Date().toISOString(), or if
you keep formatISO, adjust the formatting for complete precision. Keep the
change scoped to the nowISO function so database timestamp generation stays
consistent across timezone 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: 220972e2-518c-44bc-85e3-c311322cc097
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (45)
apps/cli/package.jsonapps/cli/src/commands/auth/login.tsapps/cli/src/commands/service/worker.tsapps/cli/src/constants/file.tsapps/cli/src/core/acp/events.tsapps/cli/src/core/threads/coordinator.tsapps/cli/src/handlers/controller/chat.tsapps/cli/src/handlers/controller/projects.tsapps/cli/src/handlers/controller/threads.tsapps/cli/src/store/database.tsapps/cli/src/store/projects.tsapps/cli/src/store/threads.tsapps/cli/src/utils/error.tsapps/cli/src/utils/streams.tsapps/server/package.jsonapps/server/src/db/index.tsapps/server/src/db/models/auth.tsapps/web/package.jsonapps/web/src/constants/storage-keys.tsapps/web/src/lib/orpc.tsapps/web/src/stores/identity.tsapps/web/src/utils/conversation-cache.tsapps/web/src/utils/identity.tsopenspec/changes/replace-cli-store-with-turso/tasks.mdpackage.jsonshared/connections/src/schemas/rtc/chat.tsshared/connections/src/schemas/rtc/common.tsshared/connections/src/schemas/rtc/projects.tsshared/connections/src/schemas/rtc/threads.tsshared/database/drizzle.config.tsshared/database/package.jsonshared/database/src/connection.tsshared/database/src/models/conversations.tsshared/database/src/models/index.tsshared/database/src/models/projects.tsshared/database/src/models/threads.tsshared/database/src/repositories/conversations.tsshared/database/src/repositories/projects.tsshared/database/src/repositories/threads.tsshared/database/src/utils/error.tsshared/database/tsconfig.jsonshared/hooks/src/derive-thread.tsshared/utils/package.jsonshared/utils/src/identity.tsshared/utils/src/time.ts
💤 Files with no reviewable changes (4)
- apps/web/package.json
- apps/web/src/utils/identity.ts
- apps/cli/src/store/projects.ts
- apps/cli/src/store/threads.ts
Reopen the database after pushSchema so Drizzle writes flush to disk, accept only a connect factory in DatabaseConnection, close the DB on worker shutdown, and add db:file:studio plus Drizzle Kit 1.0 server migration layout. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
shared/database/src/connection.ts (1)
67-82: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winConsider wrapping
batchhandler in a transaction.The
batchhandler executes statements sequentially viaclient.exec(). If a statement fails midway, previously executed statements are already committed, potentially leaving the schema in a partially-applied state. IfpushSchemaexpects atomicity frombatch, this could cause data integrity issues during schema migration.If the Turso client supports it, wrapping in an explicit transaction would provide rollback safety:
🔒️ Suggested transactional batch handler
batch: async (statements) => { - for (const statement of statements) await client.exec(statement); + await client.exec("BEGIN"); + try { + for (const statement of statements) await client.exec(statement); + await client.exec("COMMIT"); + } catch (error) { + await client.exec("ROLLBACK"); + throw error; + } },If
pushSchemaalready handles atomicity at a higher level or schema statements are idempotent, this can be skipped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@shared/database/src/connection.ts` around lines 67 - 82, The batch handler in push currently runs statements one by one with client.exec, which can leave a partially applied schema if one statement fails. Update the push method’s batch implementation to execute the statements inside an explicit transaction when the DatabasePromise client supports it, so the schema migration is atomic and can roll back on failure; if pushSchema already guarantees atomicity, keep the handler as-is only after confirming that behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@shared/database/src/connection.ts`:
- Around line 67-82: The batch handler in push currently runs statements one by
one with client.exec, which can leave a partially applied schema if one
statement fails. Update the push method’s batch implementation to execute the
statements inside an explicit transaction when the DatabasePromise client
supports it, so the schema migration is atomic and can roll back on failure; if
pushSchema already guarantees atomicity, keep the handler as-is only after
confirming that behavior.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5cd103ab-3f83-45f9-9e21-ebb7a74cd44d
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (18)
apps/cli/package.jsonapps/cli/src/commands/service/worker.tsapps/cli/src/store/database.tsapps/server/src/db/migrations/20260624170920_db_init/migration.sqlapps/server/src/db/migrations/20260624170920_db_init/snapshot.jsonapps/server/src/db/migrations/20260628062857_device_authorization/migration.sqlapps/server/src/db/migrations/20260628062857_device_authorization/snapshot.jsonapps/server/src/db/migrations/meta/0000_snapshot.jsonapps/server/src/db/migrations/meta/0001_snapshot.jsonapps/server/src/db/migrations/meta/_journal.jsonapps/server/src/handlers/signaling.tsapps/web/package.jsonapps/web/src/hooks/use-controller-threads.tspackage.jsonshared/database/.env.exampleshared/database/drizzle.config.tsshared/database/package.jsonshared/database/src/connection.ts
💤 Files with no reviewable changes (4)
- apps/server/src/db/migrations/meta/0001_snapshot.json
- apps/server/src/db/migrations/meta/0000_snapshot.json
- apps/server/src/db/migrations/meta/_journal.json
- apps/web/package.json
✅ Files skipped from review due to trivial changes (3)
- shared/database/.env.example
- apps/server/src/handlers/signaling.ts
- apps/server/src/db/migrations/20260628062857_device_authorization/snapshot.json
🚧 Files skipped from review as they are similar to previous changes (6)
- shared/database/drizzle.config.ts
- apps/cli/src/commands/service/worker.ts
- package.json
- shared/database/package.json
- apps/cli/src/store/database.ts
- apps/cli/package.json
Relax thread ID validation to accept v4 UUIDs, emit turn completion events from the worker, and derive turn state correctly so the composer returns to send once a response finishes. Co-authored-by: Cursor <cursoragent@cursor.com>
Exit cleanly when DB init fails, fix thought delta buffering, scope ensureThread to project ownership, and tighten repository error narrowing. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/cli/src/commands/service/worker.ts (1)
60-66: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winGuard
connection.close()to ensure shutdown always exits.If
connection.close()throws,process.exit(0)on line 65 is never reached. TheunhandledRejectionhandler at line 70 only logs — it doesn't exit — so the process could hang on shutdown. Consider wrapping the cleanup in a try-finally to guarantee exit.🛡️ Proposed fix for shutdown reliability
const shutdown = async () => { - await runtime.agentPool.shutdown(); - device.close(); - signalingSession.close(); - await connection.close(); - process.exit(0); + try { + await runtime.agentPool.shutdown(); + device.close(); + signalingSession.close(); + await connection.close(); + } finally { + process.exit(0); + } };🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/cli/src/commands/service/worker.ts` around lines 60 - 66, The shutdown flow in shutdown currently awaits connection.close() before calling process.exit(0), so a thrown error can prevent the process from exiting. Update the cleanup sequence to guarantee exit even if connection.close() fails, preferably by wrapping the shutdown steps in a try-finally and placing process.exit(0) in the finally path. Keep the existing cleanup for runtime.agentPool.shutdown, device.close, and signalingSession.close, and make sure the fix is applied in the shutdown function so the worker always terminates cleanly.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@openspec/specs/conversation-persistence/spec.md`:
- Around line 64-72: The ordering requirement is too broad and conflicts with
the live-delta flow in chat handling; scope it to persisted ConversationEntry
rows only. Update the spec around the persist-before-broadcast behavior to state
that only durable entries must be written to Turso before their corresponding
ChatChunk is broadcast, while streaming deltas in chat controller paths like
chat.ts may still broadcast immediately with a placeholder seq and persist later
on completion.
---
Outside diff comments:
In `@apps/cli/src/commands/service/worker.ts`:
- Around line 60-66: The shutdown flow in shutdown currently awaits
connection.close() before calling process.exit(0), so a thrown error can prevent
the process from exiting. Update the cleanup sequence to guarantee exit even if
connection.close() fails, preferably by wrapping the shutdown steps in a
try-finally and placing process.exit(0) in the finally path. Keep the existing
cleanup for runtime.agentPool.shutdown, device.close, and
signalingSession.close, and make sure the fix is applied in the shutdown
function so the worker always terminates cleanly.
🪄 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: a4d7e7a8-77bc-46e5-b4c1-f4be37388bdf
⛔ Files ignored due to path filters (1)
bun.lockis excluded by!**/*.lock
📒 Files selected for processing (21)
apps/cli/src/commands/service/worker.tsapps/cli/src/handlers/controller/chat.tsapps/cli/src/utils/streams.tsapps/web/src/components/chat/main/thread-workspace.tsxapps/web/src/hooks/use-controller-threads.tsapps/web/src/stores/chat-ui.tsopenspec/changes/archive/2026-07-08-replace-cli-store-with-turso/.openspec.yamlopenspec/changes/archive/2026-07-08-replace-cli-store-with-turso/design.mdopenspec/changes/archive/2026-07-08-replace-cli-store-with-turso/proposal.mdopenspec/changes/archive/2026-07-08-replace-cli-store-with-turso/specs/acp-session-router/spec.mdopenspec/changes/archive/2026-07-08-replace-cli-store-with-turso/specs/conversation-persistence/spec.mdopenspec/changes/archive/2026-07-08-replace-cli-store-with-turso/tasks.mdopenspec/specs/acp-session-router/spec.mdopenspec/specs/conversation-persistence/spec.mdpackage.jsonshared/connections/src/schemas/rtc/chat.tsshared/connections/src/schemas/rtc/common.tsshared/database/src/repositories/threads.tsshared/database/src/utils/error.tsshared/hooks/src/derive-thread.tsturbo.json
💤 Files with no reviewable changes (1)
- openspec/changes/archive/2026-07-08-replace-cli-store-with-turso/tasks.md
✅ Files skipped from review due to trivial changes (1)
- turbo.json
🚧 Files skipped from review as they are similar to previous changes (6)
- shared/connections/src/schemas/rtc/common.ts
- package.json
- shared/database/src/utils/error.ts
- apps/cli/src/handlers/controller/chat.ts
- apps/cli/src/utils/streams.ts
- shared/database/src/repositories/threads.ts
Comment on lines
+64
to
+72
| ### Requirement: Persist-before-broadcast ordering | ||
|
|
||
| The worker SHALL persist a conversation entry and obtain its assigned `seq` before broadcasting the corresponding `ChatChunk` to connected peers. | ||
|
|
||
| #### Scenario: Broadcast carries a durable sequence | ||
|
|
||
| - **WHEN** an event is emitted during a chat turn | ||
| - **THEN** the entry is written to Turso first, and the `ChatChunk` broadcast to peers includes the `seq` returned by that write | ||
|
|
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Scope the ordering guarantee to persisted entries.
As written, this conflicts with the live-delta path in apps/cli/src/handlers/controller/chat.ts:29-72, which broadcasts streaming deltas immediately with seq: 0 and only persists the coalesced completion event first. The requirement should say persisted ConversationEntry rows are written before broadcast, not every ChatChunk.
Proposed wording
-The worker SHALL persist a conversation entry and obtain its assigned seq before broadcasting the corresponding ChatChunk to connected peers.
+For persisted conversation entries, the worker SHALL persist the entry and obtain its assigned seq before broadcasting the corresponding ChatChunk to connected peers.
+Streaming deltas may still be broadcast immediately with a placeholder seq.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### Requirement: Persist-before-broadcast ordering | |
| The worker SHALL persist a conversation entry and obtain its assigned `seq` before broadcasting the corresponding `ChatChunk` to connected peers. | |
| #### Scenario: Broadcast carries a durable sequence | |
| - **WHEN** an event is emitted during a chat turn | |
| - **THEN** the entry is written to Turso first, and the `ChatChunk` broadcast to peers includes the `seq` returned by that write | |
| ### Requirement: Persist-before-broadcast ordering | |
| For persisted conversation entries, the worker SHALL persist the entry and obtain its assigned `seq` before broadcasting the corresponding `ChatChunk` to connected peers. | |
| Streaming deltas may still be broadcast immediately with a placeholder `seq`. | |
| #### Scenario: Broadcast carries a durable sequence | |
| - **WHEN** an event is emitted during a chat turn | |
| - **THEN** the entry is written to Turso first, and the `ChatChunk` broadcast to peers includes the `seq` returned by that write |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@openspec/specs/conversation-persistence/spec.md` around lines 64 - 72, The
ordering requirement is too broad and conflicts with the live-delta flow in chat
handling; scope it to persisted ConversationEntry rows only. Update the spec
around the persist-before-broadcast behavior to state that only durable entries
must be written to Turso before their corresponding ChatChunk is broadcast,
while streaming deltas in chat controller paths like chat.ts may still broadcast
immediately with a placeholder seq and persist later on completion.
This was referenced Jul 9, 2026
This was referenced Jul 17, 2026
This branch was successfully deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
@cyrus/databasewith Turso/SQLite persistence for projects, threads, and conversations underCYRUS_HOME, replacing the CLI's in-memoryMapstoresseqon conversation entries, optionalafterSeqcursor reads, and persist-before-broadcast ordering in chatResultfrom repositories and map errors to ORPC in handlers; align wire schemas withoptionalStringfor nullable DB columnsCloses #13
Test plan
cyrusdworker, create a project and thread, send a multi-turn chatlistProjects,listThreads, andgetConversationsreturn prior datagetConversationswithafterSeqmid-conversation and confirm only later entries are returnedbun checkandbun check:typesMade with Cursor
Summary by CodeRabbit
New Features
Bug Fixes