From f09da3e16bc6a83a6038c0358b2e05d36af2b8fe Mon Sep 17 00:00:00 2001 From: Benjamin Borbe Date: Sun, 27 Sep 2026 00:02:27 +0200 Subject: [PATCH] fix: give the Pi runner a session identity, so continuity actually happens MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit PersistSession only omitted --no-session, which governs whether pi writes the transcript. Reading it back is a different flag — so an agent configured for continuity saved every conversation and remembered none of them: a two-prompt proof returned the first answer and then "No token was previously requested to be remembered", which is what a fresh session says. PiRunnerConfig.SessionID now passes pi's --session-id, creating the session on first use and continuing it thereafter. --session-id rather than --continue deliberately: it creates the session when it is missing, so a brand-new agent's first prompt behaves exactly like its thousandth, where --continue would be resuming nothing. Left empty for a task-routed agent, whose runs are unrelated tasks that must not be joined into one identity. --- CHANGELOG.md | 4 ++++ pi/pi-runner-config.go | 18 ++++++++++++++++++ pi/pi-runner.go | 8 ++++++++ pi/pi-runner_test.go | 18 ++++++++++++++++++ 4 files changed, 48 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 95abf2f..5fa1e7a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Please choose versions by [Semantic Versioning](http://semver.org/). * MINOR version when you add functionality in a backwards-compatible manner, and * PATCH version when you make backwards-compatible bug fixes. +## Unreleased + +- fix: give the Pi runner a real session identity, so a long-running agent actually remembers its conversation. `PersistSession` only omitted `--no-session`, which governs whether pi **writes** the transcript — reading it back is a different flag. So an agent configured for continuity saved every conversation and remembered none of them: a two-prompt proof returned the first answer and then *"No token was previously requested to be remembered"*, which is precisely what a fresh session says. `PiRunnerConfig.SessionID` now passes pi's `--session-id`, creating the session on first use and continuing it thereafter. `--session-id` rather than `--continue` deliberately: it **creates the session when it is missing**, so a brand-new agent's first prompt behaves exactly like its thousandth, where `--continue` would be resuming nothing. Left empty for a task-routed agent, whose runs are unrelated tasks that must not be joined into one identity. Worth recording how this got through: the plan that scoped the work said *"runs `pi --print --mode json --no-session`, so session storage is never written. The delta is dropping `--no-session`."* The first clause is true and the second does not follow — **writing a session and continuing one are different flags** — and nothing exercised the difference until a second prompt was sent to the same pod. + ## v0.90.3 - fix: read the answer out of the `message_end` event the current pi CLI emits, not only the `agent_end`/`message_update` pair older builds used. pi is installed **unpinned** in the `agent-pi` image and its event vocabulary has already changed once: a `v0.4.0` agent-pi pod produced `{"type":"message_end","message":{...,"role":"assistant",...}}` followed by `{"type":"agent_settled"}`, and `extractEventText`'s two-case switch matched neither — so a run that had answered correctly came back as `no result found in pi CLI output`. **The symptom points at the model and the defect is in the parser**, and since only a real prompt through the runner exercises the path at all, it survived: the fleet's working agents run older images, and the recently-built ones had never been prompted. The `message_end` case carries a **role guard**, because that event fires for every role — without it a user or system message would be returned as the answer, and a run that failed would look like a successful echo of its own prompt. Specs cover both vocabularies and the role guard, using the file's existing `pi`-shim pattern. diff --git a/pi/pi-runner-config.go b/pi/pi-runner-config.go index bcb6c19..d3a6b7a 100644 --- a/pi/pi-runner-config.go +++ b/pi/pi-runner-config.go @@ -27,5 +27,23 @@ type PiRunnerConfig struct { // let a later run resume the previous task's conversation from the shared // ~/.pi/agent/ volume. Set it only for a long-running identity agent, where // continuity across prompts is the point. + // + // Persisting is necessary but **not sufficient** for continuity. `--no-session` + // governs whether pi *writes* the transcript; reading it back is a different + // flag. Setting this alone yields an agent that saves every conversation and + // remembers none of them — which is exactly what happened: a two-prompt proof + // returned the first answer and then "no token was previously requested". PersistSession bool + + // SessionID, when set, passes pi's --session-id: one stable identity whose + // transcript is created on first use and continued on every later run. + // + // This is the flag that makes continuity real — see PersistSession, which only + // stops pi discarding the transcript. It is `--session-id` rather than + // `--continue` because it **creates the session when it is missing**, so the + // first prompt of a brand-new agent behaves exactly like the thousandth; + // `--continue` would be resuming nothing on that first run. + // + // Leave empty for a task-routed agent, where each run is an unrelated task. + SessionID string } diff --git a/pi/pi-runner.go b/pi/pi-runner.go index 8a4bec0..b30d8bd 100644 --- a/pi/pi-runner.go +++ b/pi/pi-runner.go @@ -101,6 +101,14 @@ func (r *piRunner) buildCommand(ctx context.Context, prompt string) *exec.Cmd { args = append(args, "--no-session") } + // Continuity needs this *as well as* PersistSession. `--no-session` only + // controls whether pi writes the transcript; reading it back is a separate + // flag, and without one every prompt starts a fresh session — an agent that + // remembers nothing while saving everything. + if r.config.SessionID != "" { + args = append(args, "--session-id", r.config.SessionID) + } + if r.config.AllowedTools != "" { args = append(args, "--tools", r.config.AllowedTools) } diff --git a/pi/pi-runner_test.go b/pi/pi-runner_test.go index 8eb067f..207234f 100644 --- a/pi/pi-runner_test.go +++ b/pi/pi-runner_test.go @@ -96,6 +96,24 @@ printf '{"type":"agent_end","messages":[{"role":"assistant","content":[{"type":" Expect(err).NotTo(HaveOccurred()) Expect(result.Result).NotTo(ContainSubstring("--no-session")) }) + + It("passes --session-id when one is configured", func() { + // Persisting alone does not give continuity: --no-session governs whether pi + // *writes* the transcript, and --session-id is what reads it back. + runner := pi.NewRunner(pi.PiRunnerConfig{PersistSession: true, SessionID: "identity"}) + result, err := runner.Run(ctx, "test") + Expect(err).NotTo(HaveOccurred()) + Expect(result.Result).To(ContainSubstring("--session-id identity")) + }) + + It("passes no --session-id when none is configured", func() { + // A task-routed run must not be pinned to one identity: each run is an + // unrelated task, and a shared session id would join them. + runner := pi.NewRunner(pi.PiRunnerConfig{}) + result, err := runner.Run(ctx, "test") + Expect(err).NotTo(HaveOccurred()) + Expect(result.Result).NotTo(ContainSubstring("--session-id")) + }) }) var _ = Describe("piRunner event vocabulary", func() {