Skip to content

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

Open
testikun wants to merge 8 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 8 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. Web cannot inspect terminal/default Pi Session history without conflating history observation with activation. Review identified global transcript discovery and missing cancellation of preview work.

Value

Expose a workspace-scoped, read-only history API without copying terminal Sessions, switching the Web controller, or attaching to a running TUI process.

Approach

  • Authenticated GET /api/terminal-sessions with bounded query/cursor/limit validation.
  • Discover only the selected visible workspace's encoded default-store directory, without SessionManager.listAll() or the mkdir-producing SessionManager.list(workspace) path. Read at most 256 KiB per candidate for at most WEB_MAX_SESSIONS candidates; never retain allMessagesText.
  • Query only name/cwd/firstMessage metadata. Preview requires path/cwd membership and uses existing compaction-aware loadSessionPreviewData(path, { signal }) with 80-message / 1 MiB retained-output limits.
  • Carry request disconnect and Host shutdown cancellation through discovery and preview; reject an aborted read instead of returning success. Always remove request listeners.
  • Label rows source: "pi-default", origin: "terminal", readOnly: true; reject mutations with 405 and unavailable targets with 404 SESSION_NOT_FOUND.

Validation

Revalidated on 2026-09-09 using Node 24 and Bun 1.3.14:

  • bun run check: passed.
  • bun run test: Node 1467 passed, 1 skipped; Vitest 130 passed.
  • Fresh GitHub CI for 11348ee: Node 22.19.0 / 24 / 26, Web E2E, and Windows all passed.
  • Adapter/Host focused tests: 44 passed after the final scoped change.
  • Real two-workspace fixtures and file-open spy prove unrelated transcripts are not opened during list/preview; later-message-only query remains excluded. Default global listing is forbidden by a spy.
  • Pre-aborted and mid-preview cancellation reject with AbortError. Hidden/unbound workspaces do not walk the default store. Adapter/Host output excludes allMessagesText.
  • Compacted fixture retains the firstKeptEntryId window within 80 messages / 1 MiB. Read-only requests preserve terminal bytes, Web controller identity, and empty Web Session history; no archive sidecar is created. POST/PATCH/DELETE return 405.
  • Ablation: disconnecting the preview cancellation path makes the regression fail with missing expected AbortError; restoring it passes. An unrelated file-identity check added during implementation was removed to retain the minimal cancellation scope.
  • No installed UI/provider/live-attach validation was performed.

Impact

  • User-visible: authenticated history listing/inspection API, no UI added.
  • Model-visible context/tools: none.
  • Runtime/lifecycle: cancellation follows the HTTP request and Host shutdown; active Web Session/controller stays unchanged.
  • Persisted config/data: no copy, Session mutation, Web index or archive write.
  • Compatibility/risk: history preview is NOT live attach and cannot reconstruct terminal process-local /ps state. Metadata scans remain capped prefixes and candidate-directory stat work is not an indexed catalog. Maintainer approval is still required.

@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.

@testikun

testikun commented Sep 9, 2026

Copy link
Copy Markdown
Contributor Author

Review follow-up pushed as 11348ee: request disconnect / Host shutdown now cancel terminal metadata and the existing preview loader. Expanded real-file regressions for two-workspace open isolation, later-message query exclusion, hidden/unbound discovery, compaction, controller/file/sidecar preservation, mutation 405, and abort-before/during-preview. The cancellation ablation fails without the fix and passes restored. bun run check passed; full suite Node 1467 passed / 1 skipped, Vitest 130 passed; focused adapter/Host 44/44. PR body now describes the actual bounded metadata implementation instead of obsolete listAll discovery. Please re-review; this remains file history, not live attach or terminal /ps.

@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.

Reviewed exact head 11348ee.

This is a useful read-only backend seam for terminal Session history, and the earlier global-history scan, cross-workspace exposure, and unbounded transcript accumulation are fixed. I found no P0/P1. Two P2 correctness/boundedness issues remain:

  • The 256 KiB prefix is presented as complete metadata. I reproduced a valid 581,477-byte Session with 2,201 messages being reported as 996; a later session_info rename was omitted and an exact search for that name returned zero results. Expose truncation/approximation explicitly or derive correct bounded metadata.
  • Discovery readdir/stat calls are not fully bounded: all .jsonl entries are statted concurrently before WEB_MAX_SESSIONS is applied. Filesystem calls and memory still scale with the whole directory.

Focused exact-head tests pass 44/44 and required CI is green. I am not auto-merging because this changes core Session-history discovery and identity semantics; the remaining P2s directly affect the feature contract, and there is not yet user-facing browser acceptance.

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