Skip to content

feat(tui): resolve the start-up session by asking the agent (ADR1 PR 2) - #23

Merged
zy1233 merged 1 commit into
mainfrom
feat/adr1-resolve-startup
Oct 18, 2026
Merged

zy1233 merged 1 commit into
mainfrom
feat/adr1-resolve-startup

Conversation

@zy1233

@zy1233 zy1233 commented Oct 16, 2026 •

Copy link
Copy Markdown
Owner

Summary

zypi --continue, --resume and --session-id now ask the agent which session is meant. They no longer read the session files of the old Rust agent. This is PR 2 of ADR1 ("pure ACP"): it adds the new path and wires it in. The old code is still in the tree, unused; PR 3 deletes it.

 zypi --continue | --resume [id|title] | --session-id <id>
   app::run
     flags -> SessionStartupIntent                 (before any terminal; unchanged)
-    materialize_startup()                         (reads ~/.pi-python/sessions/**)
-    list_summaries() / pin_local_resume_target()
     terminal up, agent spawned, ACP connected
+    resolve_startup(tx, intent, cwd)              (session_resolve.rs; 30 s limit, cancel-aware)
+      --continue, --resume   session/list(cwd)    -> latest `updatedAt`
+      --resume <uuid>        session/list(all)    -> that id (stops when found)
+      --resume <title>       session/list(cwd)    -> exactly one title match
+      --session-id <id>      session/list(all)    -> the id must be free
+      no match / refusal: restore the terminal, stop the agent, exit 1 with a message
     event loop: session/load (or session/new with _meta.sessionId)

Both the resume picker and the start-up lookup now share one page walker (acp/session_list.rs): 100 pages at most, a repeated or empty cursor ends the walk, and the end is reported (Done, Stopped, Cut).

Behaviour that changes (decisions 4, 5 and 7 of the plan, answered by the owner on 2026-10-16):

--continue / --resume        was: newest *created* session      now: latest *activity* (updatedAt max, all pages)
--resume <unknown id>        was: error inside the TUI          now: exit 1 before the TUI is drawn
--resume <title>             was: never matched a Python one    now: matches `title` of session/list; two equal titles = error with a list
--session-id <id in use>     was: TUI stays up, "Invalid params" now: exit 1, "Session ID <id> is already in use."
--session-id <not a UUID>    unchanged text, but no double "Error: Error:" prefix any more

Evidence

  • Before: scripts/tui_pty/resume_matrix.py on main (12349d3): 11 cases pass, 2 are known gaps. --resume <title> in the session's directory exits 1 with "no session id or title matched". --session-id <id in use> shows "Session creation failed: Invalid params: ..." and the TUI stays up.
    After: resume_matrix.py --strict on this branch: 13/13 PASS. Case 8 (--resume <unknown id>) now needs exit status 1 and no replay, and the script checks it that way.
  • New tests: 25 unit tests for the resolver (session_resolve_tests.rs: a scripted agent over the wire; paging; latest activity and ties; rows without a time; a cut walk; UUID spelling; a title that two sessions share; an agent that cannot list), 4 for the 30 s limit and the cancel (bounded_resolve_* in app/mod.rs), 3 against the real Python agent (continue_and_resume_are_resolved_by_asking_the_agent, a_title_two_sessions_share_is_ambiguous, a_session_id_is_checked_against_the_agents_sessions). The 9 fetch_session_list_* tests of the picker protect the shared walker.
  • Full manifest on Linux (WSL2, PI_E2E_REQUIRED=1): pi-pager 5,675 passed, 0 failed (main has 5,643: +32 is the three groups above). python_agent_e2e 14/14. Compile: 0 warnings. tui_baseline.py check: all gates pass, warning ceiling 0, cargo check --workspace --tests clean. The whole manifest gave 8,730 passed and 2 failed: two timing tests in crates that this PR does not touch (the pi-shell lock heartbeat and the pi-pager-render tmux probe), run while the machine was compiling. Both pass when run alone.
  • CI on this head (506769b): TUI CI run 38889657279: 8,732 passed, 0 failed, 22 ignored (8,700 on main plus these 32). CI (Ruff, pytest 3.11, 3.12 and 3.13) is green.
  • Real zypi under a pseudo-terminal (scripts/tui_pty/): smoke.py, exit_matrix.py, print_exit.py, mode_probe.py, status_line_probe.py, net_audit.py all exit 0.
  • ruff check . and ruff format --check . pass. scripts/tests: 174 passed, 8 skipped on Windows.
  • Not solved: the Python agent writes the session file when session/new runs, so a session that never got a message exists with the title (no messages). --continue can pick it. The fix is lazy creation in the agent. I did not special-case it in Rust.
  • Cost: --session-id (and --resume <uuid> for an id that is not found) reads every page of session/list. The agent lists its directory again for each page, so the cost is the number of pages times the size of the directory, and it grows faster than the number of sessions. scripts/session_list_bench.py, column walk all, median of 3 (WSL2 / Windows): 100 sessions 42 ms / 90 ms; 1,000 sessions 1.3 s / 1.8 s; 5,000 sessions 28 s / 34 s. --continue reads only the pages of this directory: 0.6 s / 0.8 s at 1,000 sessions, 10 s / 12 s at 5,000. At about 5,000 sessions on Windows the lookup passes its 30 s limit, and the run ends with a message that names GROK_CONNECT_UI_TIMEOUT_SECS. The fix is on the Python side (list the file names only, no FileInfo): a prototype that I did not commit takes the Windows number at 5,000 sessions from 34 s to 3.6 s. It is plan decision 18; this PR does not do it.
  • Not verified: macOS and Windows builds of zypi; a real model turn after a resume.

Merge Danger

Door: two-way

Code only. A revert brings back the disk-reading start-up. The old functions are still in the tree until PR 3, so the revert is a one-line change in app::run.

Blast Radius: start-up of --continue, --resume and --session-id

Other start-ups (zypi, zypi -p, zypi export) do not call the new code. A bug in the lookup would stop a resume with exit 1 and a message. It cannot damage a session: the lookup only reads session/list. The docs for this change (plan r13, execution log section 10.16) come with PR 3, so the comments that point to section 10.16 point to a section that does not exist until then.

zypi looked for the session of --continue, --resume and --session-id in the
session files of the old Rust agent. The Python agent writes another layout, so
five cases of scripts/tui_pty/resume_matrix.py did not hold. Now zypi starts the
agent first and asks it over ACP: session/list (page after page) and
session/load. It reads no session file for these flags.

- acp/session_list.rs: one page walker (cursor, repeated cursor, page cap) for the
  /resume picker and for the start-up lookup.
- app/session_resolve.rs: resolve_startup(). --continue and --resume without a
  value take the latest activity (decision 4). --resume <id> reads every
  directory and loads in the directory of the session. --resume <title> matches
  in this directory. --session-id reads every page and refuses a used id. A miss
  ends the run with exit status 1 (decision 5).
- app/mod.rs: run() resolves after the connect, within the connect limit. On a
  failure it restores the terminal and stops the agent, as a failed connect does.
- resume_matrix.py --strict: 13 of 13 pass. Cases 5 and 11 were known gaps, and
  case 8 now needs exit status 1.
- session_list_bench.py: a "walk all" column, the cost of the --session-id check.

The old readers of the session files stay for now. The next change deletes them
(plan decisions 2 and 7).
@zy1233
zy1233 merged commit 7e0ca5a into main Oct 18, 2026
7 checks passed
@zy1233
zy1233 deleted the feat/adr1-resolve-startup branch October 18, 2026 15:36
@zy1233
zy1233 restored the feat/adr1-resolve-startup branch October 18, 2026 15:37
zy1233 added a commit that referenced this pull request Oct 18, 2026
@zy1233
zy1233 deleted the feat/adr1-resolve-startup branch October 18, 2026 15:38
zy1233 added a commit that referenced this pull request Oct 19, 2026
…n 17) (#25)

* feat(agent): write a session file with its first message (plan decision 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.

* test(tui): check the late session file at the TUI level (plan decision 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).

* test(agent): close two gaps that a mutation run found (plan decision 17)

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.

* docs: record plan decision 17 (the session file is written at the first 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.
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