Skip to content

fix(agent): write a session file with its first message (plan decision 17) - #25

Merged
zy1233 merged 4 commits into
mainfrom
fix/write-session-at-first-message
Oct 19, 2026
Merged

zy1233 merged 4 commits into
mainfrom
fix/write-session-at-first-message

Conversation

@zy1233

@zy1233 zy1233 commented Oct 18, 2026 •

Copy link
Copy Markdown
Owner

Summary

Plan decision 17 (answered by the owner on 2026-10-18: do it). The Python agent wrote the file of a session when it answered session/new. A run in which nobody typed left a file titled (no messages). That file stayed in the /resume list, and zypi --continue took it when it was the latest session of the directory. Now the agent writes the file with the first user message.

before                                       after
session/new                                  session/new
  write sessions/<time>-<id>.jsonl             keep the session in memory
  (header only)                                (the header and the entries made so far are held)
user types nothing                           first user message
  file stays, title "(no messages)",           ONE write: header + held entries + the message
  listed, `--continue` can take it           later entries
                                               appended as before
                                             user types nothing
                                               no file: not listed, `--continue` cannot take it
 pi-agent-harness
 session/jsonl_storage.py
+  DeferredJsonlSessionStorage   holds the entries before the first `message`; one `write_file` at the first one
+                                (write first, record after: a failed write records nothing, the next message retries)
+  new_session_header(timestamp=) header time = file-name time = `session/new` time
 session/jsonl_repo.py
+  create({"deferred": True})    only this option returns the deferred storage; `fork` and plain `create` write at once
 pi-agent-cli
 agent.py
+  session/new                   makes deferred sessions
+  _reserved_ids                 an id chosen by the client (`--session-id`) stays taken while its session is open
+  _refuse_id_taken_meanwhile    before the first message: if another process saved that id meanwhile, refuse
 pi-pager: no production code changed

What each ACP method does for a session that has no file:

session/new                    makes it in memory; a chosen id is reserved
session/prompt (the first)     writes the file; a chosen id is checked once more first (invalid_params if taken)
session/list                   not listed
session/load, session/resume   resource_not_found
second session/new, same id    refused (invalid_params)
session/close, pi/session/delete   release the id; delete stays idempotent (deleted: true)

Evidence

  • Before: with session/new made to write at once again (the old behaviour), the real-zypi matrix resume_matrix.py --strict fails: 15 PASS / 1 FAIL for the no-id path (/new, left with no message); 13 PASS / 3 FAIL for the chosen-id path (--session-id, left with no message, and --continue and --resume with no argument, which then take that empty session).
    After: 16 PASS / 0 GAP / 0 FAIL (four runs).
  • First draft of the matrix was empty. Its new case started zypi with no flag. With the old behaviour put back, all 14 cases still passed: zypi with no flag sends no session/new before the first message, so the case could not see the agent. The cases now use --session-id and /new, the two entries that send session/new at once. The matrix has 16 cases (13 before): --session-id and /new, each left with no message and no session file; /new and then a message, which saves a new session. The run types the steps of a case when the screen shows the step before took effect (Step, wait_for_run).
  • 15 one-line changes to the production code, each run against the tests that watch it: 15 caught. At the first run 2 were not caught: a header time that did not match the file name (the two reads of the clock fell in the same millisecond), and a second look at the id that also refused a session that was already saved. Both have a test now (fa62c5a).
test_a_session_in_which_nobody_typed_is_not_kept      (real agent process)
  open a session, type in another one
  list      -> only the one that has a message
  load      -> resource_not_found for the empty one; the same after a restart of the agent
  • Python tests: 16 new (10 for the storage, 6 for the agent) and 12 new in scripts/tests (the matrix cases and wait_for_run, with a fake terminal). The older tests that expect a session in the list now send a message first. Full pytest: Windows 2,559 passed, 39 skipped, 31 deselected; Linux (WSL2) 2,568 passed, 30 skipped, 31 deselected. ruff check . and ruff format --check . pass.
  • Rust (WSL2): python_agent_e2e 14/14 (the sessions that must be listed get a first message; the id check also asserts the refusal of a second session/new with the same id). rustfmt is clean for the changed file. No production Rust code changed.
  • Real zypi under a pseudo-terminal (mock model): smoke.py 23/23 (it no longer presses Down in the /resume picker: the empty session is not listed), exit_matrix.py 12/12, print_exit.py, mode_probe.py, status_line_probe.py 4/4, net_audit.py --mock 13/13: all exit 0.
  • CI (head c74cc18): CI run 39150839662: Ruff passes; pytest passes on Python 3.11, 3.12 and 3.13. TUI CI run 39150839672: Gates + cargo check passes (3 m 59 s); cargo test passes (16 m 34 s) with 8,706 passed, 0 failed, 21 ignored, the same total as main (no production Rust code changed).
  • Not verified: macOS and Windows builds of zypi; a real model turn (the e2e tests and the PTY scripts use the mock model); two agent processes that use one home at the same moment (see below).

