fix(session): a web session whose server stopped is ended, not reattachable - #47
Merged
Merged
Conversation
…or reattach A PTY/ACP session lives inside the `studyloop web` process that started it, and the state file records that process as `pid`. When the server stops without ending the session, the file keeps `mode=focus`. /session/start already treats such a claim as stale (claim_blocks_web_start: the crash-then-restart cell), but /api/session/state held no slot, fell back to the file verbatim and reported the session live with no reattach_url. The console's load-time adoption then called start() with transport "pty" and no URL, and _mountUnavailable told the learner: 'This session reports transport "pty", which this view cannot render' -- about the transport it renders with xterm. Seen live after restarting `studyloop web --lan`; the sessions DB still holds the dead session open (ended_at NULL since 23 Sep). RED, each for the stated reason: - /api/session/state reports a dead-pid pty session, and an acp one, as `focus` (2 tests); - the console says it "cannot render" pty and acp when only the URL is missing (2 tests). Guards that pass now and must keep passing: a live foreign server's session, this process's own in-flight reservation, and a CLI session (whose pid is not its liveness) are left alone; nothing is deleted (an unended session's parking lot is unflushed learner notes); an unrecognised transport still names what the view cannot render.
…chable `_get_full_state()` cleared a dead CLI (tmux) session but had no rule for a web-owned one, and read_session_state() drops tmux keys for pty/acp files, so a browser-terminal or ACP session whose server died kept reporting `mode=focus` forever. /api/session/state held no slot, served that file verbatim with no reattach_url, and the console adopted it into its "unavailable" fallback. - session_state.web_claim_is_orphaned(): the read-side twin of claim_blocks_web_start's crash-then-restart cell -- a pty/acp claim whose pid is neither this process nor alive. CLI sessions (pid is not their liveness), this process's own in-flight reservation, and pid-less claims are never judged orphaned. - _get_full_state(): an orphaned web claim is reported `mode=ended`, the verdict /session/start already reached. Nothing is deleted: an unended session never flushed its parking lot. Covers /api/session/state and the SSE dashboard stream, which share the reader. - live-agent-console.js: a known transport (pty/acp) with no connection URL now reads "The server did not return a connection for this session"; the "cannot render" sentence is kept for transports the view does not render. That sentence is what docs/troubleshooting.md already promised for this case; every caller passes a transport, so it was unreachable. - docs/troubleshooting.md: the stopped-server case. CHANGELOG Unreleased/Fixed.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
PID liveness can misclassify unreaped zombie processes, leaving stale sessions exposed.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
Fixes stale web-session recovery by marking orphaned PTY/ACP sessions as ended and improving unavailable-console messaging.
Changes:
- Detects orphaned web claims using server PID liveness.
- Preserves session state and learner notes.
- Adds regression tests and documentation updates.
| File | Description |
|---|---|
packages/studyloop/tests/test_session_slot_reconcile.py |
Tests orphaned-session behavior. |
packages/studyloop/tests/js/live-agent-console-unavailable.test.js |
Tests frontend fallback messages. |
packages/studyloop/src/studyloop/web/static/js/components/live-agent-console.js |
Clarifies missing connection errors. |
packages/studyloop/src/studyloop/web/routes/session/_ipc.py |
Reports orphaned claims as ended. |
packages/studyloop/src/studyloop/session_state.py |
Adds web-claim PID liveness detection. |
docs/troubleshooting.md |
Documents stopped-server behavior. |
CHANGELOG.md |
Records the fix. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| def test_a_cli_session_is_left_alone(self, client: TestClient) -> None: | ||
| """A CLI session's pid is the CLI process, which can exit while its | ||
| multiplexer session lives on; the zombie rule judges CLI sessions.""" |
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
After
studyloop webrestarted, going back to the Study view showed This session reports transport "pty", which this view cannot render. That is the one transport the view renders with xterm.A browser-terminal or ACP session runs inside the server process that started it, and the state file records that process as
pid. When the server stopped without ending the session, the file keptmode=focus./session/startalready treats such a claim as stale (claim_blocks_web_start, the crash-then-restart cell)./api/session/state, holding no slot, served the file verbatim with noreattach_url. The console's load-time adoption then fell into its "unavailable" fallback, which named the transport as the problem.Changes
session_state.web_claim_is_orphaned(): a pty/acp claim whosepidis neither this process nor alive. It never judges CLI sessions (their pid is not their liveness), this process's own in-flight reservation, or claims with no pid._get_full_state()reports an orphaned web claim asmode=endedand deletes nothing. An unended session's parking lot holds unflushed learner notes. This covers/api/session/stateand the SSE stream.live-agent-console.js: a known transport with no connection URL now reads The server did not return a connection for this session. The "cannot render" sentence stays for unrecognised transports. That is whatdocs/troubleshooting.mdalready described; the sentence was unreachable because every caller passes a transport.RED
4953b52f(4 failing for the stated reasons, 6 guards passing) -> GREENc0812286.Verification
Not in this PR
study_sessionsrow stays open (ended_atNULL). The endpoint reports it ended but does not write.invalid agent config: kiro_default.jsonline in kiro-cli's TUI is unrelated to StudyLoop. kiro-cli 2.24.0 refuses a user config named after its reserved built-in agent and falls back to the built-in.