Repository navigation
Conversation
|
The following comment was made by an LLM, it may be inaccurate: |
![opencode-agent[bot]](https://avatars.githubusercontent.com/in/1549082?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for working on this. The new test passes, so foreign events no longer allocate store state, but the filtering also drops events the TUI depends on, so this needs changes before it can land.
How I checked: ran solid-data.test.ts (31 pass) and the typecheck scripts in packages/client and packages/tui (clean). I also wrote a scratch test against createData on the base and on this branch: a followed parent session spawns a subagent child, then the child asks for permission and starts executing. On the base, the child joins the parent's family, its permission is stored and it shows as running. With this PR, all of that is dropped. The child's session.created is skipped, so resolveRoot can never link it to the followed parent. In the TUI, subagent permission and form prompts would not appear while you watch the parent session, so the turn can hang. I didn't drive the TUI end to end.
Public API: adds an optional isFollowed?: (sessionID: string) => boolean to CreateDataInput in @opencode/client/solid. This is backwards compatible: omitting it keeps the current behaviour.
Main concerns, details inline:
- Subagent children of the followed session are never followed (blocking).
- "Followed" means only the session on screen, but the TUI shows state for other sessions too: background-tab busy/attention/unread, the open menu's "awaiting input" and running indicators (#53435), and the session list. Execution, permission and form events for those sessions are now dropped.
- Updates to session info that is already loaded (rename, agent/model, moved, revert) are dropped, so titles and other details go stale. Sessions created by other clients are no longer synced into the list. Clearing events (
permission.replied,form.replied/cancelled) are dropped too, so a stale prompt can remain after the session becomes followed later.
A safer direction might be to skip only allocation for sessions the store doesn't know yet, and keep applying events for sessions it already holds or that belong to a known family. Alternatively, rely on server-side interest scoping (#36443).
| let activeUpdates: Map<string, DataSessionStatus | undefined> | undefined | ||
| const pendingUpdates = new Map<string, Map<string, SessionInboxInfo | SessionInbox.Delivery | undefined>>() | ||
|
|
||
| function isFollowed(sessionID: string): boolean { |
There was a problem hiding this comment.
This decides whether to follow a session from config.isFollowed, from resolveRoot and from loaded transcripts. A new subagent child has no info loaded yet, so resolveRoot(child) === child, and its session.created is now skipped (line 688), so its info is never loaded. Every later event for the child is dropped, including permission.asked. I reproduced this with a test against createData: on this branch the child's permission and running status are lost, while on the base they are stored. In the TUI, a subagent's permission prompt wouldn't appear while the parent session is on screen.
| ) | ||
| return | ||
| } | ||
| case "session.renamed": { |
There was a problem hiding this comment.
This guard (and those on agent.selected, model.selected, moved and revert.*) drops updates even when store.session.info[sessionID] is already loaded. Sessions shown in tabs, the session list or the open menu then keep stale titles and other details. These handlers already do nothing for unknown sessions, so they don't need the follow check.
| setStore("project", "info", event.data.id, reconcile(event.data)) | ||
| return | ||
| case "session.created": | ||
| case "session.created": { |
There was a problem hiding this comment.
Skipping the sync for sessions this client didn't create also stops new sessions from other clients appearing in the session list, which reads data.session from the store. And it is what cuts subagent children off from their followed parent.
| @@ -1203,17 +1242,20 @@ | |||
| if (event.data.inputID) compacting.get(event.data.sessionID)?.observed.add(event.data.inputID) | |||
| return | |||
| case "permission.asked": | |||
There was a problem hiding this comment.
session-tabs.tsx (attention), dialog-open.tsx (awaiting input, #53435) and the session list (running indicator) read permissions, forms and status for sessions other than the one on screen. With the TUI predicate, these events are dropped for any session not opened in this process. The matching permission.replied/form.replied guards also mean a prompt resolved elsewhere stays in the store if it was loaded earlier through a permission.sync.
| event: client.event, | ||
| connection: client.connection, | ||
| directory: props.directory, | ||
| isFollowed: (sessionID) => (route ? route.data.type === "session" && route.data.sessionID === sessionID : true), |
There was a problem hiding this comment.
This treats only the session currently on screen as followed. Its subagents, background tabs and sessions listed in the open menu or session list are not covered. The predicate needs to include at least the route session's family and the open tabs, or the filtering in createData needs to be limited to allocation for sessions it doesn't know yet.
53dba41 to
75df496
Compare
|
Updated and pushed commit 75df496 to address the review feedback:
|
Issue for this PR
Closes #37792
Type of change
What does this PR do?
The process-global SSE event stream (
/api/event) broadcasts live events across all sessions and workspaces connected to the server.In
packages/client/src/solid/data.ts, incoming session events were unconditionally projecting into the reactive store. Even for sessions the client never opened, callingmessage.updateallocated empty message arrays instore.session.messageand persistent entries inmessageIndex. Additionally, foreignsession.created,session.execution.*, and foreign location updates triggered redundant background HTTP sync calls (result.session.sync,result.location.*.sync), resulting in unnecessary CPU and memory allocation in multi-session setups.This PR:
isFollowed?: (sessionID: string) => booleanpredicate toCreateDataInputin@opencode/client/solid.isFollowed(sessionID)guard increateDatathat checksconfig.isFollowed, family root resolution (resolveRoot), and existing loaded transcripts instore.session.message. Ifconfig.isFollowedis omitted, it defaults to accepting all sessions for backwards compatibility.isFollowedinpackages/tui/src/context/data.tsxusinguseRoute()to check against the active session.packages/client/test/solid-data.test.tsverifying foreign events do not create store allocations or trigger sync.How did you verify your code works?
ignores foreign session events until the session is followedinpackages/client/test/solid-data.test.ts.bun testacrosspackages/client(226 tests passed, 0 failures).tsgo --noEmitinpackages/clientandtsgo -binpackages/tui(both clean, 0 errors).Checklist
Added by the opencode bot (AI-generated).
Related: #36443, #36441, #53203
Public API: adds optional
isFollowedtocreateDatainput in @opencode/client/solid (backwards compatible)