Repository navigation
fix(tui): keep session tab busy while background shell runs - #63676
aniruddhaadak80 wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Copilot was unable to review this pull request because the user who requested the review has insufficient remaining quota. Try requesting a review at a lower effort level.
An organization owner can choose the repository organization as the billing source for reviews requested by members with a paid Copilot license. Learn about Copilot code review billing.
![opencodejr[bot]](https://avatars.githubusercontent.com/in/5497873?s=60&v=4){kind=small}
There was a problem hiding this comment.
The fix works. I reproduced the bug on the base by running !sleep 300 in the TUI with tabs on: the shell row and the 1 shell footer spin but the tab doesn't. With this change the tab shows the spinner (screenshots below). The session-tabs tests pass, the new test fails on the base as it should, and tsc -b is clean in packages/tui.
Public API: nothing changes in shape. The plugin API's tabs.list() now reports busy: true for sessions with running shells, and the terminal program status reports them as working.
One design question for maintainers (see the inline comment): every running shell owned by the session now makes the tab busy. That includes shell({ background: true }) with no timeout, which is how dev servers are started, and user ! shells. A session with a dev server would look busy, both in the tab and in the terminal title, for as long as the server runs. The open #60500 deliberately leaves explicit background shells out of "busy" for this reason, so these two pull requests need to agree on one rule.
This only covers the tab strip and terminal status. The /sessions list still uses its own running check in component/dialog-session-list.tsx, so #51810 stays open.
Related: #51810, #48482, #56660
| data.session.status(id) === "running" || | ||
| data.session.pending.list(id).some((item) => item.type !== "synthetic"), | ||
| data.session.pending.list(id).some((item) => item.type !== "synthetic") || | ||
| data.shell.listBySession(id).some((shell) => shell.status === "running"), |
There was a problem hiding this comment.
This counts every running shell owned by the session, including shell({ background: true }) (no timeout, typically a dev server or watcher) and user ! shells (SessionShell.start, timeout: 0). Because createProgramStatus reads the same status(), a session with a dev server would show a spinning tab and a working terminal status for as long as the server lives. The open #60500 deliberately keeps explicit background shells out of busy for this reason and counts only auto-backgrounded work that will report back. Should this match that rule, e.g. skip shells started with background: true? Otherwise maintainers need to choose which behaviour they want.
Issue for this PR
Closes #62505
Type of change
What does this PR do?
A session tab showed no spinner while a background shell in that session was still running, even though the shell row in the transcript kept spinning.
SessionTabsProvider.status().busyonly looked at the session'sactivestate and its non-synthetic pending items. It never looked atdata.shell, so once the turn went idle after a shell was backgrounded, the tab indicator took the non-runs()path and showed nothing.ShellDisplaykept spinning because it checksdata.shell.get(id)directly.The fix adds the running shells of the session family to the busy check:
Shell entries are removed from the store on
shell.exited/shell.deleted, solistBySessionreflects only the shells that are still running. The samestatus()feedscreateProgramStatus, so the terminal title / program status is corrected by the same change.How did you verify your code works?
running background shell keeps its tab busyinpackages/tui/test/context/session-tabs.test.ts: it emitsshell.createdwithstatus: "running"for a background tab and assertsbusyflips to true, then emitsshell.deletedand asserts it flips back.bun test test/context/session-tabs.test.ts -t "running background shell"→ 1 pass, 0 fail.tsc -b packages/tui→ clean.Screenshots / recordings
Not a UI change in the visual sense; the tab strip animation is the visible effect.
Checklist
Added by the opencode bot (AI-generated).
Related: #51810, #48482, #56660