Repository navigation
fix(opencode): clear session status before publishing status events - #14225
Merged
Merged
Conversation
Reload refused to run when a stopped session was still marked busy. The session runner removed its entry, then published idle notifications, and only then cleared the busy status. A failing event listener aborted the clear, so the completed session stayed busy in the project status store and every later reload returned 409. Apply status map mutations first, then publish. Listeners now observe the settled status, and a listener failure can no longer block reload for a session that already stopped.
![kilo-code-bot[bot]](https://avatars.githubusercontent.com/in/2193792?s=60&v=4){kind=small}
Contributor
Code Review SummaryStatus: No Issues Found | Recommendation: Merge Files Reviewed (2 files)
Previous Review Summary (commit 88d2315)Current summary above is authoritative. Previous snapshots are kept for context only. Previous review (commit 88d2315)Status: 1 Issue Found | Recommendation: Address before merge Overview
Issue Details (click to expand)WARNING
Files Reviewed (3 files)
Reviewed by deepseek-v4.1-flash · Input: 0 · Output: 0 · Cached: 0 Review guidance: REVIEW.md from base branch |
Moving the non-idle publication after the map writes created a new permanent-busy path. A failing status listener left the entry in data, stores, and byDirectory before SessionRunState.startShell forked its shell. startShell awaits onBusy first, so no shell ran, no onIdle fired, and the project store kept the session busy until a backend restart. Publish the status event before mutating the map on the busy path, as the original code did. The idle path still clears first, which is what keeps a stopped session from blocking reload. Add a busy-path regression test with a failing listener.
marius-kilocode
enabled auto-merge
September 17, 2026 09:23
WebReflection
approved these changes
Sep 17, 2026
This was referenced Sep 18, 2026
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
2 participants
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.
What Problem This Solves
/reloadcan report "Cannot reload while a session is running in this project" when no session is running. The block persists until the backend restarts.Why This Change Was Made
Session status cleanup ran after event publication:
SessionStatus.set(id, { type: "idle" }).session.statusandsession.idle.Event listeners run inline without failure isolation. If a listener fails during step 2, step 3 never runs. The runner is gone, but the project status store still lists the session as busy, and
SessionStatus.listAll()keeps returning that entry to the reload guard added in #12454.The fix applies all status map mutations before publishing. Listeners now observe the settled status, and a failing listener can no longer leave a stopped session busy.
User Impact
Reload works after a session stops even when a status listener fails. A session that is still running blocks reload as before.
Evidence
packages/opencode/test/kilocode/session-status.test.tscover successful idle publication and deliberate listener failures on bothsession.statusandsession.idle. Each asserts thatlist()andlistAll()no longer contain the busy entry, both during and after publication.bun test test/session/status.test.ts test/kilocode/session-status.test.ts test/kilocode/instance-reload.test.ts test/kilocode/instance-store-project-reload.test.ts: 17 passed.bun run typecheckinpackages/opencode: passed.bun run compileinpackages/kilo-vscode: passed.Limitation: the reported multi-project incident was not reproduced on an unpatched build. This addresses a confirmed stale-status failure path rather than proving the original report's exact trigger.