Repository navigation
Conversation
The PR description documents sufficient testing: the representative built-CLI fixture migration satisfies the real-CLI requirement. An authenticated Pi/OMP run or full provider × agent matrix is not required. No commands from the PR were run. |
|
Fixed the Windows matrix failure in b1664c0: the sessions locate matrix now covers Pi/OMP default, moved-home, profile, project, and PI_CODING_AGENT_SESSION_DIR stores; sessionStoreDir also honors the explicit Pi-family session-dir override. Local Windows-safe suite: 33 files, 4,876 tests passed. Lint, tsc, and focused session tests pass. |
The PR description documents sufficient testing: the representative built-CLI fixture migration satisfies the real-CLI requirement. An authenticated Pi/OMP run or full provider × agent matrix is not required. No additional confident findings. Only the specified diff was read; no PR code was run, built, or installed. |
Reviewed at b1664c0 against #1418's Pi and Oh My Pi rows. The adapter works on real stores: I wrote sessions with Pi 1.4.0 and Oh My Pi 18.12.6 (through a local stub provider) and ran the built CLI. Two things to fix before merge:
Non-blocking:
@jeff-r2026 Your call on this one once the two blocking items are fixed: it adds Pi and Oh My Pi as readable session platforms. |
|
Addressed the blocking and follow-up findings in f74bfb9:\n\n- replaced the three stale tests that used Pi as an unreadable-session example with Copilot\n- made sessionStoreDir honor Pi's settings.json sessionDir, after the env override and with ~/relative expansion via �bsoluteOn\n- fixed PI_CODING_AGENT_SESSION_DIR tilde expansion\n- resolve assistant models and model_change records in active-branch order, so the latest event wins\n- fall back to the newest entry for a dangling �ctiveLeafId\n- added Pi/OMP to both dashboard docs\n- added regression tests for settings resolution, latest model changes, and dangling leaves\n\nLocal verification: lint, tsc, build, Windows-safe suite (33 files / 4,894 tests), focused adapter/store tests, and the dashboard E2E all pass. |
|
No new confident findings in the specified diff.
Only the specified diff was read. No PR code was run, built, or installed. |
Re-checked at f74bfb9 with real Pi 1.4.0 and Oh My Pi 18.12.6 sessions. Resolved:
One left:
Nits:
@jeff-r2026 your call once the project-level |
Summary
PiAdapterfor Pi and Oh My Pi v3 JSONL sessionsTOOLS, every README matrix, and the generated support docsRefs #1418 (Pi / Oh My Pi row only).
Verification
npm run lintnpx tsc --noEmitnpm run buildnpx vitest run src/__tests__/pi-session-adapter.test.ts src/__tests__/session-platform-facts.test.ts src/__tests__/readme-matrix.test.ts src/__tests__/tool-support.test.ts src/__tests__/agent-session-list.test.ts --maxWorkers=2npx vitest run tool-support -uRepresentative built-CLI run against a Pi v3 fixture store:
I did not label this as a real authenticated Pi/OMP session run: the local machine has no configured Pi/OMP provider session. The fixture uses the documented v3 record graph and the adapter tests cover both variants, including OMP's title slot.
Shared-state review
PiAdapterrecursively reads.jsonlfiles only.writeSessionthrowsToolUnavailableErroranddeleteSessionreturnsfalse.sessionStoreDirremains the single resolver, so recorded member tool roots and OMP profiles keep their existing behavior.Real-CLI record on real Pi and Oh My Pi sessions (added by a reviewer)