Skip to content

fix(server): load session history without starting a Pi process - #424

Merged
oxwen11 merged 7 commits into
mainfrom
fix/cold-session-history
Oct 5, 2026
Merged

oxwen11 merged 7 commits into
mainfrom
fix/cold-session-history

Conversation

@oxwen11

@oxwen11 oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner

Summary

Opening a session was slow because getMessages and getModelState acquired a pie-pi-process (about 6–12s) just to read settled history.

Requests now go through the Pie session. If a Pi process is held or still starting, those reads use it. Otherwise the session opens the Pi file once with SessionManager.findById + SessionManager.open and reuses that handle. Starting a process drops the handle so the daemon and the child do not both keep the file open.

Isolated pie serve check against a seeded session file: getMessages 46ms, getModelState 4ms, second getMessages 2ms, and no new pie-pi-process.

Test plan

  • pnpm --filter @getpie/server exec vitest run test/harness/session.test.ts test/harness/session-service.test.ts test/harness/pi-agent.test.ts test/harness/pi/session-tools-composition.test.ts
  • Isolated serve: WebSocket getMessages / getModelState return the on-disk transcript and Pi model, with no new Pi process

@pkg-pr-new

pkg-pr-new Bot commented Sep 30, 2026 •

Copy link
Copy Markdown
npx https://pkg.pr.new/oxwen11/pie/@getpie/cli@424

commit: 6e6a623

@oxwen11

oxwen11 commented Sep 30, 2026

Copy link
Copy Markdown
Owner Author

Required CI blocker — not reviewed or merged

Head 06ff28412b9d6b89a1480b25dea3703ae02f63ca; current target/trusted rules 01f864fe30b4376714547bd6bd6576b5540e3d03. Check failure reports TS2322 in packages/server/src/harness/session.ts at lines 187, 553, 557 and 558: mismatched Effect error channels (AgentOperationError | SessionClosed versus AgentOperationError | SessionNotResumable) in the session shape/history/model methods. @getpie/server#typecheck fails.

Stopped at CI, before full code review/independent E2E. Reconcile those interface/implementation error channels without masking them with assertions, obtain successful required checks, then restart review and real cold-history acceptance. No rerun, implementation changes, conflict resolution or merge performed.

@oxwen11

oxwen11 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

CI type-error repair submitted — not independent acceptance

Repair commit 4d878c99, base 4a338cb0f5e1b22d24ca1dea46ce1ee6a50c2cd3. Independently reproduced the four reported TS2322 errors with the server Turbo typecheck before switching to implementation.

The private read dispatcher now preserves the union of its live-runtime and cold-file error channels. The injected cold model reader handles only SessionNotResumable as missing model data (undefined), matching the existing production missing-file/metadata-fallback contract. Other read failures still propagate; message-history reads still report SessionNotResumable, and live reads still preserve SessionClosed. No casts, broad catch/swallow, public error-contract widening or storage changes.

Added a runnable regression covering absent cold data, unreadable cold data and closed live-runtime reads. Before the repair it failed with SessionNotResumable escaping modelState; after repair, 4 files / 67 tests passed with no test type errors. Server Turbo typecheck and pre-commit lint/format passed. Ordinary push; no conflicts resolved.

This removes the locally reproduced compilation blocker only. New required CI, full independent correctness/security review and cold-history/no-child-process/resume runtime acceptance remain required. No approval, merge or deferred auto-merge.

@oxwen11

oxwen11 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

Queue update after verified #423 merge — waiting at CI

Head d44bfb8e2c489154656e4b206335b70de50cccf1 now contains main/trusted-rules 0920277492d381b108b520b341c449bcc3e3dbd8 (behind_by=0 verified). GitHub performed a conflict-free merge update guarded by the previous exact head; no conflict resolution or source repair.

Required CI for this new head is queued/in progress at this observation, so no old-head review or acceptance is carried forward. The earlier repair/self-test record is not independent acceptance. No additional repair was made.