Merge Danger

Door: two-way

The format of a session file does not change: a file written by this agent is read by an older agent, and the other way round. A revert brings back the file at session/new. No data is migrated.

Blast Radius: an ACP client that calls session/load or session/resume for a session before the first message; chosen session ids; old empty files

  • A client that loads or resumes a session before it sends a message now gets resource_not_found (it succeeded before). zypi does not do this. No other client was tried.
  • zypi with no flag starts no session until the first message. zypi --session-id <id> and /new start one at once, with no file. A chosen id stays reserved in the agent while its session is open, so a second session/new with it is refused although no file has it.
  • The gap between two agent processes that share a home is narrowed, not closed: if the other process saves that id between the check and the write, two files can still exist.
  • Files that earlier versions wrote (header only) stay in the list with the title (no messages). They are not cleaned. pi/session/delete removes one.

…on 17)

A new session was a file from session/new on: a header and nothing else. A
user who started the TUI and left, left such a file behind. session/list
showed it as "(no messages)", and zypi --continue could take it for the
latest session.

session/new now makes a deferred session (DeferredJsonlSessionStorage, in
the harness). It keeps its entries in memory until the first message entry.
That entry writes the header, the entries held before it (a model choice, a
name, a leaf move) and itself in one go. After that each entry is appended
as before. The write comes before the entry is recorded, so a failed write
leaves no trace of the message and the next message tries again. A session
in which nobody typed has no file: nothing to list, load or continue.
Sessions that older versions left with no message keep the title
"(no messages)".

An id the client chose (--session-id) must stay taken while its session has
no file, so the agent keeps the ids of its open sessions in a set. The set
leaves an id when the session is closed or deleted, or when the session
could not be opened. Another process cannot see that set. So the first
prompt of a session with a chosen id looks for the id in the files once
more, and refuses it with the same error as session/new if a session has
saved under it since.

session/load and session/resume answer "not found" for a session that has no
file: there is nothing to load.
…n 17)

The agent now writes a session file with the first user message. Three
checks that drive the real pieces had to change with it:

- scripts/tui_pty/resume_matrix.py (zypi under a pseudo-terminal): the TUI
  makes a session at once, with no message, for `--session-id` and for
  `/new`. (A start with no flag makes none until the first message.) Two new
  cases leave such a session with no message and require that no session
  file exists. The first one comes first on purpose: under an agent that
  writes at once, its empty session is the newest one, so `--continue` and
  `--resume` with no argument take it and fail too. Two cases type a message
  and look for the file after it: `--session-id <fresh uuid>`, and `/new`
  and then a message. A case can have steps (`Step`). wait_for_run types the
  first one when the TUI is up (the model name on the border of the input
  box), and each next one when the screen shows that the one before took
  effect. The `/new` cases start with `--resume <id>`, so they do not depend
  on which session is the newest. scripts/tests/test_tui_pty.py tests the
  cases and wait_for_run with a fake terminal.
  Mutation check: with either of the two `repo.create` calls in agent.py
  made to write at once, the case for that call fails (and for the call that
  serves `--session-id`, the two cases that take the newest session fail too).
- scripts/tui_pty/smoke.py: after a restart, the /resume picker used to
  list the new empty session first and the script pressed Down to skip it.
  That session has no file now and is not listed, so the earlier session is
  the first row and the script no longer presses Down.
- python_agent_e2e.rs (the Rust ACP client against the real Python agent):
  the sessions that must be listed (51 for the paging test, the `existing`
  one for the id check) now get a first message. The id check also asserts
  what the agent does for a session that has no file yet: it refuses a
  second `session/new` with the same id (InvalidParams), and once the first
  message is sent the pager's own check finds the id through session/list.

No test is added or removed on the Rust side (the count of
python_agent_e2e stays 14).
A run of 15 one-line changes to the production code of this PR, each
against the tests that watch it, found two that no test noticed:

- The header of a deferred session took its time from a second look at
  the clock and not from the time in the file name. The test compared the
  two, but both looks fell in the same millisecond, so it passed by
  chance. It now gives every look at the clock a new time.
- The second look at a chosen id (before the first message) also refused
  a session whose own file was there. No test sent a second message in a
  session with a chosen id. A new test sends three.

After the change, both are caught. The other 13 were caught before.
…st message) and r14

