Skip to content

feat(cli): add --setup and offer it to ACP clients as a terminal login - #46

Merged
zy1233 merged 4 commits into
mainfrom
feat/acp-terminal-auth-setup
Oct 28, 2026
Merged

zy1233 merged 4 commits into
mainfrom
feat/acp-terminal-auth-setup

Conversation

@zy1233

@zy1233 zy1233 commented Oct 28, 2026 •

Copy link
Copy Markdown
Owner

Summary

Based on main (after #45 and #47). The first TUI CI run of this PR failed on an old order bug in pi-file-utils that has no link to this change; #47 fixed it.

first run: no model, no key
  before  Zed: chat error that names a missing package; initialize -> authMethods []; no fix in the client
  after   initialize   -> authMethods [pi-setup]            (only to a client that can run a terminal login)
          session/new  -> auth_required (-32000), no session
          client runs  -> <agent command> --setup            (terminal: provider, model, key)
          session/new  -> agent reads agent.toml + local.env again (only while the model is unusable)
                          -> a session with the new model
client announces                       authMethods
auth.terminal = true        (Zed)      [ {type: terminal, id: pi-setup, args: [--setup]} ]
_meta["terminal-auth"] only (older)    [ {id: pi-setup, _meta: {terminal-auth: {command, args, env}}} ]
nothing                     (zypi)     []   (as before)
0498544 feat(cli)  --setup               setup_command.py: the dialog; writes agent.toml and local.env
760d9e8 fix(cli)   first run             "No model is set ..." text; -p exits 1 and writes the reason to stderr
5dbf270 feat(acp)  terminal login        auth_methods.py, agent.py: the method, the gate, the read-again
1c2ea9f docs                             spec, AGENTS.md, READMEs, CHANGELOG

Evidence

  • Before: no [model]: the first turn says "Provider 'mock' needs init_chat_model from the 'langchain' package: pip install langchain" (install without langchain). -p prints an empty line and exits 0. initialize always answers authMethods: [].
    After: "No model is set. Run the agent with --setup, or add a [model] table to <agent.toml>." -p writes error: <reason> to stderr and exits 1. initialize answers the method in the table above.
  • 78 new tests (dialog and files 40, ACP side 32, the two fixes 6) and 2 in test_config.py. The wire test sends initialize as raw JSON: the SDK drops a plain AuthMethodTerminal from authMethods without an error, and the test fails on it. Mutation checks: 12 on the dialog, 11 on the ACP side, all caught.
  • One live agent process, played as Zed (Linux, pseudo-terminal): initialize -> one terminal method; session/new on an empty home -> -32000; the agent command with the method's args exits 0; session/new on the same process -> a session with the saved model; authenticate -> {}.
  • Whole packages/pi-agent-cli/tests (state of 5dbf270; the last commit is docs only): Windows 884 passed, 29 skipped; Linux 910 passed, 3 skipped. Other tests that import pi_agent_cli (pi-mcp ACP, tool annotations, TUI pty script): Windows 85 passed, 8 skipped; Linux 79 passed, 5 skipped (pi-mcp is not in that venv). Ruff check and format check pass for the whole repository. The full run is left to CI.
  • Not run inside Zed. What Zed does after the terminal exits, and whether Zed stable has first-class terminal login, are read from the Zed sources, not seen. Spec: docs/specs/2026-10-28-acp-terminal-auth-setup-design.md (§5).

Merge Danger

Door: two-way until the next release. After that, clients see pi-setup in authMethods.

Blast Radius: pi-agent-cli only: initialize, session/new, authenticate, the new --setup flag, and the exit status of -p (behavior change: a script that read 0 after a failed model call now reads 1). zypi is unchanged: it announces no auth, so it still gets authMethods: [] (the pager's e2e test runs with PI_USE_MOCK=1, which has no problem). The key goes in plain text to <PI_HOME>/local.env (mode 0600 on POSIX; the rights of the user folder on Windows). A running agent keeps its old key when --setup replaces it.

Open: the ACP registry question is registry#726. The npx or binary route is not started.

@zy1233

zy1233 commented Oct 28, 2026

Copy link
Copy Markdown
Owner Author

The red cargo nextest (Linux x86_64) check is not caused by this PR.

  • Failing test: pi-file-utils queue::tests::cleanup_orphans_uses_sidecar_age_for_pairs (queue_tests.rs:4550). It failed on both attempts. All other suites passed, including the pi-pager tests with the python_agent_e2e tests (the part that this PR changes).
  • Why the TUI CI runs for this PR: tui-ci.yml watches packages/pi-agent-cli/pi_agent_cli/**. This PR changes no Rust file.
  • Cause: the sweep of the upload queue deletes files while it walks read_dir, and the age of a pair is read from its sidecar file. If the sidecar is met first, the temp file loses its age and stays. The order of read_dir belongs to the file system, and the runner image changed (ubuntu-24.04 20261012.334.1, green, to 20261025.353.1, both failing attempts).
  • Fix: fix(tui): read every age of the queue janitor before it deletes a file #47 (separate PR on main). After it merges, push this branch again (for example, rebase on main): a re-run of the old job uses the old merge commit and fails again.

zy1233 added a commit that referenced this pull request Oct 28, 2026
#47)

The test `queue::tests::cleanup_orphans_uses_sidecar_age_for_pairs` of `pi-file-utils` was green
on every earlier TUI CI run, and failed on both attempts of the run for PR #46 ("expired-by-sidecar
temp removed"). The code did not change: queue.rs and queue_tests.rs are unchanged since ace8852,
and PR #46 touches no Rust file (the TUI CI runs for it because it changes `pi_agent_cli/**`,
which the pager's end-to-end tests drive). What changed is the runner image (`ubuntu-24.04`
20261012.334.1 on the last green runs, 20261025.353.1 on both failing attempts).

Cause. A pair is two files: the temp file and `<temp>.meta.json`. The age of a pair is the
`enqueued_at` in its sidecar (`pair_age`, which reads the sidecar file when it is called), and
the mtime only when there is no readable sidecar. `cleanup_queue_dir` deleted files while it
walked `read_dir`. When the walk met the sidecar of an expired pair first, it deleted the sidecar;
the temp file then had no readable sidecar, was judged by its own mtime (fresh in the test), and
stayed until a later sweep. When it met the temp file first, the temp file was removed. The order
of `read_dir` is the order of the file system: by hash on ext4 (with a seed that is set when the
file system is made, so a new image can list the same names in the other order), by creation on
tmpfs. So the same code and the same test passed on one file system and failed on another. The
comment of the function already says that pairing is decided before any deletion; the ages were
not.

Fix. `cleanup_queue_dir` now makes two passes: the first one decides every age (and sweeps
`scratch/`, as before) and collects the expired entries; the second one deletes them. Nothing
else changes: the same entries are deleted, with the same counts, and the lone-file counter still
uses the name snapshot.

Test. `cleanup_orphans_removes_expired_pairs_whatever_the_listing_order` makes 32 expired pairs,
half of them with the temp file written first and half with the sidecar first, so that whatever
order the file system lists them in, some pairs have the sidecar first. It fails on the old code
on every file system that was tried, and does not depend on the order of the host.

Checked:
- The real crate was not built here (it needs aws-sdk and tokio and is too heavy for this
  machine); the `cargo nextest` job of this PR will build and run it. Instead, the old and the
  new janitor code (queue.rs of origin/main and of this change: `QueueItemSidecar`,
  `sidecar_path_for`, `cleanup_queue_dir`, `pair_age`, `is_mismatched_queue_file`,
  `cleanup_scratch_subdirs`, `dir_size`), and both tests, were cut out of the real files by
  marker (nothing retyped; the two copies differ by exactly this diff) into one small crate and
  run with cargo 1.94.0 (the pinned toolchain), with the build in tmpfs.
- tmpfs (it lists the sidecar before the temp file): the old code fails the original test with
  the CI message, at the same assert; the new code passes it.
- ext4 (it lists the temp file first): the old code passes the original test (which is why it
  passed on the old image); the new code passes it.
- The new test fails on the old code on both file systems and passes on the new code on both.
- rustfmt --check with tui/rustfmt.toml: the same two hunks as on origin/main (top of queue.rs),
  none from this change.
- Runner image of the TUI CI runs: 2026-10-26 and 2026-10-27 (green): 20261012.334.1; both
  attempts of run 40620910485: 20261025.353.1. The order of `read_dir` on a runner cannot be
  seen from here, so the link between the image and the order is an inference; the reproduction
  above shows that the order alone gives this failure with this message.
- Not touched: scripts/tui_baseline.toml and docs/baselines/tui.md. The `pi-file-utils` suite
  goes from 216 to 217 tests, inside the 5 % band of `reference_passed`; the floor is unchanged.
Base automatically changed from feat/pi-agent-cli-lc-script to main October 28, 2026 05:36
…API key

A first run has no model and no key. The only way to set them was to write agent.toml and
local.env by hand, and an ACP client such as Zed cannot do that for the user. `--setup` asks in
a terminal and writes both files, so a client that can run a terminal command can offer it (the
later commits of this branch do that).

- setup_command.py: the dialog. A home with no model: provider (OpenAI, Anthropic, DeepSeek),
  model id, base URL (OpenAI and DeepSeek only), the name of the variable that holds the key,
  and the key, which is read without echo. A model that has no key: only the key is asked.
  A model that has its key: it asks before it replaces the key. It needs a terminal on stdin
  and stdout (exit 2 if not), and it saves nothing when the input ends or Ctrl+C is pressed.
- agent.toml: created, or a [model] table is added at the end of the file. The text that is
  already there stays to the byte, line ends included. A file that has a [model] table, a file
  that is not TOML, and a legacy config.toml (a new agent.toml would hide it from the agent) are
  refused before the first question. The name of the key variable is saved, not the key.
- local.env: the key line is put in by writing a file beside it and renaming it over local.env;
  other lines stay, a second line for the same name goes. Mode 0600 on POSIX, also when a
  temporary file of a killed run was left. A symbolic link stays a link.
- config.py: `read_local_env` returns the pairs without touching the environment (`load_local_env`
  uses it, same behaviour), and `MOCK_PROVIDER` names the provider of an empty configuration.
- __main__.py: the flag `--setup`, in the group of -p, --prompt-json and --prompt-file.

Checked:
- 42 new tests (40 in test_setup_command.py: dialog, bad answers, each state of the home, file
  bytes, modes, links, the flag in a subprocess; 2 in test_config.py). Windows: 124 passed,
  4 skipped (the POSIX ones); Linux (WSL): 127 passed, 1 skipped. Both runs include the tests of
  console scripts, extension trust and headless.
- 12 mutations of setup_command.py (the key printed, other env lines dropped, no early check, no
  legacy guard, no separator, line ends ignored, no confirmation, duplicate lines kept, temp file
  left, hint always shown, key check off, mode not tightened) are all caught; the mode one only
  on Linux, after the test for a leftover temporary file was added.
- The real input() and getpass() under a pseudo-terminal on Linux: the key is not echoed,
  local.env has mode 0600, agent.toml does not hold the key, the exit status is 0, and a second
  run asks before it replaces the key.
- Ruff check and format check pass for the package.
…t status on model errors

Two things that a run with an empty home showed.

1. A configuration that sets no model falls back to the provider "mock". LangChain has no such
   provider, so the turn failed with the text of a missing package ("init_chat_model ... pip
   install langchain"), and that sends the user the wrong way. `default_stream_fn()` now answers
   a model of the provider "mock" with an error event that says "No model is set. Run the agent
   with --setup, or add a [model] table to <agent.toml>." The scripted test LLM
   (PI_USE_MOCK=1) and the real providers are not touched; they take the same way as before.
2. `-p` printed an empty line and exited 0 when the model request failed, so a script could not
   tell a failure from an empty answer. `run_print` now writes `error: <reason>` to stderr and
   exits 1. Stdout still gets the text of an answer that came before the error, and nothing else.

- config.py: `no_model_message()`, one text for both paths.
- factory.py: `_say_when_no_model` wraps the LangChain stream function; `_error_stream` builds
  the stream of one error event the way a stream function must (it never raises).
- headless.py: the exit status and the stderr line.
- test_first_run_errors.py: 6 tests; the mock provider gets the message, a real provider still
  reaches the LangChain adapter, the scripted LLM is not touched, an ACP prompt shows
  "Error: <message>", `run_print` returns 1 with an empty stdout, and `python -m pi_agent_cli -p`
  exits 1 in a subprocess.

Checked:
- 4 of the 6 new tests fail on the code before this change, and all 6 pass with it.
- The tests that call `run_print` or `default_stream_fn()` (headless, headless trust, project
  resource trust, extension trust and notices, print shutdown, model reasoning, factory skills,
  and the new file): Windows 132 passed, 15 skipped (POSIX only); Linux (WSL) 146 passed,
  1 skipped.
- Ruff check and format check pass for the package.
…o it

An ACP client that can run a terminal login (Zed can) shows the authentication methods of an
agent when `session/new` fails with `auth_required`. The agent had `authMethods: []`, so a user
without a model or a key got a chat error and no way to fix it from the client. Now such a
client is offered `pi-agent-cli --setup`, and a new session that could not work is answered with
`auth_required`.

What a client announces decides the form of the method (auth_methods.py):
- `clientCapabilities.auth.terminal = true` (Zed declares it): a method of type `terminal`. The
  client runs the command that started the agent with `args` (`--setup`) added to it.
- only `_meta["terminal-auth"] = true` (older clients, and the registry validator): a method of
  the default type with `_meta["terminal-auth"]` = the command (`sys.executable -m
  pi_agent_cli --setup`) and `PI_HOME`, because the terminal has not the environment that the
  agent was started with.
- neither (the TUI zypi is one of these): `authMethods` stays `[]` and a new session opens as
  before; the first turn says what is missing (the previous commit).

agent.py:
- `initialize`: the method, as above. `authenticate` accepts the method id once the files are
  usable, answers `auth_required` while they are not, and `invalid_params` for any other id.
- `session/new`: when no model is set, or the variable named by `api_key_env` (and the variable
  that the provider's SDK reads, for openai, anthropic and deepseek) has no value, a client that
  can run the login gets `auth_required` (code -32000, a message that says what is missing and
  names the method), and no session is made. Other clients get their session.
- The agent reads `agent.toml` and `local.env` again at `session/new`, `session/load` and
  `session/resume`, but only while what it has cannot run a turn: `--setup` runs in the client's
  terminal while this process is up. A working configuration is never read again under a
  running agent. `session/load` and `session/resume` never refuse, because old history can be
  read without a model.
- Only an agent that reads its settings from the files offers this. A program that gives its own
  `config` or `stream_fn` (the tests, an embedding) is left alone.

config.py: `config_problem()` says why a configuration cannot run a turn (no network; the
scripted LLM `PI_USE_MOCK=1` has no problem), and `STANDARD_KEY_ENV`, which setup_command.py
uses for its default variable names too.

Checked:
- 32 new tests in test_acp_auth_methods.py: the form for each announcement (Zed, first class
  only, the registry validator, zypi, nothing, wrong values), the raw JSON of the `initialize`
  answer over stdio (the SDK drops an invalid item of `authMethods` without a word: a plain
  `AuthMethodTerminal` has no `type` and is not on the wire; this test fails on it), the gate of
  `session/new`, the read-again after `--setup`, `authenticate`, and `config_problem`.
- 11 mutations (plain terminal method, no capability gate, no PI_HOME in the legacy command, the
  wrong error code, `authenticate` for any id, `session/new` without the error, no read-again,
  always read again, `resume` that refuses, always offering, the provider's own variable
  ignored) are all caught.
- One live agent process, played as Zed would (Linux, pseudo-terminal): `initialize` answers one
  terminal method; `session/new` on an empty home answers -32000; the agent command with the
  method's `args` runs the dialog and exits 0; `session/new` on the same process gives a
  session whose model option is the model just saved; `authenticate` answers `{}`. This was not
  run inside Zed.
- The tests of the whole package pi-agent-cli: Windows 884 passed, 29 skipped; Linux (WSL) 910
  passed, 3 skipped.
- Ruff check and format check pass for the package.
…Es, changelog)

The three commits before this one add `--setup`, the terminal login in `initialize`, and two
fixes for a first run. This commit writes down what they do, why, and what was not checked.

- docs/specs/2026-10-28-acp-terminal-auth-setup-design.md: the problem, the facts that the
  design rests on (Zed adds the `args` of a terminal method to the agent command and does not
  call `authenticate` for it; the registry text says "replace"; the SDK drops an invalid item
  of `authMethods`; `zypi` announces no `auth` and must keep getting `[]`), the dialog, the two
  forms of the method, the gate and the read-again, the two fixes, the choices that were not
  taken, what was verified, and what was not (nothing was run inside Zed).
- packages/pi-agent-cli/AGENTS.md: the section "First run: `--setup` and the terminal login",
  two rows in the mode table, and rows for `setup_command.py` and `auth_methods.py`.
- AGENTS.md: the `pi-agent-cli` row points to the same topic.
- README.md and packages/pi-agent-cli/README.md: the first-run step for Zed, the command to run
  the dialog in a shell, and the way to set up by hand.
- agent.example.toml: a comment that `--setup` writes the `[model]` table and the key.
- CHANGELOG.md, Unreleased: Added `--setup` and the terminal login; Fixed the first-turn text
  and the exit status of `-p` (the second one is a behavior change for a script that read 0).

Checked:
- Every statement about the dialog and the gate was compared with the code of
  setup_command.py and agent.py (states, refusals, file handling, `authenticate`).
- The test counts in the spec match `pytest --collect-only` (40 + 32 + 6 = 78, and 2 more in
  test_config.py).
- What `zypi` announces was read in tui/crates/codegen/pi-pager/src/acp/mod.rs
  (`ClientCapabilities::new()` with `fs` and `terminal`, an empty `_meta`, no `auth`).
- Ruff check and format check pass for the whole repository. Only documentation changed in
  this commit, so no tests were run for it.
@zy1233
zy1233 force-pushed the feat/acp-terminal-auth-setup branch from 6397928 to 1c2ea9f Compare October 28, 2026 05:38
@zy1233
zy1233 merged commit 0a6aa4c into main Oct 28, 2026
7 checks passed
@zy1233
zy1233 deleted the feat/acp-terminal-auth-setup branch October 28, 2026 06:27
zy1233 added a commit that referenced this pull request Oct 28, 2026
…ust-cache key across image updates (#48)

* ci(tui): run only python_agent_e2e when only the Python agent changes

A change that touches only `packages/pi-agent-cli/pi_agent_cli/**` or
`packages/pi-agent-cli/tests/_tool_agent.py` started the whole TUI CI, because the end-to-end
tests of the pager (`python_agent_e2e`) talk to the Python agent. The TUI CI builds and runs ten
suites and takes 18 to 35 minutes. A change of the Python agent can change only that one test
target. The runs of PR #46 (`--setup`, Python only) took 25 to 35 minutes, and the first two
failed on a Rust test that the PR did not touch (#47).

Change.
- New workflow `tui-e2e.yml` (job `python_agent_e2e`): the two paths above and its own file
  start it. It runs `cargo nextest run --locked --no-fail-fast -p pi-pager --test
  python_agent_e2e`: the command that `scripts/tui_baseline.py` builds for that target of the
  `pi-pager` suite, with the terminal variables of `[test.env]`. The Python agent is installed
  as in `tui-ci.yml`, and `PI_E2E_REQUIRED=1` stays, so a missing Python is a failure.
- `tui-ci.yml` no longer lists the two paths. It starts for `tui/**`, the baseline script, the
  manifest and its own file, and it still runs these tests. A change of both runs both workflows.
- Cache: the new job restores the cache of the `test` job (same key) and never saves. A cache
  with the same key but fewer artifacts would stop the `test` job from saving the whole one,
  because a key is saved once.
- Six tests in `scripts/tests/test_tui_baseline.py` keep the two workflows in line: the paths,
  the command, the environment, the install lines (agent, nextest) and the cache key.
- AGENTS.md (root and `pi-agent-cli`), `docs/baselines/tui.md`, the comment in
  `scripts/tui_baseline.toml` and CHANGELOG [Unreleased] say it.

Checked:
- actionlint 1.7.12: no finding for the two workflows (none for the old `tui-ci.yml` either).
- `scripts/tests/test_tui_baseline.py`: 56 passed, 3 skipped on Windows; 59 passed on Linux
  (WSL). `scripts/tests`: 230 passed, 11 skipped on Windows. Ruff check and format check pass.
- 13 changes of the two workflow files (the agent path back in `tui-ci.yml`, a renamed path, a
  file that does not watch itself, a flag removed, another target, another TERM, `PI_HOME` set,
  `PI_E2E_REQUIRED=0`, another nextest version, another pip line, a cache that saves, another
  key, a changed event header): each one fails the test that is meant to catch it.
- Not checked here: a run on GitHub. The CI of this PR runs both workflows (it changes both
  files). The first PR that changes only `pi_agent_cli/**` shows that the TUI CI stays away.

* ci(tui): keep only the pinned Rust toolchain before rust-cache

rust-cache puts the version of every Rust toolchain on the runner into its
cache key. The image has its own `stable`, and an image update changes it
(1.99.0 to 1.99.1 on 2026-10-25). The key changed, no cache matched, and the
`test` job built everything again (35 minutes, 31 of them for the build).

Add the step "Keep only the pinned Rust toolchain" after the toolchain install
in every job of tui-ci.yml and in tui-e2e.yml. It makes the toolchain of
tui/rust-toolchain.toml the default and uninstalls the others, so the key does
not depend on the image. `add-rust-environment-hash-key: false` is not a fix:
it also removes the hash of Cargo.lock, so the key never changes and the cache
is never saved again.

Tests (scripts/tests/test_tui_baseline.py) keep the step equal in every job
and before the cache step, and keep the variables that the key hashes equal in
both workflows (the end-to-end job restores the key of the `test` job).
Docs: CHANGELOG.md, docs/baselines/tui.md.
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