No merge or deferred auto-merge. Continue only after the applicable blocker is resolved and the new version passes required CI → review → independent acceptance.

@oxwen11

oxwen11 commented Oct 1, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

head: d44bfb8e2c489154656e4b206335b70de50cccf1

Fail closed on exclusions + no matching allowed group:

  • non-presentational fix / refactor: cold transcript/model reads without starting a Pi process — new SessionColdRead, manager.messages / modelState, and session-service path changes in session-manager.ts, session-service.ts, session.ts, pi/agent.ts.
  • Changes Effect/session control flow and agent interface (getMessages / getModelState error types), not docs/tooling, lint-format, or ui-presentation.

Din review-first; no merge.

iamdin added 2 commits October 5, 2026 18:08
Opening a session read transcript and model state by acquiring a
pie-pi-process. The session now sends those reads to the live process
when one is held, and otherwise opens the Pi session file once with
SessionManager and reuses that handle.
@oxwen11
oxwen11 force-pushed the fix/cold-session-history branch from d44bfb8 to 2014707 Compare October 5, 2026 10:08
@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

Review — blocked on a persistence decision (not merged)

  • Head 5c9ee6ec (merge-update of reviewed source 20147071 onto main 2111ae73; no source change in the update). Base/trusted rules 2111ae73. Required CI for this head was still running when this was written.
  • Full source diff, the SessionManager.open / findById paths in @earendil-works/pi-coding-agent 0.99.1, and docs/host-persistence.md were reviewed. Runtime verification was not started because of the finding below.

Blocking: daemon now opens Pi's transcript files in-process

session.ts cold reads call SessionManager.findById + SessionManager.open in the daemon. That depends on Pi's physical files, not its runtime, and the library can write on open:

  • _loadEntries → migrateToCurrentVersion → _rewriteFile() (full rewrite of an older-version transcript),
  • loadEntriesFromFile appends "\n" to a file whose last line is incomplete,
  • an empty file is rewritten with a fresh header.

Before this change only the pie-pi-process child opened that file. docs/host-persistence.md (Pi-owned writes) states: "Any design that starts depending on Pi's physical files rather than its public runtime behavior requires a new Developer-approved persistence decision", and persistence.md requires the inventory update in the same slice. Neither is present.

Needed to unblock:

  1. Developer decision on whether the daemon may open Pi transcripts in-process (including those migration/repair writes) or must stay read-only / runtime-only.
  2. If approved, update docs/host-persistence.md in this PR (writer, when it can rewrite, interaction with a concurrently starting child).

Non-blocking notes

  • route checks the lifecycle, then the cold path calls openFile. A runtime start that begins between the two leaves piFile opened just before the child takes over. forgetFile runs on Start and releaseRuntime only, so if that runtime later exits by any path other than releaseRuntime, the next cold read reuses a pre-turn handle (stale transcript). Clearing the handle wherever held is cleared would close this.
  • The cached handle also never notices an out-of-band writer (e.g. the same Pi session continued from the pi CLI) until the session object is dropped.

Restart at CI after the PR changes.

iamdin and others added 4 commits October 5, 2026 12:49
Cold history reads may open a Pi transcript in the daemon. Pi 0.99.1 can
append a missing newline, rewrite the file on migration, or write a header
into an empty file. Record owner, trigger, atomicity, and concurrency.

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

auto-merge: no

