Skip to content

fix(web): keep synthetic session keys verbatim so marks survive persistence - #353

Closed
onychen wants to merge 1 commit into
openpi-dev:mainfrom
onychen:fix/web-synthetic-session-keys
Closed

fix(web): keep synthetic session keys verbatim so marks survive persistence#353
onychen wants to merge 1 commit into
openpi-dev:mainfrom
onychen:fix/web-synthetic-session-keys

Conversation

@onychen

@onychen onychen commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

Problem

Fixes #351.

When the current workspace is removed while its active session has not yet persisted (no session file yet), removeWorkspace keys the session into ungroupedSessions with the synthetic current:<sessionId> marker — but passes it through resolve() 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/select is rejected by requireSession with "Workspace is not available". archiveSession applies the same pattern and loses archived the same way.

Value

  • The workspace-delete confirmation promise ("Its conversations will move to Ungrouped") now holds for the active, not-yet-persisted conversation — today it breaks on the very first message.
  • Archiving an unpersisted session keeps the archive mark across persistence.
  • No more fake <cwd>\current:<uuid> entries accumulating in workspace-state.json / archived-sessions.json.

Approach

Follows the minimal fix proposed in #351:

  • Synthetic current:<sessionId> keys now round-trip verbatim: a shared sessionKey() helper (with isSyntheticSessionKey) is used on write (removeWorkspace, archiveSession) and on load (ensureWorkspaceStateLoaded, ensureArchivesLoaded); real paths keep being resolved as before.
  • Projection lookups go through a new hasSessionMark() helper that matches a mark by either the resolved file path (marks recorded after persistence) or the synthetic current:<id> key (marks recorded before it), so marks survive the session gaining a real file without any migration.
  • Legacy polluted entries (<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.
  • Verified the new tests fail against the unfixed adapter (4 failing tests with the fix stashed), so they genuinely lock the bug.
  • tests/web pure-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.
  • Full node scripts/run-tests.mjs on 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

  • User-visible: removing the current workspace keeps its active unpersisted conversation in Ungrouped after it persists, and it remains selectable; archiving an unpersisted session keeps the mark after persistence.
  • Persisted data: new marks are stored verbatim as 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.
  • Model-visible context / runtime lifecycle: none.
  • Compatibility: marks written by the old code (polluted keys) were already non-matching orphans, so existing state sees no regression.

…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
@onychen

onychen commented Sep 2, 2026

Copy link
Copy Markdown
Contributor Author

改用中文描述重新提交,新 PR 见后续评论。

@onychen onychen closed this Sep 2, 2026
@onychen
onychen deleted the fix/web-synthetic-session-keys branch September 2, 2026 06:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug(web): removing the current workspace hides its unpersisted active session instead of moving it to Ungrouped

1 participant