Conversations opencode backward compatible update

[aappddeevv]

Small fix to update to opencode 1.2.0 that uses a sqlite database to store session information.

[aappddeevv]

Hmm....this picked up my other changes. I need to rebase properly.

[aappddeevv]

#172

[marcus]

Hey @aappddeevv! Starling here. 👋

Nice quick fix — opencode 1.2.0's move to SQLite for session storage is a breaking change that's caught a few people (see #172), and this backward-compatible update handles the transition cleanly. Flagging for @marcus to review. ✦

[marcus]

Code Review: Conversations opencode backward compatible update

Clean, well-structured migration to opencode 1.2.0's SQLite storage. The backward-compatibility design is solid. Some specific feedback:

Backward Compatibility Design ✅

The fallback pattern is consistent throughout:

if a.useSQLite() {
    result, err := a.thingSQLite(...)
    if err == nil && len(result) > 0 {
        return result, nil
    }
    // Fall through to JSON path
}
return a.thingJSON(...)

This is a good approach — SQLite-first, JSON fallback. One concern: the len(result) > 0 condition in Sessions and Messages means that if a project legitimately has zero sessions in SQLite (e.g., new project, just created), it'll fall through to the JSON path and potentially return stale data. Consider returning early on err == nil alone (checking only for error, not empty result) for Detect at minimum, since Detect uses a different condition already.

DB Connection Management

getDB() lazily opens and caches the connection with a ping-on-reuse pattern:

if a.db != nil {
    ctx, cancel := context.WithTimeout(context.Background(), queryTimeout)
    err := a.db.PingContext(ctx)
    cancel()
    if err == nil {
        return a.db, nil
    }
    _ = a.db.Close()
    a.db = nil
}

Good defensive pattern. A couple of observations:

• db.SetMaxOpenConns(1) with SQLite in read-only mode (mode=ro) is appropriate — SQLite handles concurrent reads fine but database/sql connection pool overhead for a single reader is minimal anyway.
• The connection string a.dbPath + "?mode=ro&_journal_mode=WAL" — setting _journal_mode=WAL on a read-only connection is a no-op (you can't change journal mode as a reader). Remove it to avoid confusion. The WAL mode is set by the writer (opencode itself); sidecar just needs to read it.
• No db.Close() is called when the adapter is torn down. Since the adapter presumably lives for the process lifetime this is fine, but it's worth a comment or a Close() method for completeness.

Path Discovery

openCodeDBCandidates mirrors the structure of openCodeStorageCandidates and handles macOS, Linux (XDG), and Windows correctly. The deduplication at the end:

if len(candidates) == 0 || candidates[len(candidates)-1] != defaultPath {
    candidates = append(candidates, defaultPath)
}

works correctly for the non-Linux case where the default doesn't duplicate the last entry. ✓

normalizePath

func normalizePath(path string) string {
    if abs, err := filepath.Abs(path); err == nil { path = abs }
    if resolved, err := filepath.EvalSymlinks(path); err == nil { path = resolved }
    return filepath.Clean(path)
}

Good for worktree matching. EvalSymlinks requires the path to exist — if the stored worktree path in SQLite is a deleted worktree (removed after session was created), EvalSymlinks will fail and fall back to the Abs result. That's the right behavior. ✓

messagesSQLite — Two-Query Approach

Fetching messages then parts separately and joining in Go is correct given SQLite's read-only mode:

• Messages query ordered by time_created ASC, id ASC — good stable ordering
• Parts query ordered by message_id ASC, id ASC — consistent ordering ✓

One issue: the parts query fetches all parts for the session by session_id, which is correct, but if sessionID is empty string or untrusted input, it would fetch all parts. Since sessionID comes from the adapter's own session index it's fine, but a guard at the top (if sessionID == "") would be defensive.

appendParsedPart handles text, tool, file, patch types — are there other part types in opencode 1.2.0's schema worth handling? A default/logging path for unrecognized types would aid debugging when opencode adds new part types.

DBWatcher

The directory-level watch (watcher.Add(dbDir)) combined with per-event filtering on dbPath and dbPath + "-wal" is the right approach for SQLite — WAL writes happen to the -wal file, not the main db file, during normal operation.

The debounce (100ms) is reasonable for UI responsiveness. One subtle issue:

op := event.Op
debounceTimer = time.AfterFunc(debounceDelay, func() {
    ...
    switch {
    case op&fsnotify.Create != 0:

The op variable is captured correctly (by value, not reference) since it's assigned before the closure. ✓

However, if multiple events fire within the debounce window, op is from the last event that restarted the timer — intermediate events are dropped. This is fine for WAL-based updates (any write is a "something changed" signal), but EventSessionCreated vs EventSessionUpdated distinction may be lost. Since sidecar likely re-reads all sessions on any event, this is probably acceptable. Consider collapsing to a single EventSessionUpdated for simplicity.

Test Coverage ✅

TestSQLiteStorageMode_DetectSessionsMessages covers the happy path thoroughly — schema creation, project/session/message/part insertion, and assertion of parsed output including token counts and tool uses. Good.

TestWatchScope_WithSQLiteDB is concise and correct.

TestWatch_WithSQLiteDB writes the WAL file to trigger a watcher event and waits 2 seconds — pragmatic approach. The test could be flaky under heavy system load since file events on some platforms (Linux inotify) can be delayed. 500ms timeout + retry would be more robust, but 2s is probably fine in practice.

Missing test: the fallback path — a project that has a SQLite DB but the session exists only in JSON storage (migration in progress). Worth a test that creates a DB file with no matching project and verifies JSON fallback returns data.

Summary

1. _journal_mode=WAL in read-only connection string — remove, it's a no-op and misleading
2. Empty-result fallback condition — consider err == nil instead of err == nil && len > 0 for Sessions/Messages to handle legitimately empty projects
3. No Close() on adapter teardown — minor, worth a comment or method
4. Add fallback path test — JSON fallback when SQLite exists but has no matching project

Overall this is solid work. The two-query approach for messages+parts is clean, the path discovery is thorough, and the backward-compatibility pattern is well-designed. The watcher implementation correctly targets WAL-file events.

[aappddeevv]

Updated the PR based on the code review commentary. Rebased.

[marcus]

Hey @aappddeevv! Starling here (AI assistant on the project). 👋

Thanks for the quick turnaround on the review feedback — flagging @marcus that this is rebased and ready for another look. 🪶

[marcus]

Thanks for the solid work on the SQLite adapter — really clean implementation overall!

I pushed a small follow-up commit (2fde79e) with three tidy-ups from the review:

1. rows.Err() checks — added after each for rows.Next() loop in findProjectIDSQLite, sessionsSQLite, messagesSQLite (both msg and part row sets), and discoverRelatedProjectDirsSQLite. Matches the pattern in the kiro/warp adapters.

2. appendParsedPart text accumulation — replaced the awkward strings.TrimSpace(strings.Join(...)) with a simple conditional append, consistent with how batchLoadAllParts handles text parts.

3. useSQLite() caching — wrapped the os.Stat in a sync.Once so it only runs once per adapter lifetime. Updated the related test to reflect that the check is now cached at first call rather than re-evaluated on every method.

Build passes, all tests green. Can you verify the changes look good on your end?