OpenCode Terminal: deferred medium/low findings from the round-2 verification

Four independent verifiers reviewed the OpenCode Terminal headless work (PR #882 / opencode-terminal-headless#1) and returned RED. Every high finding was fixed before that PR merged. These are the remaining medium/low findings, each reproduced by a verifier with a real failing test, deferred deliberately so a large branch could land.

Full reports are in the branch's temp/review/verify/ (git-ignored, so the substance is reproduced here).

1. [medium] The exit flush skips the retry window it promises — and lies about it

Where: packages/opencode-terminal-headless/src/reconcile/SessionSequencer.ts (~179-190)

On exit the sequencer retries only when drainNow() returns deferred. If settleOpenWork() or flushPendingUsers() returns deferred instead — which happens when the database goes BUSY between those transactions — it closes immediately, reports {complete:false}, tears down the reader and cancels the owed flush.

Two consequences: the user can lose the last prompt of a session at exit, and the diagnostic claims the database stayed busy for 2,000 ms when no such wait occurred. A fabricated diagnostic is worse than none — it sends the next investigator down the wrong path.

Reproduction (V1): take a real SQLite EXCLUSIVE lock in the window between the exit drain's transaction and the pending-user flush's transaction; assert the lock trigger ran; exit has already fired. A scratch change returning to the retry loop for either deferred operation makes it pass, and delivers the pending prompt after unlock.

Fix: put all three exit operations under one bounded retry decision; report incomplete only at the real deadline; make the diagnostic describe what was actually waited on. Keep the bound — a stuck database must never hold a session open forever.

2. [medium] A fresh history row is inserted before the prefix it should follow

Where: src/renderer/src/workspace/hook/actions/initialHistory.ts (~127)

A pane holds [a,c]. A history chunk arrives as [b,c], timestamps a < b < c. The merger pushes fresh b immediately, then flushes existing entries through anchor c, producing [b,a,c]. UUID dedup then makes that order permanent.

The loop protects the existing entries' relative order but never considers the retained prefix when placing a fresh row at the first shared anchor.

Reproduction (V3): scratch/V3/history-boundaries.renderer.test.tsx, "inserts an unseen entry immediately before its next shared anchor" — fails with that exact array, from hand-authored timestamps (not generated by the merger under test).

Why it matters: this never errors. It just silently renders the conversation in the wrong order, and the user has no way to tell.

3. [medium] Re-hydrating the same disjoint tail defeats the cursor fix

Where: src/renderer/src/workspace/hook/actions/initialHistory.ts (~408, ~430) and actions/history.ts's prepend path

Start [c,d], cursor c, permanently failed durable channel. Hydrate [g,h] — correct, appends and keeps c. Page older [a,b]. An MCP read then hydrates the same [g,h] again: because this load adds no fresh entries, appendedAfterWindow: false is hardcoded, the cursor moves to g, and the next older page yields [e,f,a,b,c,d,g,h].

Fix: don't derive cursor ownership from whether this particular load added entries. Retain the real oldest-window marker on a no-op tail refresh, and choose a deliberate gap policy (contiguous replacement tail, or explicit ordered backfill).

Reproduction (V3): same scratch file, "can recover the gap between an existing window and a newer disjoint tail".

4. [low] The host never reaches the package's database-generation selection

Where: src/providers/opencode/runtime/opencodeDatabase.ts (~44-46)

openOpencodeStore grew a generation so that an opencode.db replaced at the same path (different inode) opens a new connection. The host never benefits: it resolves the path once and caches the facade for the process lifetime, so after a replacement it keeps reading the old file.

Reproduction (V2): two seeded databases — a directly acquired package store sees ses_new; the host still sees ses_old, and advancing the clock changes nothing.

Trigger is manual replacement/relocation, not ordinary upstream migration, hence low. Either add bounded realpath/device/inode revalidation (switching new readers to a fresh generation while borrowed readers finish safely), or state the limitation honestly in that file's ownership comment.

Fixed before merge, for the record

Prompts overwrote the session's selected agent and dropped its model variant (the server resolves an omitted agent to the configured default and persists it).
A POST the server had already received was reported as safe to retry, risking duplicate submission of a user's work.
Re-sync never reconciled the busy map behind session-switch detection (missed switches; false switches).
The session-switch warning was absent from the terminal pane itself.
Stale plan/decomposition documents that still instructed the reversed behavior.

🤖 Generated with Claude Code

Merge origin/main: hand the Resume listing over to the conversation c…

5. [low] Two readers now open `opencode.db`

Noted while merging #899 into #882, so it does not go unrecorded.

packages/opencode-terminal-headless's OpencodeStore and src/main/conversations/sources/opencode.ts both open OpenCode's SQLite database, each with its own connection handling, schema expectations and column list (OPENCODE_COLUMNS vs the package's REQUIRED_COLUMNS).

Both are currently correct and they serve different callers — the package feeds live panes, parked-pane history and MCP transcript reads; the catalog feeds the conversation pickers and the nativeHistory capabilities. But they will drift: an upstream schema change has to be noticed twice, and only one of them has a schema gate that fails closed.

Not urgent and not a merge blocker — nothing is wrong today. Worth a deliberate decision later: either point the catalog's OpenCode source at the package's store (one reader, one gate, one place to update when OpenCode changes), or write down in both files why two are wanted. The thing to avoid is a third.

Completing this record (from the final pre-merge review)

A final reviewer checked this issue against every earlier finding and found it incomplete. Two corrections first:

The opening line says "Every high finding was fixed before that PR merged". It was written before feat(opencode): OpenCode Terminal reports status, turns, conditions and history through opencode-terminal-headless #882 merged. The highs were fixed on the branch; read it as "before merge".
Item 5 below is expanded, and several deferred items were missing entirely.

6. [medium] End-to-end gaps with no test and no disposition (V4 N5)

Named in V4's report and never tracked anywhere:

five-pane recovery / restart;
park → read → wake;
the full provider switch / duplicate sequence for an OpenCode Terminal pane;
the phone transcript store after a reconnect;
the close / switch busy guard.

The only real-TUI test does not exercise the new prompt path. packages/opencode-terminal-headless/src/OpencodeTerminalHeadless.live.test.ts (~121-124) still waits a fixed 10 s and calls pasteAndSubmit, not submitPrompt. So nothing automated confirms #877 against a real TUI, which is why #877 stays open.

7. [low] Host tests import the replay harness by an undocumented path (V4 N4)

About ten host tests import opencode-terminal-headless/testing/index rather than the documented opencode-terminal-headless/testing (e.g. src/main/sessions/control.system.test.ts:5). They work today through the source alias. They would break if the alias ever mapped only the documented entry.

8. [low] Nothing pins the facts the remote-feed stub relies on

src/main/sessions/forwarder.test.ts stubs getSessionKind/getSpawnKind to return 'opencode', which pins that the remote feed's plain-terminal filter does not drop an OpenCode Terminal pane. But no test asserts that a real SessionManager terminal-runtime OpenCode spawn reports kind 'opencode' on started, getSessionKind and getSpawnKind.

Why it matters: if native-TUI panes were ever registered as kind 'terminal', the stubbed test would stay green while the phone silently lost the pane, and #911's eviction would drop its state. Correct by reading today (sessionManager.ts ~2687-2694). Fix: one spawn test asserting the kind on all three surfaces.

9. [low] No test drives a capability reload on an OpenCode Terminal pane

builtInMcpReload.test.ts and sessionCommands.renderer.test.ts cover Codex/Claude reloads. The OpenCode Terminal cases there cover only duplicate and rewind.

Why it matters: if replaceSession stopped inheriting providerRuntime for same-kind reloads (session.ts ~1203-1207), toggling a capability would silently turn a native-TUI pane into a structured one. Fix: one reloadSessionWithBuiltInMcpChoice case with providerRuntime: 'terminal'.

Item 5, expanded: the two `opencode.db` readers disagree on semantics, not just columns

packages/opencode-terminal-headless/src/transcript/OpencodeStore.ts (listSessions) vs src/main/conversations/sources/opencode.ts:

	package store	conversation catalog
Archived sessions	listed (no `time_archived` filter)	filtered out
Placeholder title `'Agent Code terminal session'` (written by `createEmptyOpencodeSession`)	returned raw	mapped to no title — the literal is duplicated with no shared source and no test tying the two
Directory match	exact, case-sensitive	lowercased, includes worktree prefixes
Task children	excluded (root only)	included and flagged `isNativeSubagent` — a deliberate consumer difference
Schema gate miss	fails closed with a typed error	`discover` returns `[]` with only a perf-span reason: an empty picker, not a diagnostic
Columns gated	`part.id`, event tables	`session.time_archived`, `part.session_id`, `part.time_created`

Nothing is wrong today, because listSessions has no consumer. But pointing the catalog at the package store naively would surface archived and placeholder-titled sessions in the picker. And an upstream column change would be caught by one reader while silently emptying the other. Any consolidation has to reconcile the rows above first.

From the final pre-merge review (two independent reviewers, both GREEN)

Both reviewers returned GREEN with no blocker or high, and #882 and opencode-terminal-headless#1 merged on that basis. These are the remaining findings. Each reviewer judged these non-blocking, and says why below. Full reports are in the branch's git-ignored temp/review/final/RA-report.md and RB-report.md; the substance is reproduced here.

10. [medium] Returning to the launch session leaves a permanent, contradictory error

This is the most user-visible item left.

Scenario:

A pane follows ses_A.
In the TUI the user opens another root session and prompts it. The warning appears, correctly.
The user goes back to ses_A and prompts it.
The pane now permanently shows "OpenCode switched to session ses_A inside the TUI. This pane still follows ses_A…".
transcriptStatus stays error through successful history reads.
managedTranscriptUnavailableReason returns transcript_unavailable (agentManagementMcp.ts ~130). An orchestrating parent cannot read a transcript that is actually correct and current.

It lasts until the pane is replaced.

Why: the package's projector reports a switch back to the bound session by design (LiveStateProjector.contract.test.ts ~253-259). The adapter always names the bound identity as from, and the renderer marks every (provider_session_switched) as lifetime with nothing to clear it. It predates the round-3 fixes, but the in-pane banner now makes it permanent chrome.

Reproduced through the real package → adapter → SessionManager → forwarder → renderer chain (scratch/RA/probes/RA-switchback.renderer.test.tsx).

Why not blocking: it fails closed (withholds, never serves wrong data), needs two navigations each with a prompt, and Resume clears it. It is still exactly the "banner that teaches users to ignore it", so it should not wait long.

Fix:

When to === this.providerSessionId, the adapter emits a distinct "returned to the bound session" diagnostic instead of an error.
The renderer clears transcriptChannelError, but only when the stored marker is a switch marker, never a stopped durable channel, and restores transcriptStatus from normal readiness.
Test the away-and-back case in opencodeTerminalSwitch.renderer.test.tsx: the banner and marker clear, and a durable-stop marker set earlier survives.

Belongs with #894.

11. [medium] With the durable store unavailable, prompts again move the session to the default agent

When the pane's durable channel is off for its lifetime, the prompt-selection callback returns all-null and the prompt goes out as { parts } alone. That happens on db_path_unavailable, open_failed (including opencode.db not existing yet on a machine's first OpenCode run), unsupported_schema after an upgrade past the gate, or sqlite_unavailable.

That is the original bug for that pane. A plan/high session is moved to build, and setAgentModel persists it. A transient BUSY exactly at readSessionInfo does the same for that one prompt.

Why not blocking: it needs an already-degraded, already-visible error state, and it is the pre-fix behavior, not a regression.

Fix: fall back to the live server, which prompting already requires. GET /session/:sessionID returns Session.Info, which carries agent and model (verified in the 1.18.30 binary). Read it only when the store cannot answer, with the same readSelection parsing and "default" normalization. Add a prompt test with dbPath: null and a replay server answering GET /session/:id with agent: 'plan'.

12. [low] The false comment that caused the original bug is still in the code

packages/opencode-terminal-headless/src/live/LiveServerClient.ts ~140-147 still says agent/model are "deliberately absent" because the server applies the same selection as the TUI. That is the exact claim the binary disproved, and it sits directly above the corrected block. It is harmless to run and a trap for the next edit. Delete it.

13. [low] Test gaps found by mutation

Re-sync busy reconciliation, add-side fence. reconcileBusy's add-side fence (LiveStateProjector.ts ~391-393) is untested: removing it survives all contract tests. Add the mirror of "keeps a status the stream restated while the snapshot was in flight", with the bus saying B went idle after since and the snapshot still listing B busy.
Re-sync busy reconciliation, real composition. No composition-level regression exists: with reconcileBusy removed, switch.system and resync.system still pass. Promote V1's refreshes background %s state at reconnect before deciding a switch cases into OpencodeTerminalHeadless.switch.system.test.ts unchanged.
Switch harness assertion. opencodeTerminalSwitch.renderer.test.tsx never asserts transcriptChannelError directly.
agents.prompt on a rejection. It reports unknown for a server rejection even though promptWritten: false proves nothing started. This is conservative and harmless.

14. [low] Stale comments and docs

src/shared/types/providerConfig.ts: the feat(conversations): rebuild the conversation picker on provider-native indexes #899 merge resolution left an orphaned duplicate of the deliverPrompt JSDoc above loadHistoryChunk.
The switch diagnostic tells users to resume from "the Resume picker". After feat(conversations): rebuild the conversation picker on provider-native indexes #899 the command is Resume Session…. The text is in opencodeTerminalSession.ts and is asserted by two tests (opencodeTerminalSwitch.renderer.test.tsx, DispatchTranscriptError.renderer.test.tsx).
OpencodeStore.ts and schema.ts describe listSessions as serving a Resume picker. It has no consumer, and session.time_created is required only by that uncalled method.
committed.ts says the typed-prompt predicate lives in latestUserPrompts.ts. Today it is each provider's isTypedUserPrompt capability, which that file routes through.
docs/decomposition/opencode-terminal-headless.md still states that no Agent Code code runs its own SQL against OpenCode's schema. That has been false since feat(conversations): rebuild the conversation picker on provider-native indexes #899.
The root README.md submodule list omits opencode-terminal-headless (and workflow-mcp).
The host plan's Task 3 names files and a cursor (beforeMessageID) that shipped differently. The package plan still says 294 tests; it is 299.

Every edit above is a comment, a doc, or that one message string (plus the two tests asserting it); none changes behavior.

fix(history): the cursor follows the merged head, and the prefix flus…

fix(opencode): count the holders of the shared store instead of guess…

fix(reconcile): report what is still OWED at the exit deadline, not w…

chore(deps): bump opencode-terminal-headless for the exit-drain retry…

All four items are merged.

item	PR	what shipped
1 — the exit flush skips its retry window and lies about it	opencode-terminal-headless#5 + #1082 (submodule bump)	all three exit transactions share one bounded window, each retried only while it is the one still deferring; the outcome names the step that was actually stuck and the time actually spent
2 — a fresh row inserted before the prefix it should follow	#1081	the retained prefix is flushed first, bounded by the anchor and by timestamps
3 — re-hydrating a disjoint tail defeats the cursor fix	#1081	the cursor follows `keptWindowHead`, observed from the merged result
4 — the host never reaches the package's generation selection	#1082	bounded 5 s revalidation, with the shared store's holders counted

Both reviews found something the PR had got wrong, and both are worth recording:

Item 1's first fix made its own target worse. The stuck list was rebuilt every attempt, so a flush that deferred on attempt 1 followed by a failed drain on attempt 2 reported {complete: true} with the prompt gone — where the pre-fix code reported it correctly. Each step now carries pending/owed/settled.
Item 1's "equivalent mutant" claim was false. OpencodeStore.read opens a DEFERRED transaction, so a statement-free settle never meets the lock, reports complete and latches itself off for the window. The thrown refuseReads stand-in cannot express that at all — only a real BEGIN EXCLUSIVE can.
Item 4's first fix released the superseded handle on a timer and argued promise continuations drain first. AgentTranscriptReader walks a session page by page with setImmediate between pages, and a timer callback runs in the TIMERS phase, ahead of CHECK. The facade counts lease holders now.
Item 2's fix recreated item 3 from the other side: the prefix flush changed which row lands at merged[0], and the cursor rule did not follow.

Closing.

docs(ledger): #1079 merged, #1080 review round, #910 items 1 and 4

docs(ledger): #910 closed, #1083 redesigned, three PRs opened

Navigation Menu

Repository navigation

OpenCode Terminal: deferred medium/low findings from the round-2 verification #910

1. [medium] The exit flush skips the retry window it promises — and lies about it

2. [medium] A fresh history row is inserted before the prefix it should follow

3. [medium] Re-hydrating the same disjoint tail defeats the cursor fix

4. [low] The host never reaches the package's database-generation selection

Fixed before merge, for the record

5. [low] Two readers now open `opencode.db`

Completing this record (from the final pre-merge review)

6. [medium] End-to-end gaps with no test and no disposition (V4 N5)

7. [low] Host tests import the replay harness by an undocumented path (V4 N4)

8. [low] Nothing pins the facts the remote-feed stub relies on

9. [low] No test drives a capability reload on an OpenCode Terminal pane

Item 5, expanded: the two `opencode.db` readers disagree on semantics, not just columns

From the final pre-merge review (two independent reviewers, both GREEN)

10. [medium] Returning to the launch session leaves a permanent, contradictory error

11. [medium] With the durable store unavailable, prompts again move the session to the default agent

12. [low] The false comment that caused the original bug is still in the code

13. [low] Test gaps found by mutation

14. [low] Stale comments and docs

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

OpenCode Terminal: deferred medium/low findings from the round-2 verification #910

Description

1. [medium] The exit flush skips the retry window it promises — and lies about it

2. [medium] A fresh history row is inserted before the prefix it should follow

3. [medium] Re-hydrating the same disjoint tail defeats the cursor fix

4. [low] The host never reaches the package's database-generation selection

Fixed before merge, for the record

Activity

Juliusolsson05 commented on Sep 12, 2026

5. [low] Two readers now open opencode.db

Juliusolsson05 commented on Sep 12, 2026

Completing this record (from the final pre-merge review)

6. [medium] End-to-end gaps with no test and no disposition (V4 N5)

7. [low] Host tests import the replay harness by an undocumented path (V4 N4)

8. [low] Nothing pins the facts the remote-feed stub relies on

9. [low] No test drives a capability reload on an OpenCode Terminal pane

Item 5, expanded: the two opencode.db readers disagree on semantics, not just columns

Juliusolsson05 commented on Sep 12, 2026

From the final pre-merge review (two independent reviewers, both GREEN)

10. [medium] Returning to the launch session leaves a permanent, contradictory error

11. [medium] With the durable store unavailable, prompts again move the session to the default agent

12. [low] The false comment that caused the original bug is still in the code

13. [low] Test gaps found by mutation

14. [low] Stale comments and docs

Juliusolsson05 commented on Sep 20, 2026

Metadata

Metadata

Assignees

Labels

Projects

Milestone

Relationships

Development

Issue actions

5. [low] Two readers now open `opencode.db`

Item 5, expanded: the two `opencode.db` readers disagree on semantics, not just columns