fix(sidebar): OpenCode rail clutter and '/' project labels - #617
Merged
Conversation
… validation findings Validated 13 load-bearing assumptions (9 verified, 4 falsified) and fixed the plan: - A1: OpencodeProvider ctor takes the opencode DATA home, not user home — Task 1 fixture passes the data dir; Task 3 exports resolveOpencodeDatabasePath(dataHome = defaultOpencodeDataHome()) + default pin test - A2: sessionRef-bearing live terminals skip the manufactured block — Task 8 classifies tab/pane-derived fallback rows via runningSessionMap; Task 9 gains a paned e2e variant - A3/A12: signal rebind lane and REST/fresh-agent binder attach child ids post-create — Task 6 gains hook sites #3 (rebind_fanout + 0b) and #4 (LedgerPaneIdentityBinder), writes the flag both directions; Node bindSession re-classifies (Task 4) - A9 hedge: declare the optional field in shared/ws-protocol.ts + contract:generate so the frozen WS schema admits it Evidence ledger: .the-usual-logs/sidebar-opencode-rail-fixes/load-bearing-ledger.md
…ication, locator data_home field, ws fixture widening, main.rs locator ordering Independent review (iteration 1) found 4 blocking majors in the plan: - Task 3/4: Node subagent classification ran sync node:sqlite DatabaseSync on the main thread in the terminal.create WS path; rewritten to reuse the existing off-thread by-id worker triad (additive parent_id projection + thin wrapper over runOpencodeSessionByIdOffThread, errors swallowed at the call site). - Task 5: OpencodeLocator has no data-home field; plan now instructs adding a private data_home: PathBuf retained by both constructors (signatures unchanged) before the classify_resume_target helper. - Task 6 Step 3: ws opencode_association insert_session fixture is 4-arg with hardcoded NULL parent_id; plan now instructs widening it to the 6-arg opencode_locator.rs twin signature and updating its 3 call sites. - Task 6 Step 6.3: main.rs binder construction precedes opencode_locator creation (:433) — plan now instructs the dependency-free move of the locator block above the fresh_agent_state chain and Option-mapped classifier wiring.
…sting + by-id (Bug 2)
…ification on Node (Bug 1)
…ecord and /api/terminals (Bug 1)
…der resolution (Bug 1)
…r helper on Rust (Bug 1)
…solution (Bug 1) Two rapid resume-target changes for the same terminal (e.g. create targeting child A, then a signal rebind to root B) spawn two independent classification tasks; the OLD target's answer could land AFTER the NEW target's, leaving a stale wrong is_subagent flag until the next rebind. classify_and_mark_resume_target now captures a per-terminal generation synchronously (before spawning) via TerminalIdentityRegistry::begin_subagent_classification, and the spawned task writes (and pings terminals.changed) only while its generation is still the terminal's latest (complete_subagent_classification) — newest-request-wins regardless of resolution order. The compare-and-write is atomic under the registry lock. A write-time compare against identity.session_id would not work: the WS create-path hook fires before the identity seed, so the entry may not carry the session id yet — pinned by a dedicated test. The counter rides the existing identity entry (retained-on-retire by design), so no new unbounded growth. Mirrors the Node-side guard in server/terminal-registry.ts. The REST binder path (pane_identity_binder.rs) is synchronous/inline and deliberately unguarded; hook call sites unchanged. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…s and live session items (Bug 1)
…s hides them (Bug 1)
…t terminal hiding (both servers)
…sume targets (Bug 1)
The Node server's buildLiveTerminalSessionItem fabricated a session item
for every running terminal but never set isSubagent, so:
(a) the server-side default-visibility filter (service.ts,
!item.isSubagent) could not drop it, and
(b) on the client the projects-loop item claimed the session key first,
so the terminal-derived classification never applied.
Net: a terminal resuming an opencode SUBAGENT (child) session still grew
a sidebar rail row under default visibility on Node. The Rust server got
exactly this projection in build_live_terminal_session_item (238f16b);
this is the Node counterpart.
- TerminalMeta/TerminalSeedRecord: additive resumeTargetIsSubagent field,
threaded through seedFromTerminal -> upsert (registry records already
carry it via registry.list() and terminal.created), and compared in
terminalMetaEquals so re-seeds with a changed flag propagate.
- buildLiveTerminalSessionItem: project isSubagent: true (omitted for
root targets, matching the wire type's optional-boolean convention).
Client untouched: the server-set flag flows through the existing wire
field and existing client consumption, same as the Rust side.
🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)
Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…r Node fix The chromium (Node) leg was deliberately RED pending the Node counterpart of the Rust live-terminal isSubagent projection; that fix landed (buildLiveTerminalSessionItem now sets isSubagent from TerminalMeta.resumeTargetIsSubagent), and the spec passes on both chromium and rust-chromium. No assertions changed. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ug 1) The terminal.create path awaited isOpencodeSubagentSession() unbounded, inheriting the by-id worker runner's 15s outer timeout — a wedged worker could park terminal creation for up to 15s, violating the branch's "classification never blocks terminal creation" principle. Add raceWithDeadline() + isOpencodeSubagentSessionWithDeadline() (1s cap) and use it at the create call site. On deadline the target stays UNCLASSIFIED (undefined) — the bindSession re-classification lane self-corrects later. The timer is cleared/unref'd so it never holds the process; the classification promise never rejects by contract, so a late race loser is safely ignored. Fast path behavior is identical (covered by tmp-DB tests). 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…test, doc breadcrumbs - crates/freshell-sessions opencode.rs: session_is_subagent_by_id now reuses OPENCODE_BYID_BUSY_TIMEOUT_MS instead of a hardcoded 500ms duplicate. - terminal-registry: add regression test for the bindSession re-classification out-of-order guard (a slow first classification resolving after a second rebind must not overwrite the new target's answer). Lives in its own test file because vi.mock of opencode-subagent-query is file-wide and would weaken the existing real-behavior tests. - terminal-metadata-service: freshness breadcrumbs on resumeTargetIsSubagent (create-time-frozen; the ?? merge never unsets; the flag must travel with any future sessionId rebind lane). - sidebarSelectors.runningTerminal.test: cite the `if (terminal.sessionRef) continue` guard by construct instead of a drifted line number. 🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier) Co-Authored-By: Amplifier <240397093+microsoft-amplifier@users.noreply.github.com>
…ebind lane Fresh-eyes review found TerminalMetadataService's copy of resumeTargetIsSubagent was frozen at create time: the live opencode rebind lane (promoteAssociation -> registry.bindSession -> 'associated' -> associateSession) overwrites meta.sessionId while spreading the stale flag forward, and bindSession's async re-classification updated only the registry record. Child->root switches left the flag true forever (root's fabricated live session-directory item hidden); root->child switches left it unset (subagent item visible - Bug 1 persisted via Node's session-directory pathway). Mirror the Rust rebind-hook pattern on Node: - terminal-registry: emit new 'terminal.subagent.classified' registry event when the request-order-guarded re-classification lands - registry-events: add the event type - index.ts: route the event to the metadata service and broadcast the changed meta via the standard terminal-meta upsert fanout - terminal-metadata-service: new setResumeTargetIsSubagent() that writes explicit false (bypassing the ?? merge) under a provider+sessionId staleness guard; rewrite the misleading 'future opencode rebind lane' comments - the lane is live Tests: 5 new terminal-metadata-service unit tests; new terminal-registry.rebind-metadata-resync.test.ts pinning both directions (Node mirror of Rust tui_switch_signal_reclassifies_is_subagent_in_both_directions) plus the slow-stale-classification guard. Focused + neighbor suites green (21 + 416).
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
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
Fixes two OpenCode-related sidebar (left rail) bugs, in both the Node and Rust servers:
Bug 1 — Subagent terminals cluttered the rail: OpenCode child/helper sessions (parent_id set in opencode.db) are deliberately never bound to a live terminal's sessionRef; the client then manufactured permanent rail entries titled with the raw launch command (e.g.
opencode --session ses_...). Fix: servers now classify live terminals targeting subagent sessions (async DB lookup off the main thread, 1s deadline cap, re-synced on session rebind in both directions) and the client routes these entries through the existing "show subagents" visibility toggle. The by-design refusal to bind terminals to subagent rows is preserved.Bug 2 — Sessions displayed project "/": OpenCode's catch-all
globalproject has worktree='/'; both servers used it verbatim ('/' is truthy so the cwd-derived fallback never ran), and the client rendered literal '/'. Fix: both servers treat '/' as a non-informative placeholder and fall back to deriving the project path from the session's real cwd.Quality
Generated with Amplifier