Skip to content

feat(web): expose read-only terminal session history - #397

Open
testikun wants to merge 7 commits into
openpi-dev:mainfrom
testikun:codex/issue-347-terminal-history
Open

feat(web): expose read-only terminal session history#397
testikun wants to merge 7 commits into
openpi-dev:mainfrom
testikun:codex/issue-347-terminal-history

Conversation

@testikun

@testikun testikun commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Problem

Related to #347. The Web workbench has no way to discover compatible terminal/default Pi Sessions without copying them into Web storage or switching the active Web controller. This makes existing history invisible and risks conflating observer access with control.

Value

Provides a bounded, explicit read-only bridge to Pi's default Session source so users and future UI work can inspect terminal history while preserving Pi files and controller ownership as the authority.

Approach

  • Add authenticated GET /api/terminal-sessions listing with bounded query, cursor, and limit validation.
  • Restrict results to the currently selected, visible Web workspace and discover them from SessionManager.listAll() (Pi's default Session source).
  • Add path inspection returning a bounded preview from the existing loadSessionPreviewData loader, including omission/byte evidence.
  • Mark every result with source: "pi-default", origin: "terminal", and readOnly: true.
  • Reject unavailable targets with an explicit 404 receipt; no copy, activation, mutation, editing, deletion, or fork path is introduced.

Validation

  • Focused Web adapter/host tests: 34 passed, 0 failed.
  • Full Node/Vitest suite: 1339 passed, 1 skipped, 0 failed; Vitest 30 passed.
  • biome format / biome lint --error-on-warnings: passed.
  • tsc --noEmit: passed.
  • Config-contract, discipline-ledger, and Web syntax checks: passed.
  • Ablation: removing the typed read-only-session error boundary changed missing-session responses from the required 404 SESSION_NOT_FOUND receipt to a generic 500; the boundary was restored.
  • bun is not installed in this environment, so equivalent repository scripts were run with the bundled Node 24 executable and local Biome/Vitest binaries.

Impact

  • User-visible behavior: new authenticated read-only terminal Session listing/inspection API; no UI changes in this PR.
  • Model-visible context/tools: none.
  • Runtime/lifecycle: active Web Session/controller is never changed.
  • Persisted data: no Web index or Pi Session file is written.
  • Compatibility/risk: output is bounded and source/read-only labeled; native UI integration remains a follow-up slice.

@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 read-only terminal-history bridge is valuable, but its current discovery path introduces a P2 history-size-dependent workload on every page and preview. The inline finding was reproduced with real default Pi Session files; focused adapter/host tests pass42/42. Please correct the source-read boundary before merge. No author code was modified, and no installed/deployed performance measurement is claimed.

Comment thread web/adapter/pi-adapter.ts Outdated
@tt-a1i

tt-a1i commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Source-read follow-up (2026-09-09). This PR is still the leftover file-history slice, not live attach.

Status. OPEN, CHANGES_REQUESTED, mergeStateStatus: BEHIND. Head edeb7b3081ca79c0e384aef38a11d8f2d4c8a887. The 2026-09-07 review stands: SessionManager.listAll() with no directory walks every default Pi workspace and buildSessionInfo retains allMessagesText before the cwd filter and page slice; getReadOnlyTerminalSession repeats that scan. Reproduced on real default Session files. origin/main 0d17f4577fe31315fe6c95370d251bdb4e2413cf still has no pi-default / /api/terminal-sessions string. Checkout a9b40f0044ee59c360a6077c1c0bdbdbd30da10b has no Web.

What this PR actually does. GET /api/terminal-sessions lists Pi’s default store (~/.pi/agent/sessions), filters by the current Web workspace cwd, and inspects a path with loadSessionPreviewData (the origin /sessions preview loader: compaction-aware model window, ≤80 messages / 1 MiB). Every row is source: "pi-default", origin: "terminal", readOnly: true. Tests assert listAll(web-sessions) stays empty — no copy, no switchSession, no write, no UI. Protocol WebSessionSummary.source on origin is still the "web-session" literal; this PR does not widen that union.

What it is not. Opening that JSONL later is history. It does not attach the Web Host to the TUI process that still has the file open. Background Terminal / Subagent / Workflow managers are process-local objects keyed by the live SessionManager; a later file open or this preview cannot reconstruct /ps. That attach remains #76 observer/controller and is out of scope here.

Relation to /sessions. TUI /sessions is the same catalog seam plus Enter → ctx.switchSession (this process adopts the file: shutdown old runtime, session_start on the target). This PR is the preview half only. It is not compaction (same-file window) and not #154 rewind (same-file leaf move).

Please keep the P2 source-read fix on this PR (discover the selected workspace first; bound and cancel transcript work; prove unrelated workspace files are not parsed). Rebase onto current main after that. Do not expand the PR into live attach, and do not treat a future merge as “Web can see my running TUI /ps.”

@tt-a1i

tt-a1i commented Sep 9, 2026

Copy link
Copy Markdown
Collaborator

Replacement-test follow-up (2026-09-09). Head still edeb7b3081ca79c0e384aef38a11d8f2d4c8a887. The 2026-09-07 P2 and the earlier file-history comment stand; this list is the fail-closed matrix that comment did not enumerate. Do not merge edeb7b3 until these lock. list(cwd) alone is not a complete replacement (buildSessionInfo still retains allMessagesText; getDefaultSessionDir mkdir is a write).

Must hold

  • List candidates are only {sessions}/--<encode(workspace)>--/*.jsonl before any transcript body is opened. Zero-arg SessionManager.listAll() (and listAll(getSessionsDir())) is forbidden on this route.
  • Discovery must not materialize allMessagesText before the cwd/query/page filters. Query stays name / cwd / firstMessage. HTTP JSON must not contain allMessagesText.
  • Preview is loadSessionPreviewData(path, { signal }) after a path+cwd membership check — not a second full-tree listAll(). Preview ≠ Web archived-sessions.json / /resume corpus ≠ live /ps pids.
  • Every row source: "pi-default", origin: "terminal", readOnly: true. No copy, switchSession, or write. After list+preview, listAll(web-sessions) still does not contain that path; GET /api/sessions still has no pi-default row.

Missing vs current 42/42 (single-fixture) tests — fail closed

  1. Spy: this route never calls zero-arg listAll() (progress-only overload counts as zero-arg).
  2. Two default-store workspaces under one PI_CODING_AGENT_DIR: limit=1 total === 1 and only the current cwd path is returned. Today’s total === 1 is vacuously green.
  3. Unrelated workspace file is not body-opened (createReadStream / open / readFile spy) during list and during in-workspace preview. Absence of its token in the JSON is not enough.
  4. "allMessagesText" absent from adapter objects and Host raw JSON.
  5. Query equal to a token that exists only in another workspace’s later messages → total === 0 and that file still not opened. Do not add an allMessagesText search.
  6. Preview of the other workspace path → 404 SESSION_NOT_FOUND and loader never opens it.
  7. Compacted fixture: messages follow the firstKeptEntryId window (≤80 / 1 MiB); Web archive sidecar is unchanged and does not mark the TUI path.
  8. After list+preview, GET /api/capabilities sessionId is still the in-memory Web id; no bt- / capability ids on the terminal-history payload; GET /api/sessions has no that path.
  9. POST/PATCH/DELETE /api/terminal-sessions → 405; TUI file size/mtime/hash unchanged; Web runtime id unchanged.
  10. Hidden / unbound workspace: no default-store walk.
  11. AbortSignal cancelled before/during preview is not a 200.

Keep existing label / empty-web-sessions / 400 / 404 assertions. Rebase onto current main after the source-read fix. Do not expand into live attach. Do not treat a future merge as “Web can see running TUI /ps.”

@testikun

testikun commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Fixed and pushed as a8669c0 (merged latest main).

  • Terminal Session discovery and detail now use Pi's workspace-scoped SessionManager.list(workspace) instead of global listAll().
  • Workspace admission for this read-only route no longer triggers a full Web Session scan, so unrelated default workspaces are not parsed as part of the request.
  • Added a regression that fails if global discovery is used while listing or previewing terminal Sessions.
  • Validation: Node 24 Web adapter tests and bun run check 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.

2 participants