Repository navigation
feat(cli): add --setup and offer it to ACP clients as a terminal login - #46
Merged
Merged
Conversation
Owner
Author
|
The red
|
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.
…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
force-pushed
the
feat/acp-terminal-auth-setup
branch
from
October 28, 2026 05:38
6397928 to
1c2ea9f
Compare
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.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Based on
main(after #45 and #47). The first TUI CI run of this PR failed on an old order bug inpi-file-utilsthat has no link to this change; #47 fixed it.Evidence
[model]: the first turn says "Provider 'mock' needs init_chat_model from the 'langchain' package: pip install langchain" (install withoutlangchain).-pprints an empty line and exits 0.initializealways answersauthMethods: [].After: "No model is set. Run the agent with --setup, or add a [model] table to <agent.toml>."
-pwriteserror: <reason>to stderr and exits 1.initializeanswers the method in the table above.test_config.py. The wire test sendsinitializeas raw JSON: the SDK drops a plainAuthMethodTerminalfromauthMethodswithout an error, and the test fails on it. Mutation checks: 12 on the dialog, 11 on the ACP side, all caught.initialize-> one terminal method;session/newon an empty home -> -32000; the agent command with the method'sargsexits 0;session/newon the same process -> a session with the saved model;authenticate->{}.packages/pi-agent-cli/tests(state of5dbf270; the last commit is docs only): Windows 884 passed, 29 skipped; Linux 910 passed, 3 skipped. Other tests that importpi_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.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-setupinauthMethods.Blast Radius:
pi-agent-clionly:initialize,session/new,authenticate, the new--setupflag, and the exit status of-p(behavior change: a script that read 0 after a failed model call now reads 1).zypiis unchanged: it announces noauth, so it still getsauthMethods: [](the pager's e2e test runs withPI_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--setupreplaces it.Open: the ACP registry question is registry#726. The npx or binary route is not started.