- CHANGELOG.md, [Unreleased]: the agent writes the file of a session with
  its first message. It lists the behaviour changes (no flag: no session
  until the first message; `--session-id` and `/new`: a session at once,
  with no file; a chosen id stays reserved while its session is open),
  what library users get (`JsonlSessionRepo.create({"deferred": True})`),
  and the tests. The entry for the cost of paging says it is left as it is.
- The embedded user guide (17-sessions.md), packages/pi-agent-cli/AGENTS.md
  and docs/DESIGN.md section 6.1: a session in which nobody typed has no
  file, so it is not listed and cannot be loaded.
- scripts/tui_pty/README.md (16 cases) and docs/baselines/tui.md.
- The plan (r14): the status, the answers to the four questions (#23 and
  #24 merged, decision 18 not done, decision 17 done, decision 10 to do),
  and the index of section 10.
- The execution log: section 10.17 (the merges, the design of decision 17,
  what each ACP method does for a session without a file, the facts about
  the TUI, the tests, two mutation runs and the two gaps they found, the
  numbers, what is not verified), B.9 (how to repeat it) and the revision
  record. One old line (about the empty first row of the /resume picker)
  points to section 10.17.
@zy1233
zy1233 merged commit 2b0baf3 into main Oct 19, 2026
7 checks passed
@zy1233
zy1233 deleted the fix/write-session-at-first-message branch October 19, 2026 01:42
zy1233 added a commit that referenced this pull request Oct 20, 2026
…used in pi-shell (plan decision 10) (#27)

* refactor(tui): delete the goal prompt templates and the research script that nothing loads

Nothing in the tree names these 15 files any more (git grep by file name: no hit).
The last code that loaded them was removed with the Rust agent runtime in
455527c (#7); the files stayed, and rustc cannot see a file that no `include_str!`
names, so no warning reported them.

pi-shell/src/session/templates/ (14 Markdown prompt templates for the goal
mode) and pi-shell/src/session/workflows/deep_research.rhai (584 lines).
15 files, 1,033 lines. No Rust code changes.

* refactor(tui): delete the dead units session::worktree and session::helpers of pi-shell

The call graph (scripts/tui_callgraph.py units) reports both as dead: nothing in the
consumer build (pi-pager-bin, pi-pager-minimal, pi-update) reaches them. rustc does not
warn, because what they hold is `pub(crate)` or a re-export.

session::worktree (359 lines): a glob re-export of pi_workspace::worktree, two `From`
impls between the shell's WorktreeType and the workspace's (nothing calls them), and 24
tests of types and functions that live in pi-workspace (serde shapes, build_candidate_list,
BackgroundCopyContext, resolve_worktree_by_id_or_path).

session::helpers (3 files, 61 lines): comments, and two tests of string shapes.

The two `resolve_worktree_by_id_or_path` tests were the ones that opened worktrees.db in
whatever GROK_HOME held; they are gone with the unit. pi-shell tests: 864 -> 838.

pi_workspace::worktree has no user left in pi-shell after this; that is left for plan D.8.

* refactor(tui): remove the [storage] section from the pi-shell config

`[storage] cleanup_ttl_days` set how long old session files were kept. The code that
read it went with the session store (#26), and the pager keeps no session file now.
`StorageConfig` and the `storage` field of `Config` are deleted.

A config file that still has the section loads. The section is reported as
unrecognized, like any other unknown section: a new test pins that, and the
"valid config" test no longer lists the section.

`storage_mode` (the `--storage-mode` flag and `Config.storage_mode`) is a different
setting. This change does not touch it.

* refactor(tui): delete util::config::worktree of pi-shell

`util::config::worktree` holds a `WorktreeType` enum (with `FromStr` and a `From` impl for
`CreationMode`). Its one user was `session::worktree`, which the previous commit deletes. The
baseline `check` shows it: the glob re-export of the module is an unused import (1 warning;
the ceiling is 0).

`pi-workspace` and `pi-workspace-types` have their own `WorktreeType` and do not use this
one. The module has one test, `test_worktree_type_fromstr`; it goes with it. `pi-shell` has
no other reference to `pi_fast_worktree` now, so that dependency goes in the next commit.

* refactor(tui): remove the dependencies of pi-shell that no source file names

Ten dependencies of `pi-shell` are not named in any source file of the crate outside comments
(a text scan of `src/`, `tests/` and `build.rs`; the compiler has the last word):

- `[dependencies]`: `urlencoding`, `base64`, `pi-fast-worktree`, `pi-compaction-transcript`,
  `pi-workflow`, `tdigests`, and `nix` (unix only).
- `[dev-dependencies]`: `pretty_assertions`, `pi-test-utils`, `tokio-util`.

Eight lost their last user in #26 (the session store). `pi-fast-worktree` lost its last users in
`session::worktree` and `util::config::worktree` (this branch), and `tokio-util` in the tests of
`session::worktree`.

The dependency graph of `pi-pager-bin` goes from 980 to 970 packages on Linux: `pi-workflow` (a
crate of this workspace; no other crate depends on it now) and nine packages from the registry
(`tdigests`, `rhai`, `rhai_codegen`, `smartstring`, `thin-vec`, `const-random`,
`const-random-macro`, `crunchy`, `tiny-keccak`). `Cargo.lock` loses `tdigests` and ten
dependency lines. The other packages stay in the lock file, because `pi-workflow` is still a
member of the workspace. `THIRD-PARTY-NOTICES` loses 5 entries (it lists the packages that ship on
any platform; `tui_notices.py check`: 984 of 984).

* chore(tui): lower the ceilings of the baseline after the cleanup of pi-shell

- `[gates.residue]`: `leader` 245 -> 243 and `x.ai` 304 -> 297. The deletions of #26 removed
  those lines; `check` has said "lower the ceiling" since then.
- `[graph.max_nodes]`: 980 -> 970 packages on Linux (the dependency commit of this branch).
- `pi-shell` `reference_passed`: 864 -> 838. The tests of `session::worktree` (24, Unix only),
  `session::helpers` (2) and `util::config::worktree` (1) are gone, and one test is new (a
  `[storage]` section is reported as unrecognized). The ten references add up to 8,488 (8,514
  before).

A local run on Linux (WSL2) gives: `check` with 0 warnings, 970 packages, `leader` 243 and `x.ai`
297; the ten test suites 8,487 passed and 1 failed (the known `tmux_probe` test of
`pi-pager-render`); `tui_notices.py check` 984 of 984.

* docs: record plan r16 (the merge of #25 and #26, and what the session store left in pi-shell)

- CHANGELOG.md: the entry for the third cleanup of pi-shell (15 files, three modules, the
  [storage] setting, 10 dependencies). It also says that the earlier entry left
  `[storage] cleanup_ttl_days` for this one.
- docs/baselines/tui.md: current baseline r16 (tests 8,488, graph 970, `leader` 243, `x.ai` 297)
  and a column r16 in the table of sizes. The numbers are from local runs (WSL2); the CI numbers
  of PR #27 follow in a later commit.
- docs/baselines/archive/tui-history.md: the r16 paragraph, with the CI numbers of `7d82c27`.
- docs/baselines/tui-support.md: made again (`pi-shell` has 7 dead units, 1,008 lines, not 9 and
  1,388).
- Plan: status r16, next steps, decision 10, D.2, D.8 (new candidates: 7 dead units, the crate
  `pi-workflow`, `Config.storage_mode`, about 88 files that no code loads), D.9, a new open item
  "races between tests", the index and the revision list.
- Execution log: section 10.19, B.11 (how to repeat the checks) and the r16 revision entry.

* docs: record the CI result of PR 27 in the plan, the log and the baselines

`CI` (run 39193752450) and `TUI CI` (run 39193752451) pass on 6bd7db5.
`TUI CI`: 8,488 passed, 0 failed, 21 ignored; 970 packages in the graph;
`leader` 243, `x.ai` 297; 0 warnings. These are the numbers that section
10.19 of the execution log expected, so only the "to be added" line and
the timing bullet change, plus one sentence in each r16 entry.

The cargo test job took 17 m 34 s and the gates job 4 m 24 s. The tests
themselves took about 54 s of that time.
zy1233 added a commit that referenced this pull request Oct 25, 2026
…to #31 (plan r19) (#33)

The owner asked for three things: merge the five open PRs (#27 to #31) into main with
`gh stack`, set the V0.6 milestone from the present state, and pause development for
1 to 2 days.

The merge is done (stack #32, `gh stack merge 32 --yes --squash`). main goes from
7d82c27 to b33017a with five squash commits. The tree of each commit is the tree of its
PR head. CI and TUI CI pass on b33017a (8,490 passed, 0 failed, 22 skipped).

The plan gets a new section "发布里程碑 V0.6": the scope (the 10 commits on main after
v0.5.0), six exit conditions with their state, the known issues (two are new), and what
is not in V0.6. It also gets the r19 status, the pause, and the fixed status of the
places that still said "waiting for merge" for #25 to #28. The execution log gets
section 10.22: the install of `gh stack` (the first download was cut short), the merge,
the check of the trees, CI on main, the milestone, the pause, and what is not verified.

The GitHub milestone v0.6.0 (#2) is created. It has no due date and no PRs, as v0.5.0.
No condition is met yet. Only documents change.
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.

1 participant