Skip to content

fix(web): bind copied session selection to file identity - #584

Merged
tt-a1i merged 2 commits into
openpi-dev:mainfrom
ooiuuii:fix/web-session-file-identity
Sep 21, 2026
Merged

tt-a1i merged 2 commits into
openpi-dev:mainfrom
ooiuuii:fix/web-session-file-identity

Conversation

@ooiuuii

@ooiuuii ooiuuii commented Sep 19, 2026

Copy link
Copy Markdown
Contributor

Problem

A copied Pi JSONL session retains its embedded Session ID. Using that ID alone can read, rename, or mutate the wrong file. A UI-only selection guard is insufficient: a prompt/model/thinking request can arrive or execute after another tab activates a same-ID copy.

Fixes #583.

Value

Confirmed selection and subsequent mutations stay bound to the actual Session file. Stale requests fail closed instead of writing to a copy, and an uncertain command receipt cannot be rebound across files.

Approach

  • Reuse a small Session ID + normalized file-path comparison for the adapter and runtime. Keep SessionManager as the persistence authority; support the existing current:<id> identity for in-memory sessions.
  • Carry the confirmed sessionPath on prompt, model, and thinking HTTP mutations. The host validates it, and runtime admission/serialized execution validates again after queue waits.
  • Bind prompt replay records to that path as well as command/content/image identity. Retain the existing receipt for a legitimate retry without redispatching it.
  • For coalesced thinking writes, apply only the last still-valid caller's level; reject stale callers individually.
  • Preserve already-admitted retained-runtime work. This is not a new cancellation policy, controller, persistence format, or cross-session lock.
  • Keep copied-session reads/renames/selection path-aware; use confirmed canonical snapshots before enabling writes. Update dependent UI and browser fixtures rather than weakening the guard.

Validation

  • Native HTTP regressions: partially send a real request body, select a copied JSONL via HTTP, then finish the old request. Prompt/model/thinking reject with 409 and no runtime mutation; the same copy's valid request succeeds. Missing/invalid file identity and cross-file receipt replay are also covered. 7 tests RED before the host fix, GREEN afterward.
  • Runtime queued-race regressions: 4 tests RED before guards, GREEN afterward, using native SessionManager copied-file identities, plus normalized-path/in-memory identity coverage.
  • Combined adapter/runtime/host/race tests: 119/119 passed.
  • Final head: 0c035fd6287d12dcffc256ca7ad313b4aef58a30. Web unit/spec tests: 19 files / 279 passed with one worker; bun run check passes. A parallel attempt had one unchanged provider-settings timeout; no timeout/assertion was altered.
  • Review closeout: five additional RED/GREEN recovery tests protect pending thinking state, revision ownership and stale reconciliation GETs across same-ID file switches. The final scoped full-branch structured review reports no actionable findings.
  • Final native Windows browser suite: 49/50 passed. The remaining terminal-disposal failure is independently reproduced on unmodified main; both first-prompt creation and delayed cross-tab receipt tests pass 2/2 in isolation. No full-repository-green claim is made.
  • The old four Web E2E failures were traced to inconsistent canonical snapshot mocks (selected/current file missing from the controlled session list). All four pass after correcting those fixtures, without deleting tests or relaxing product checks.

Limitations: full local bun run test hit its predeclared 240-second bound after reporting setup integration and reused-PID lock failures, so it did not produce a complete aggregate. A native Windows full-browser run also exposed Signals not supported on windows in the unchanged interactive-terminal disposal path before prompt admission. Those independent surfaces are not changed here. No paid/live model request or user's installed Pi configuration was used.

Impact

  • Three local Web mutation endpoints now require sessionPath; refresh stale browser assets after upgrading. Internal runtime callers may still omit the optional expected identity.
  • Newer intents must not inherit stale file-scoped pending state after canonical snapshot recovery.
  • No change to stop/cancel, command discovery, workbar APIs, stored JSONL IDs, or unrelated lifecycle policies.
  • Updated onto main f991f9e as one linear topic commit; the already-merged workbench changes are not part of this PR's diff. Tests and generated UI assets are included with the same identity invariant.

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The path-aware UI checks are a useful projection fix, but the mutation boundary still identifies a session only by its embedded ID. A copied JSONL can share that ID, so an in-flight request from the old file can still be admitted against the newly selected file. Please carry the canonical session path/file identity through the mutation protocol and enforce it in the host/runtime, with a regression covering a request that races a switch between copied sessions.

Comment thread web/ui/src/store/web-store.ts

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

审查提交:8bc6375ae65948ae0476c9ffb0675812137887d2

需要修改后再合并。

具体问题

P1 · web/ui/src/store/web-store.ts:1275

Copied-session file identity is checked only in local projection. Prompt/model/thinking requests still carry only embedded sessionId, and server admission checks only that ID. Original and copied JSONL share it, so an already-in-flight original request arriving after selection of the copy is admitted against the wrong file. Carry canonical file identity or runtime generation through mutations and revalidate at admission.

验证范围

Reviewed complete production/test diff and current client/runtime request checks; 64 adapter/runtime tests passed, including copied-file projection and rename isolation. Existing exact-head finding remains applicable.

限制与后续

Did not independently run a delayed network race or browser E2E. Tests cover UI suppression after mismatch, not stale request admission.

已核对的 CI 失败

Run 35427046809:Web E2E 四项失败:暗色预期得到 light、运行状态按钮禁用、归档历史元素缺失、轨迹按钮等待超时;不能归为已确认基础设施故障。

@ooiuuii
ooiuuii force-pushed the fix/web-session-file-identity branch from 8bc6375 to 0c035fd Compare September 21, 2026 14:08

@tt-a1i tt-a1i left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Re-reviewed 0c035fd. The copied-session mutation finding is addressed: prompt/model/thinking requests carry the file identity, Host validates it, and queued runtime mutations revalidate before execution. Local verification passed 96 Node tests and 128 UI/protocol tests. Current CI, including Web E2E and Windows, is green. This supersedes my previous request for changes. I did not independently rerun browser E2E locally.

@tt-a1i
tt-a1i merged commit 245de29 into openpi-dev:main Sep 21, 2026
6 checks passed
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): copied session IDs alias the active session file

2 participants