fix(web): keep synthetic session keys verbatim so marks survive persistence - #353
Closed
onychen wants to merge 1 commit into
Closed
fix(web): keep synthetic session keys verbatim so marks survive persistence#353onychen wants to merge 1 commit into
onychen wants to merge 1 commit into
Conversation
…stence removeWorkspace and archiveSession resolved the synthetic current:<sessionId> key against the process cwd before persisting it. The polluted key matched the projection only while both sides resolved identically in the same process; once the session gained a real file the mark was lost forever, making the active session disappear from the sidebar after deleting its workspace and unrecoverable via /api/sessions/select. Keep synthetic keys verbatim on write, load, and lookup via a shared sessionKey() helper, and match persisted marks by either the resolved file path or the synthetic current:<id> key so marks recorded before persistence keep applying after it. Legacy polluted entries stay inert. Update the two assertions that locked the resolved form and add regression tests covering both the ungrouped and archive flows across the unpersisted-to-persisted transition. Fixes openpi-dev#351
5 tasks
Contributor
Author
|
改用中文描述重新提交,新 PR 见后续评论。 |
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.
Problem
Fixes #351.
When the current workspace is removed while its active session has not yet persisted (no session file yet),
removeWorkspacekeys the session intoungroupedSessionswith the syntheticcurrent:<sessionId>marker — but passes it throughresolve()first, persisting<cwd>/current:<uuid>. The polluted key only matches the projection while both sides resolve identically inside the same process. Once the session gains a real file (first message), the mark is lost forever: the session disappears from every sidebar group, and/api/sessions/selectis rejected byrequireSessionwith "Workspace is not available".archiveSessionapplies the same pattern and losesarchivedthe same way.Value
<cwd>\current:<uuid>entries accumulating inworkspace-state.json/archived-sessions.json.Approach
Follows the minimal fix proposed in #351:
current:<sessionId>keys now round-trip verbatim: a sharedsessionKey()helper (withisSyntheticSessionKey) is used on write (removeWorkspace,archiveSession) and on load (ensureWorkspaceStateLoaded,ensureArchivesLoaded); real paths keep being resolved as before.hasSessionMark()helper that matches a mark by either the resolved file path (marks recorded after persistence) or the syntheticcurrent:<id>key (marks recorded before it), so marks survive the session gaining a real file without any migration.<cwd>\current:<uuid>) resolve idempotently and simply never match anything — inert, left in place per bug(web): removing the current workspace hides its unpersisted active session instead of moving it to Ungrouped #351.Validation
npm run format:check,npm run lint,npm run typecheck— pass.node --test --experimental-strip-types tests/web/pi-adapter.test.ts— 15/15 pass, including 2 new regression tests that reproduce the full unpersisted → persisted transition for both the ungrouped and archive flows.tests/webpure-logic suites (protocol, http-dispatcher, observer-registry, terminal-status, web-host-lease): 38 pass / 0 fail / 2 platform skips;app-render.test.ts: 21/21.node scripts/run-tests.mjson Windows hits the pre-existing background-terminal kill-test flakes/hang tracked in test: eliminate Windows background-terminal process-test flakes in the full suite #304 (process-management domain, unrelated to this change); not fully runnable on this machine.Impact
current:<id>; legacy polluted entries remain readable and inert (no migration, per bug(web): removing the current workspace hides its unpersisted active session instead of moving it to Ungrouped #351). No schema change.