Head 6e6a623a77ba, checked against main 8bc87e299b65 (#430).

  • Gate passes: open, non-draft, MERGEABLE/CLEAN, all 6 checks succeeded.
  • Exclusion, a non-presentational fix and performance change: session.ts (+182/-25), session-manager.ts, and session-service.ts send getMessages/getModelState through the Pie session. They also add a cold SessionManager.findById + SessionManager.open handle and drop it when a Pi process starts. That changes server control flow and how session files are opened.
  • pi/agent.ts and pi/protocol.ts change the agent/protocol surface, and the new and changed tests cover that new control flow.
  • The docs/host-persistence.md part would be allowed on its own, but the rest of the diff excludes the PR.

This needs Din's review.

@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

审查与独立验证记录:通过

  • Head 6e6a623a77ba1d98716ac30e1fcb9cd966cb6651;base / 可信规则 commit:main 8bc87e299b65099057f7711abe7f824b95c0fefb(head 已包含)
  • CI:当前 head 的必需检查全部 SUCCESS,见 Check、react-doctor 及两项 Publish preview。

之前的阻塞

  • 持久化决策已由 Developer 批准:允许 daemon 在进程内对 Pi transcript 调用 SessionManager.findById / open,包括 Pi 0.99.1 在迁移或修复时对文件的改写。
  • docs/host-persistence.md 已在本 PR 中登记这条写路径,内容覆盖路径、owner、触发条件、三种修复写入、非原子的 _rewriteFile、并发规则、回滚兼容和保留策略。
  • 清单写明,更新版本的 Pi 若带来新的 open 副作用,需要重新做持久化决策。这条直接约束 chore: upgrade pi coding agent to 1.0.2 #433(Pi 1.0.2)。

审查

  • route() 的优先级是:已持有的 runtime → 正在获取 runtime 的 ticket → 文件。
  • 只有在没有 Pi 子进程、也没有子进程正在启动时,才会在进程内 open。
  • 启动或释放子进程前会先 forgetFile,所以 daemon 和 Pi 子进程不会同时持有同一个文件句柄。
  • 冷读与热读的错误契约保持不变。
  • 不改 RPC 契约。

独立验证

在 reviewer 自己的 worktree 中检出该 head,执行 pnpm install --frozen-lockfile,再用 Turbo 构建 server 和 CLI。

  • 测试:session、session-service、pi-agent 三个 harness 测试文件,共 60 个测试,全部通过。
  • 运行时:用 pie-verify cli 起一个隔离 PIE_HOME 的 daemon,使用操作者现有的 Pi 配置和真实模型,没有传 --provider / --model-id,所以不会改写 Pi 的全局默认值。步骤如下:
    1. pie run "Remember the code word PIE-COLD-424…":建 Session,助手回复 noted。
    2. pie daemon stop → pie daemon start,得到新的 daemon。
    3. pie logs 冷读:完整返回两条历史消息。冷读前后,daemon 的子进程都只有 resource-monitor,没有启动 pi-process。
    4. pie send "What was the code word?…":这时才启动 pi-process,并恢复同一个 transcript,助手回答 PIE-COLD-424。这说明冷读 open 之后放掉了句柄,没有影响后续的 live resume。
## after restart: pi processes = 0
## pie logs (cold read)
user	Remember the code word PIE-COLD-424. Reply with just: noted.
assistant	noted
## after cold read: daemon children = resource-monitor only
## live turn after cold open
user	What was the code word? Reply with only the word.
assistant	PIE-COLD-424

这是 server 和 CLI 路径的证明,属于非 UI 改动,按 acceptance 规则用命令结果作证据。Session 的 transcript 写在操作者共享的 ~/.pi/agent 下,这是 Pi 自己的数据。

未覆盖

没有构造 version < 3 的旧 transcript、末尾缺换行的文件或空文件,现场验证修复写入。这三种情况由 Pi 实现决定,清单里已登记,本 PR 的测试覆盖了路由和句柄生命周期。

结论:通过,可以合并。若 head 或 base 变化,从 CI 重新开始。

@oxwen11
oxwen11 merged commit 580ec56 into main Oct 5, 2026
6 checks passed
@oxwen11
oxwen11 deleted the fix/cold-session-history branch October 5, 2026 18:11
@oxwen11

oxwen11 commented Oct 5, 2026

Copy link
Copy Markdown
Owner Author

已合并(squash)。merge commit 为 580ec5626ef448a7fceff6f7600a01214d3f0f4b,合并时的 head 为 6e6a623a。审查与验证记录见 #424 (comment) 。

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