AGENTS.md · git:20260924.e0aecd7 · 2026-09-24 · sha256 94bbea44dd5196ba
AGENTS.md git:20260924.e0aecd7A
Immutable. This exact content never changes and is served at /api/v1/blob/94bbea44dd5196ba.
# Working on local-operator
Notes for agents (and humans) changing this codebase. This file covers the
things that are easy to get wrong here and expensive to discover later. For
what the rewrite set out to do see `docs/REWRITE.md`; for the evidence behind
each round see `docs/VERIFICATION.md`.
## Environment
```sh
cd ~/local-operator
ISO=$(mktemp -d) # every block in this file makes its own; see the note below
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" \
PATH="$PATH" TERM=xterm-256color \
.venv/bin/python -m pytest tests/unit -q # 22865 tests collected; a full run is 40-55 min under fleet load
```
**That comment's range is the point: a full local unit run is 40-55 minutes here,
not "minutes".** The suite is about 108 test-minutes of serial weight — **82.3% of
it under `tests/unit/tui`** — and it runs beside ~25 concurrent agent sessions on
this host, so the same command lands anywhere in that range (and it is the reason
`tests/durations.json` exists as the sharder's input). Do not treat the whole suite
as a routine inner-loop step: run the targeted command for what you changed (see
"Scoping the inner loop" below, and note which jobs still narrow), and let CI's
five shards be the whole-tree run.
**Every pytest invocation in this file is isolated, and that is not decoration.**
Each block also carries its own `ISO=$(mktemp -d)`, because the variable lives in
the shell that created it: a block that borrows another's expands to an *empty*
`HOME`, and Python 3.14 treats an empty `HOME` as `/` rather than as unset, so
the isolated roots land on `/.local-operator*` — a confusing `OSError` at best,
and a real root-level store on a host where `/` is writable. Copy one block, not
half of two.
The suite constructs real `ConfigManager`s, so a run without a redirected `HOME`
reads the operator's live `~/.local-operator/config.yml`, and the home-derived
roots beside it (cache, agent home) follow that same home. *Writing* to it is the
part that has done damage — a run in September 2026 collapsed an operator's
`subagents.models` tiers to `{}`, which disabled a model tier and killed the
background jobs that depended on it — and that write path is now closed
(`tests/unit/test_config.py::test_the_migration_has_exactly_one_caller_and_marking_has_two`
pins its only caller), but the read path is not, and the next write path nobody
has thought of starts from the same read. `LOCAL_OPERATOR_CONFIG_DIR` alone
does not cover this: the override relocates the config dir, and the logs that
deliberately follow it, but not the cache or the agent home, which derive from
the home directory independently — see
[Isolating a run](#isolating-a-run-local_operator_config_dir-alone-is-not-enough)
for why, and for the `CMUX_*`/`LOP_*` variables `env -i` also strips.
The `REAL CONFIG CHANGED DURING THIS TEST RUN` guard is a **post-mortem, not a
seatbelt**: it runs in `pytest_sessionfinish`, after the write. If you see it in
a log, the damage is done — restore the config from a backup and treat every
number that run produced as void. It is also armed only on the runs that need
it: it resolves the store from `LOCAL_OPERATOR_CONFIG_DIR`/`HOME`, so the fresh
`$ISO/.local-operator/sessions` above does not exist and it stands down — the
snapshot is never taken and its `shutil`/`pathlib` destructive-op wraps are
never installed. The runs it covers are the un-isolated ones, which is exactly
the case it exists for.
**The suite caps its own parallelism.** `addopts` asks for `-n auto`, but the
root `conftest.py` implements xdist's `pytest_xdist_auto_num_workers` hook and
resolves that to a capped count (2-8) instead of the one-worker-per-core xdist
would otherwise pick. It has to sit at the rootdir: the hook runs before the
`tests/` conftest is loaded, so a copy under `tests/` is never called.
The reason is that this repo is worked through many concurrent worktrees. On a
14-core / 36 GB host, one-per-core meant 14 workers and a measured **3,661 MB
of xdist-worker RSS** against 1,579 MB for the capped run; three suites at once
drove load average to 98-128 and consumed 6.1 of 7.2 GB of swap. Fewer workers were also *faster* there — an interleaved A/B
on `tests/unit/server` measured `-n 4` at 5.3-5.9s against `-n 14` at
7.8-11.3s, because the suite waits on the event loop rather than on CPU. The
cap is the smaller of a CPU share and a memory budget, so a machine already
under pressure from sibling worktrees backs off on its own. The budget divides
by **600 MB per worker** — a deliberately conservative envelope, not the measured
per-worker RSS: cleanly measured workers sit at 226-262 MB, but a worker's RSS
depends on which tests it draws, and the suite's known heavy corner
(`tests/unit/tools/test_eval_tool.py`, which spawns real kernel subprocesses,
`tests/unit/evaluation/adapters/osworld`, which builds the shipped adapter wheel
and a real copied interpreter, and the boot-bound
`tests/unit/tui/test_slash_echo.py`) measured a peak worker **process tree** of
**441.5 MB** at `-n 6` on 2026-09-14 with a peak aggregate of 1,647.3 MB across
the six — so the charge is 1.36x over the heaviest draw *observed*, and the
corner it comes from is 3.4% of the tree, sampled every 1.5 s: that refutes a
400 MB charge but does not calibrate the suite. The worst single worker ever
observed is ~1,090 MB and is NOT covered by the charge; what bounds that case is
the aggregate, because a count of N is only ever produced on a host
with at least 1,200 x N MB of available memory (the budget is the smaller of
half of available and available minus the reserve) — an inequality over what the
host had *free* when it chose N, not over what N workers then claim.
**The cap was titrated on 2026-09-13/14 — it had been too tight, not too loose.**
Before this, at this host's chronic 4,500-5,500 MB of *available* memory, the
reserve below was binding on every run and resolved **3 workers** at
4,964-5,313 MB (**2** at 4,500-4,848 MB and **4** above ~5,472 MB) on 14 cores
that were 0-2.5% busy. Interleaved A/B waves of 3 concurrent instances of the
same boot-bound slice, at real fleet depth (11-13 sibling suites live),
per-instance wall time: **cap 3 → 313.3 s / 225.2 s; cap 6 → 188.9 s / 161.6 s /
188.7 s; cap 8 → 171.3 s** — 6 beat 3 in that run, and 8 beat 6 by only 9%. An
independent A/B (review round 1) reproduced the ordering but not the magnitude
(15-16%, with a different baseline), so quote the direction and never a headline
speedup percentage — the **cap-6-over-cap-3** percentage the waves above produced
is withdrawn as a claim, while their CPU deltas and the 9% for cap 8 over cap 6
are measured figures and stay. What ships is the reserve CAP moving
(`min(3072, total / 8)` → `min(2048, total / 8)` — the 1/8 fraction, and so every
small-host floor and the proportional protection a CI container gets, is
unchanged), which takes this host from the released constants' 2 / 2 / 3 / 3 / 3 / 4
to 3 / 4 / 4 / 4 / 4 / 4 at 4,522 / 4,848 / 4,964 / 5,140 / 5,313 / 5,555 MB
available: **two** workers at
4,848 MB, **one** at 4,522 MB and at 4,964-5,313 MB, none at 5,555 MB. The
**per-worker charge did NOT move**: an earlier revision halved it to 400 MB, and
the 441.5 MB peak tree above refused it. `_CPU_SHARE` and the 2..8 clamp did not
move, and the four documented CI runner shapes resolve exactly as they did
before.
**The hook says why it chose N, and does not divide by the fleet.** One stderr
line at startup names every term that produced the count:
```
pytest worker cap: 4 (bound by memory, cpu arm 7, memory arm 4, available 5,313 MB, reserve 2,048 MB, siblings 11)
```
Silence it with `PYTEST_QUIET_WORKER_CAP=1` (`=0`/`false`/`no`/`off` leave it on).
An operator override (`PYTEST_XDIST_AUTO_NUM_WORKERS`) gets its own one-line
report naming the override as the term that bound, because that is the path where
the count is least explicable otherwise. Neither line ever writes to stdout, which
`-q` output is parsed from. The `siblings` field is a read-only count of the
other live pytest suites (one `ps`, no lock file) and it is REPORTED ONLY:
dividing both arms by it was implemented and measured on 2026-09-13 and rejected
- on this host it floors every suite at 2 workers and took a boot-bound TUI
slice from 219.8/216.3/209.8 s to 338.4/314.5/308.1 s per instance at identical
CPU, i.e. it buys nothing and costs wall time. `conftest.py` carries the numbers
so it is not re-tried without them.
The memory probe reads *current* pressure, which is subtler than it sounds:
`psutil` is deliberately not a dependency, so macOS is read from `vm_stat`
(`free + speculative + file-backed`) and Linux from `MemAvailable`. Two
plausible-looking alternatives are both wrong and are documented in
`conftest.py` so nobody re-adds them. Counting `Pages inactive` as free reports
phantom headroom on a swapping machine (8,137 MB while the host had 452 MB
free); `File-backed pages` is the subset `vm_stat` itself marks clean and
cheaply reclaimable, so no invented discount fraction is needed. And subtracting
`vm.swapusage` looks like a pressure signal but is **cumulative** — macOS never
decrements it, so a host that swapped hours ago stayed pinned to 2 workers while
the OS reported 78% free. Every term used here is instantaneous and recovers on
its own. Any probe failure degrades to the CPU-only cap; the hook never raises.
Override it per session, or bypass it entirely:
```sh
ISO=$(mktemp -d)
# The isolation prefix is the one from ## Environment; only the worker control differs.
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" PATH="$PATH" \
TERM=xterm-256color PYTEST_XDIST_AUTO_NUM_WORKERS=12 \
.venv/bin/python -m pytest tests/unit -q # honoured unclamped
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" PATH="$PATH" \
TERM=xterm-256color \
.venv/bin/python -m pytest tests/unit -n 12 -q # explicit -n bypasses the hook
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" PATH="$PATH" \
TERM=xterm-256color \
.venv/bin/python -m pytest tests/unit -n0 -q # serialise for a debugger
```
**CI keeps every core.** The CPU share is skipped when `CI` is set: a hosted
runner is dedicated and single-purpose, so the contention the share protects
against does not exist there. Applying it anyway halved CI parallelism (a
4-vCPU runner resolved to 2 workers instead of 4), which is a regression paid on
every PR. The memory budget and the 2-8 clamp still apply on CI.
**One exception, and it is our own doing.** The bash tool sets `CI=1` on every
agent-run command so CLIs behave non-interactively, which made every agent-run
suite on a developer laptop look like a dedicated runner and take all 14 cores
— disabling the share exactly where it was needed. The tool now also exports
`LOCAL_OPERATOR_AGENT_SHELL=1`, and the hook denies on it, so `CI` still means
"dedicated runner" for every real provider. Deliberately a denylist: an
allowlist of provider variables (`GITHUB_ACTIONS` and friends) would silently
halve parallelism on every provider nobody remembered to add, which is the
regression above. If you are adding a new non-interactive variable to
`NON_INTERACTIVE_ENV`, check nothing else reads it as "dedicated machine".
The sibling marker `LOCAL_OPERATOR_AGENT_MAY_DELEGATE` is deliberately NOT in
that dict: it is written in three arms rather than on every command — `1` when
`ToolContext.may_delegate` (the session holds `task`); the empty string only when
the name really is inherited from the launcher and has to be cleared, so the
allowance cannot outlive the session it described; and NOT WRITTEN AT ALL
otherwise, so a session
that never had the allowance is not handed the spelling — the name is the
mechanism (`LOCAL_OPERATOR_AGENT_MAY_DELEGATE=1 lop exec` is a one-token
self-grant). So `NON_INTERACTIVE_ENV` is the wrong home for it, and
nothing reads it as a CI or "dedicated machine" signal — the guard in
`agent_shell.py` is its only consumer. See the section on what an agent may
start below.
**A memory reserve is also held back**, scaled per host (`min(2048, total / 8)`
MB), because the budget otherwise claims a fraction of what *remains* and
sibling suites converge toward zero free memory instead of toward a floor. Know
its actual reach before tuning it: the shape is `min(share, available -
reserve)`, so it binds only below **twice** itself (~4 GB free on a 36 GB box)
and is invisible above that — and this host spends hours at a time BELOW that
boundary, so here the reserve is the ordinary binding term rather than a rarely
armed safety net. That is exactly how 5 GB of free memory used to buy 3 workers,
and why the titration above lowered the reserve's cap (only its cap: the 1/8
fraction is unchanged, so the floors on small hosts are exactly what they were).
It is a floor under one suite's appetite, not a cap on the fleet: a cross-process
budget would be a truer lever for the fleet total, and is deliberately not built
(see `harness/group_reaper.py` on wedged `flock` holders propagating a freeze
between sessions). A fleet divisor was also measured and rejected — see
`conftest.py`'s module docstring before proposing one.
`--dist worksteal` is also in `addopts` — per-test
durations here vary by orders of magnitude, and the default `load` scheduler
pre-assigns chunks, leaving workers idle at the tail while one grinds through
the slow Textual pilot tests.
TUI tests need a colour-capable terminal, so run them with the environment the
suite expects:
```sh
ISO=$(mktemp -d)
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" PATH="$PATH" \
TERM=xterm-256color .venv/bin/python -m pytest tests/unit/tui -q
```
**A local failure CI does not have is usually your shell, and the fix belongs
in the fixture.** Tests that construct a third-party client inherit whatever
the developer exports. The OSWorld AWS tests scrub `AWS_PROFILE`,
`AWS_DEFAULT_REGION` and `AWS_REGION` in an autouse fixture for exactly this
reason — but botocore also reads `AWS_DEFAULT_PROFILE`, which the fixture
missed, so a developer with that exported gets 24 failures
(`ProfileNotFound: The config profile (sandbox) could not be found`) on a tree
that is green on CI. Confirm the diagnosis by unsetting the variable for one
run:
```sh
ISO=$(mktemp -d)
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" PATH="$PATH" \
.venv/bin/python -m pytest tests/unit/evaluation -q # -i also drops AWS_DEFAULT_PROFILE
```
Then add the missing name to the fixture's scrub list. Do **not** leave it as a
local workaround or a note in the PR: the next agent re-diagnoses it from
scratch, and a fixture whose docstring promises "no ambient configuration may
reach these tests" is simply wrong until it covers every variable the client
reads.
`tests/e2e` is a separate stage that drives the **assembled** application —
boot, a real turn through a real tool, and `/resume` — and it is **deselected
from the default run** (`-m "not e2e"` in `addopts`). Run it explicitly:
```sh
ISO=$(mktemp -d)
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" PATH="$PATH" \
TERM=xterm-256color .venv/bin/python -m pytest tests/e2e -m e2e -n0 -q
```
It exists because the whole unit suite was green while the TUI was completely
frozen (#401: a blocking `flock` in the MCP OAuth refresh lock deadlocked the
event loop on `/resume`). Two things about it are load-bearing rather than
stylistic, and both are documented at length in `tests/e2e/watchdog.py`:
- **Its failure mode is a hang, not an assertion.** The deadlock parks two
threads inside syscalls, so `asyncio.wait_for`, thread watchdogs and
signal-based timeouts all fail to fire — verified, not assumed. Only
`faulthandler.dump_traceback_later(exit=True)` survives it, because it runs
in a C thread and needs no GIL. A tripped watchdog kills the process and
writes every thread's stack to a file; `python -m tests.e2e.watchdog` prints
it back.
- **It is why the stage is deselected and runs `-n0`.** Under xdist a fired
watchdog would kill a worker carrying unrelated tests and report them as an
infrastructure error rather than as the freeze they are.
Most of it is headless (Textual's `run_test()` pilot, no window, no display),
but not all of it, and the exception is load-bearing: a few members drive the
real console script in a real **pty** — `test_terminal_close_survives_e2e.py`,
and `test_tui_boot_e2e.py`, which feeds the raw bytes a legacy X10 mouse
terminal sends. The pilot paints into an in-memory compositor and never reads
stdin, so a failure in the terminal INPUT path is invisible to a pilot-only
test; 0.54.37's production crash was exactly that (a strict UTF-8 decode on the
driver's input thread, raised by three raw coordinate bytes), and those pty
members are what makes it observable. It uses no API key, so its CI job
carries **no fork gate** — unlike `cli-sanity`/`server-sanity`, whose live-LLM
secrets force one. That is deliberate: the resume-liveness assertion is the
regression guard, so it has to run on every PR whose diff is not inert,
**including forks**, and the macOS leg below is mandatory. Change-scope gating
narrows it in exactly one direction — a diff confined to `docs/**` or to
`local_operator/mobile/web/**` does not run it — because nothing the stage
exercises reads either tree.
**It runs on a `[ubuntu-latest, macos-latest]` matrix, and the macOS leg is the
one that makes it a regression guard.** The deadlock is a macOS/BSD property —
`close()` blocks there behind a sibling `flock()`, and on Linux it returns in
microseconds. Measured, not assumed: the same probe reports `close_blocked=True`
on darwin and `False` on linux, and the resume test run against the pre-fix tree
(`80df237b^`) in `python:3.12-slim` reports `1 passed`. Linux still runs the
stage because boot, the write-turn artifacts and the transcript replay are
platform-neutral, but **do not drop the macOS leg** — without it this stage goes
green against the exact commit it exists to catch.
The `tui-e2e` job must not `need` the unit `test` job or `pip-audit`. It exists
because a green unit suite did not catch #401; gating it on `test` skipped the
freeze guard on every red unit run (observed on PR #426, which *fixed* a
deadlock while `tui-e2e` reported `skipping`). It needs only `changes` (the
scope classifier, which always runs), `lint` and `type-check` — cheap syntax
gates. A flake or a newly-published CVE must not disarm the macOS
resume-liveness assertion.
Its `tui` scope flag is deliberately **equal** to the unit matrix's `unit` flag,
and that equality is load-bearing: `tests/e2e/test_fork_e2e.py` and
`test_mcp_failure_reaches_the_viewer_e2e.py` import `scripts.*`, so a `scripts/`
change is an input to this stage, and a flag narrower than the unit matrix would
re-create the #426 disarm in a new costume. `tests/unit/test_ci_hygiene.py`
asserts the implication (`types(tui-e2e) => types(lint)`), not the comment.
Gates, all of which must be clean before a PR. These are the four command-shaped
jobs of `.github/workflows/ci.yml`, spelled as they are for a full-tree run:
```sh
.venv/bin/python -m flake8 .
uvx --from black==26.1.0 black --check .
uvx isort==5.13.2 --check .
.venv/bin/python scripts/run_bounded.py --timeout 1800 -- .venv/bin/python -m pyright --pythonpath .venv/bin/python .
```
The `pyright` line goes through a bounded, process-group-reaping wrapper: it is
whole-tree, exactly as before, but it cannot leave an analyzer running. See
"The local `pyright` gate is bounded and process-group-reaped" below.
**CI no longer runs all four over the whole tree on every PR.** Each job is
gated on a scope flag computed by `scripts/ci_scope.py`, which classifies the
diff and skips work a change provably cannot affect — a one-line `docs/**` edit
runs two cheap checks instead of sixteen. The local equivalent of the whole CI
job set is:
```sh
make check-changed # scripts/ci_scope.py --since <merge-base origin/main> --run
```
It selects its gates from the SAME module the workflow's `changes` job runs, so
the local answer and CI's answer cannot drift into two opinions, and it runs
only the jobs whose flags are true for your diff. **Four gated jobs are never
part of a local run** — `filesystem-boundaries-windows` (native Windows
junction semantics), `cli-sanity` and `server-sanity` (live-LLM: they need
`OPENROUTER_API_KEY` and spend real tokens), and `pip-audit` (its CI shape is
`pypa/gh-action-pip-audit`, which installs the project and then audits it inside
a hermetic venv that cannot be reproduced here). `--run` prints each exclusion
with its reason, so a local green is not evidence about those four — the audit in
particular, since a dependency-touching PR is exactly where a local green and a
red audit can coexist. Use the four commands above by hand when you want the
whole-tree form regardless — a release PR, or a diff that touches `.github/**`,
both of which set every flag true.
Do **not** invoke `.venv/bin/black`, `.venv/bin/flake8`, `.venv/bin/isort`, or
`.venv/bin/pyright` directly. Those console scripts carry a shebang baked in
at install time; after a parallel-agent worktree that owned the venv is
deleted they fail with `bad interpreter` and exit **126**. `rc=126` is
swallowed by a pipeline (`cmd | tail` reports `tail`'s 0), so a lint or
format gate that never actually ran looks green. `python -m` and `uvx`
cannot hit that path — that is why the commands above are spelled that way,
and why `make lint` / `make format` / `make type-check` go through them
rather than the console scripts.
Narrowing the last two to `local_operator tests`, or passing `--profile black`
instead of letting isort read the repo's own config, checks something CI does
not: both combinations pass on a file that CI then rejects. An unsorted
function-local import reached CI exactly that way.
Editing `exclude` under `[tool.pyright]` in `pyproject.toml`? It **replaces**
pyright's built-in defaults rather than extending them, so always restate
`"**/node_modules"`, `"**/__pycache__"` and `"**/.*"` alongside whatever you
are adding. Dropping `**/.*` makes pyright follow the `.venv` symlink every
worktree has and type-check all of site-packages — 29466 files and a 30-minute
run instead of 566 files and about 15 seconds. CI never creates a `.venv`, so
it stays green while every local run of the gate becomes unusable.
The venv is uv-managed and has the package installed **editable**, so source
edits are live. After a pull that changes dependencies:
```sh
uv pip install -e ".[all,dev]" --python .venv/bin/python
```
### Scoping the inner loop, and the whole-tree triggers that stop it
`scripts/ci_scope.py --run` decides at JOB granularity; the fourth section of
that module narrows the *local* commands of FOUR of those jobs — `lint`,
`type-check`, `test`, `tui-e2e` — at FILE granularity. What each one becomes for a diff of one changed module:
| job | whole tree | scoped |
|---|---|---|
| `lint` | `flake8 .`, `black --check .`, `isort --check .` | the same three tools over the changed files |
| `type-check` | `pyright … .` | `pyright … <the changed files plus their transitive reverse dependents>` |
| `test` | `pytest tests/unit -q` | `pytest <the test files that transitively import the change> -q` — **only when nothing below stops it**, which on this tree is never, today |
| `tui-e2e` | `pytest tests/e2e -m e2e -n0 -q` | the same, over the e2e files that reach the change — and nothing at all when no e2e file does |
`type-check` narrows because a file-list `pyright` reports diagnostics **only for
the files it is given** (measured: an error in an imported but unlisted module is
not reported, while a signature change in a listed file's *dependency* IS
reported in the listed file). The list is therefore the changed files plus every
file that transitively depends on them, which is complete for "what this change
can break", and it is also what the run costs. Its protocol-sync step has no file
list to narrow and runs unchanged, which the report says out loud.
The selection comes from a STATIC import graph (the ASTs of `local_operator/`,
`tests/` and `scripts/`; nothing is imported). It is an under-approximation of
"what this change can break", so it is only allowed to run when the
approximation is safe — **the rule is a whitelist, and everything it does not
name runs the whole-tree command and prints the path that stopped it.** A path
narrows a gate only if it is a `.py` the graph covers, or documentation no gate
reads. Named barriers:
* any `conftest.py`, at any level;
* `pyproject.toml`, `uv.lock`, `Makefile`, `.flake8`, `setup.cfg`, `tox.ini`;
* anything under `.github/`, `extension/` (the suite reads it by PATH — see
`tests/unit/browser_bridge/test_extension_version_skew.py` — so no import edge
exists), or the vendored
`benchmarks/osworld_v2_adapter/src/evaluation_examples/`;
* any package `__init__.py` (module surface, and pytest's collection semantics);
* `local_operator/cli.py` and `local_operator/__main__.py` — the entry points;
* `tests/helpers/**` — a shared helper tree no import edge is a contract for;
* package data and test data (a `.tcss`, a `.md`, a `.json` under
`local_operator/` or `tests/`): read at run time, importing nothing;
* a `.py` the graph cannot place — deleted, outside those three trees, or inside
a tree the graph could not parse — and any `.pyi`;
* **any changed file inside the app BOOT CLOSURE**: `local_operator/cli.py`,
`local_operator/tui/app.py`, `local_operator/session_factory.py` and everything
they transitively import — 452 of the 512 `local_operator/**` modules here. Every
test that imports the app reaches those files, which is not a reference-graph
question at all, so that job runs whole-tree whatever the selection would have
been;
* **any unresolved reference the graph is carrying** (see below) — one is enough,
and it stops the `test`/`tui-e2e` jobs for the whole diff;
* anything else that is neither such a `.py` nor a `.md` outside those two trees.
* the trees the GRAPH parses are wider than the trees the suite imports: a gate reads
`benchmarks/osworld_v2_adapter/**`, so those files are parsed too (they are
`ANALYZED_TREES`, and a change inside the covered trees can break them). Leaving
them out made "complete for what this change can break" false for 20 files, 9 of
them importers of `local_operator.*` (9 files across 8 modules; #1322 QA round 1,
Q-1; re-derived here — the round-1 grep did not descend into `providers/`). The list is
literal, not derived from pyright's own file set, and one analyzer file is still
outside it (`extension/scripts/generate-icons.py`, which imports nothing of ours) —
a new top-level tree that imports `local_operator` reopens the class until it is
named in `ANALYZED_TREES`.
* a selection over **25% of the suite's measured weight** or **50% of its test
files** (`tests/durations.json` supplies the weights; an unreadable manifest
holds the file arm to the weight fraction rather than loosening it), or a
`type-check` file list whose import closure would reach **more than 50% of the
program** — naming most of the tree is the whole-tree command with extra steps.
**The figures in this section are a measurement, not a property of the tool:** they
move as the tree moves, so they carry the date they were taken (2026-09-20) and the
command that takes them again. Every round of this PR re-derived them, and every round
found the previous round's numbers stale by a few files: 503 `local_operator` modules
and 576 unresolved sites, then 508 and 597, then **512 and 607** here. Re-derive with
`python -c "import sys; sys.path.insert(0, 'scripts'); import ci_scope; g = ci_scope.build_import_graph(__import__('pathlib').Path('.')); print(len(g.files), len(g.boot), len(g.unresolved_sites()))"`
rather than quoting them forward.
**Where it pays, and what the fail-closed rule costs — measured, not assumed.**
This suite's tests import the assembled app, so **88% of `local_operator/**` is
inside the boot closure** (452 of its 512 modules) and a change there is a barrier
by construction. What is left, and what survives the unresolved-reference rule:
| job | narrows for | whole-tree when |
|---|---|---|
| `lint` | any diff: the three tools over the changed files (per-file by construction, so no selection can be incomplete) | a barrier path stopped the plan, or the diff has no lint input |
| `type-check` | the changed `.py` files plus every file that transitively depends on them, when the change is outside the boot closure and its import closure stays under 50% of the program | the change is inside the boot closure, the closure arm fires, or the diff touches a path the graph does not cover |
| `test`, `tui-e2e` | **nothing on this tree today.** The classes are implemented — changed test files, `scripts/**`, modules outside the boot closure — and they narrow the moment nothing is unresolved; measured, the graph carries **607 unresolved references** (475 directory scans + 132 names), **423 of them in files inside the test universe**, so the rule fires for every diff |
Two measurements, because the distinction is the whole design:
| a change to | `lint` | `type-check` | `test` |
|---|---|---|---|
| `scripts/shard_tests.py` | 1 file | 4 files | whole tree — the tree's own readers scan for it |
| `local_operator/agents.py` (inside the boot closure) | 1 file | whole tree | whole tree (boot closure) |
**What this buys TODAY, stated plainly: the barrier and per-file lint/format
scoping — not a narrower pytest run.** `test` and `tui-e2e` are whole-tree for every
diff on this tree (607 unresolved sites arm them out), so the file-level pytest
selection is a mechanism that is INERT until that count drops; the review round could
not construct a diff that narrows either job. What is live is: the barrier itself
(measured load-bearing — neutralising `unresolved_sites` lets a real green-while-red
through, `test: commands=[]`, rc 0, while the probes are `1 failed, 1 error`), the
`type-check` closure narrowing, and `lint` over the changed files, where the win is
the whole gate's file count (measured on this fleet: flake8 49 s + black 137 s +
isort 25 s whole-tree against 1 s + 14 s + 1 s over a two-file change). The realised
`type-check` win is the large one and it is measured end to end: **838.5 s whole-tree
against 16.0 s scoped** on a change whose file list is 14 files (QA round 1's
measurement, on this fleet — load-affected, as every wall time here is: the host sat at
200-373 during it, and the same round's whole-tree pyright read 269 s against the 508 s
recorded elsewhere in this section). The graph build is the price of admission for any
non-barrier diff — an envelope rather than a number, because the same tree reads
**37.9 s** on one run and **52.7 s** on another here at load average 370, with earlier
readings up to 93 s, which is why `lint` is decided
before the graph is built at all and why the report prints the graph's own timing. Do
not read the class table below as a claim that pytest narrows here.
**The unresolved-reference rule is unconditional, and that is deliberate.** A
reference the parser can see and cannot place — a computed name
(`importlib.import_module(name)`), a scan whose directory the evaluator cannot
resolve, whatever its pattern — could read the file you changed, and the tool has
no sound way to decide that it could not. Trying to decide anyway is exactly what
produced **four consecutive rounds of false greens on this feature**: a `.py`
reached by path; the `/x.py`, `./x.py` and `../x.py` spellings; a variable-held
scan receiver, then `iterdir()` and a multi-argument `Path()`; and the
`.py`-only "arming" policy that each of those rounds came through. So the rule
refuses to narrow rather than pricing the risk, and the barrier prints the sites
that stopped it.
**An edge is not always an import.** The suite also reaches files by NAME: a test
runs `scripts/visual_gallery.py` through `sys.executable` + a path, and several
modules are spawned as `-m local_operator.x`. No import statement records either,
so an imports-only graph selected NOTHING for such a change and a local run
printed `all selected gates passed` while CI's `test` job was red (the blocker on
#1322). A string constant that resolves to a covered file — a path (`scripts/x.py`,
`./x.py`, `../x.py`, `~/x.py`, an absolute path, or a bare basename, which is
also what an f-string like `f"{ROOT}/scripts/x.py"` leaves behind) or a dotted
module name — is therefore an edge in
the REVERSE direction: when the named file changes, the file that NAMES it is
selected, and so is anything that imports that namer. It is reverse-only on
purpose — a name is not a static import, so it must not inflate the `type-check`
cost estimate. Two tests guard the class rather than one instance of it:
`test_a_repo_python_file_a_test_names_is_always_selected` walks the LITERALS the
real tree's tests carry (not the resolver's output), and
`test_every_spelling_of_a_repo_path_is_collected_and_resolved` asserts each
spelling above — collection and resolution — so a narrower regex, a stricter
resolver or a lost edge fails a test rather than a CI job.
A conftest that NAMES a changed file is a namer pytest runs and nothing imports,
so it is not in the test universe: seeding on it alone selected nothing. Its
subtree is selected instead, which is the scope pytest itself gives it.
**A glob is a reader, and reads have the same edge.**
`tests/unit/tui/test_visual_gallery.py` iterates `(ROOT / "scripts").glob("*.py")`
and `tests/unit/tui/test_visual_capture.py` the same directory through a
variable, so in the REFERENCE MODEL a one-token change to any of the 201 covered
`scripts/*.py` selects both — before the scan edge it selected NOTHING and the
local run printed `all selected gates passed` while CI's `test` job failed (QA
round 2, Q-1; the variable-held reader was round 3's blocker, printed but not
armed). That is what the model does; the shipped plan runs the pytest jobs
whole-tree here (the barrier row above), so today it is the model's completeness
that this paragraph is about, not the command you get.
`glob`/`rglob`/`iterdir`/`listdir`/`scandir`/`walk` are therefore read edges.
The scanned directory is the one the receiver's expression denotes — path
literals, `__file__`, `.parent`, `.parents[N]`, `.resolve()`, and up to four
`name = <expr>` hops, so `SCRIPTS = ROOT / "scripts"` places — and the pattern is
matched against the repo-relative path, so `*/*.py` is exactly one level and
`**/*.py` any depth. Placement is all-or-nothing: a receiver whose WHOLE literal
chain is not a directory (`scripts/diag`, never its `scripts` ancestor) is
unplaced rather than resolved to the wrong directory, because a wrong edge is
silent.
**What a narrowed run is not.** The model reads imports, literal path and module
names, directory scans and their arguments. It cannot see dynamic attribute access
(`getattr`), `eval`/`exec`, or a path assembled from data at run time — a config
value, an environment variable, a string built in a loop. The barrier above fires
only on references the parser can *see*; the rest it cannot see at all. So a
narrowed run is evidence about the files it ran and never about the tree: CI's full
matrix is the authoritative run and is unchanged, and every narrowed plan prints
that sentence on the run itself. A selected run also cannot see cross-test
pollution outside the selection.
Two guards hold the property rather than a list of shapes.
`test_no_changed_file_can_be_narrowed_while_the_graph_has_unresolved_references`
walks **every covered file in the real tree** and asserts the barrier refuses each
one — the shape the previous guard got wrong, because it walked the resolver's own
*output* and so was blind to a reference that never resolved.
`test_the_barrier_is_a_barrier_and_not_a_constant` asserts the same helper returns
*no* reason on a tree whose references all resolve, so "refuse everything" fails
that too, and each unresolved class has its own row in
`test_every_unresolved_reference_class_stops_the_test_selection`.
### The local `pyright` gate is bounded and process-group-reaped
The local `type-check` command is spelled
`.venv/bin/python scripts/run_bounded.py --timeout 1800 -- .venv/bin/python -m pyright …`.
The gate itself is **whole-tree**, exactly as `ci.yml` spells it: the wrapper
bounds and reaps, it never narrows.
**1800 s, deliberately NOT the 900 s that would mirror that job's
`timeout-minutes: 15`.** A whole-tree `pyright` measures **508 s on a quiet host
and 1170 s under load** on this fleet, and CI's 15 minutes also cover checkout,
dependency install and that job's protocol-sync step — so a bound equal to CI's
provision can fire on a merely loaded host and red a gate CI would pass. Timing
out a legitimately slow host is the worse failure, so the local bound is twice
CI's provision, and a bound that does fire says what it is **on either path**: the
wrapper's own stderr prints *"that is the BOUND (1800s) firing, not the gate
failing — re-run it, or raise it (`--timeout`, or `make type-check
BOUND_TIMEOUT=<seconds>`)"* — which is the only line `make type-check` shows — and
`make check-changed` adds *"rc=124 is the BOUND (--timeout 1800s) firing, not the
gate failing … re-run it; if it fires again, raise the bound"* from
`scripts/ci_scope.py`. `make type-check BOUND_TIMEOUT=<seconds>` raises it for one
run; `BOUNDED_GATE_TIMEOUT` in `scripts/ci_scope.py` is the same number for
`make check-changed`, and a test asserts the two agree.
**Why a wrapper.** `pyright` is a Python wrapper around an npm/node analyzer, and
node runs as a *separate* process. The fleet shows what goes wrong when the group
is not reaped: orphans re-parented to `ppid 1` holding **2.28 GB and 1.50 GB**,
one of them still alive **81 minutes** after its parent died, ten analyzers alive
at once at ~5 GB while sessions queued more behind them.
Be precise about what was measured and what was not: with `timeout 8|25` — and
even with `timeout -s KILL` — over a whole-tree `pyright`, the analyzer died with
its wrapper in every attempt here, and GNU `timeout(1)` signals the child's whole
process group on this host, so the tidy causal story ("the bound fires, the
wrapper dies, node survives") is **not** the mechanism, and the TRIGGER behind the
fleet's own orphan cases was never captured. What IS measured, and what the unit
tests drive with a real process tree, is the case `timeout(1)` cannot cover at
all: a leader that exits while a descendant lives on. `run_bounded.py` runs the
command in its own process group and signals the GROUP — on the bound, on a signal
to the wrapper, and as a sweep once the leader exits — so all three paths end with
nothing left running; the sweep is what removes a survivor the plain invocation
leaves.
It also escalates where a bare bound does not: over a SIGTERM-ignoring tree,
`timeout 3` fires at 3 s and then **waits the tree out** (30 s for a 30 s tree,
measured; 300 s in an earlier run for a 300 s one), where the wrapper SIGKILLs the
group after `--grace` and clears the same tree in 5 s **with `--grace 2`** (the
shipped `--grace` default of 10 takes ~13 s for it). It keeps
`timeout(1)`'s statuses (124 on a fired bound, 125 when the command could not
start), reports **128 + signum** when a signal ends the run — forwarded by this
wrapper, or delivered to the child by someone else, where `sys.exit` used to
render the raw `-9` as 247 — forwards **SIGHUP and SIGQUIT** as well as
SIGINT/SIGTERM (SIGHUP used to kill the wrapper and leave the group alive), writes
diagnostics to stderr only, and reports what it reaped.
Its own limit, stated: a `SIGKILL` to the wrapper itself cannot be caught, so
nothing runs a sweep in that case — what protects the group there is that the
members were signalled as a group in the first place.
### Every feature worktree owns its own venv. Never symlink one.
This repo is worked through many concurrent worktrees, and **each one gets a
real `.venv` installed editable from itself**:
```sh
git worktree add -b feat/my-change ~/workspace/repos/lo-my-change main
cd ~/workspace/repos/lo-my-change
uv venv --python 3.12 .venv
uv pip install -e ".[all,dev]" --python .venv/bin/python
```
It is tempting to symlink the parent's venv instead — it is one line and the
test suite passes either way. Do not. An editable install resolves imports
through a generated finder holding **one** hard-coded source root:
```python
MAPPING = {'local_operator': '/Users/you/workspace/repos/local-operator/local_operator'}
```
A symlinked venv therefore imports the tree it was *installed from*, not the
worktree you are standing in. The failure is silent and total:
`.venv/bin/local-operator` launches the TUI, the banner shows your branch's
version, the status bar shows your worktree's path, and every line of code
executing is `main`'s. An agent that "tested the feature interactively" this
way has tested nothing, and will report a working change as broken — or worse,
a broken change as working.
Verify which source a venv actually resolves before trusting an interactive
session, especially after any venv repair:
```sh
cd ~/workspace/repos/lo-my-change
.venv/bin/python -c "import local_operator; print(local_operator.__file__)"
# MUST print .../lo-my-change/local_operator/__init__.py
```
Wrong path? Rebuild the venv with the two `uv` commands above. `PYTHONPATH=$PWD`
is prepended ahead of the finder and will also work, but it is a per-invocation
plaster that the next command forgets — fix the venv instead.
**Why `cd` alone does not save you.** Python puts the *script's* directory on
`sys.path[0]`, not your current directory. For a console entry point that is
`.venv/bin`, so the worktree you are in never enters the path and the finder
wins. `python -c` and `python -m` do put the cwd first, which is exactly why a
quick `python -c "import local_operator"` can look correct while
`.venv/bin/local-operator` runs someone else's code — do not use the former to
clear the latter.
The one legitimate symlink is a **throwaway** worktree used to capture a
before-frame (see "Always capture before AND after"), where the parent's code
is what you want and the checkout is deleted minutes later. Scripts under
`scripts/` self-correct with `sys.path.insert(0, ...)` at the top, so they read
the tree they live in regardless. Anything you intend to *run as the product*
needs a real venv.
### A dead venv fails loudly; a wrong one does not
When the worktree that owned a shared venv is deleted, the console scripts in
it keep a shebang pointing at a `python` that no longer exists:
```
zsh: .venv/bin/local-operator: bad interpreter: .../lo-old-branch/.venv/bin/python: no such file
```
The fix is to reinstall editable **from the checkout you want to run**, not
merely to make the error go away:
```sh
cd ~/workspace/repos/local-operator # or the worktree that should own it
uv pip install -e ".[all,dev]" --python .venv/bin/python
```
Repairing it from the *wrong* checkout converts a loud failure into a silent
one — the import now resolves, to the wrong tree. That has already cost a
session: a stale editable install was also collecting `tests/unit/tools/
test_eval_tool.py` as **24 failures** that three agents in a row wrote off as
"environmental". They were a broken venv, and they all pass once it points
somewhere real. Treat a standing block of "known environmental" failures as an
unverified claim, not as weather.
### Isolating a run: `LOCAL_OPERATOR_CONFIG_DIR` alone is not enough
`LOCAL_OPERATOR_CONFIG_DIR` redirects the **config** dir. It does *not* redirect
the **cache**: `model/catalogue.default_cache_dir()` derives its root from the
home directory independently (and `skills/index.py` and `update.py` hardcode the
same root, deliberately — one place to clear), so a run with only that variable
set reads and writes `~/.local-operator/cache` while believing it is isolated.
**Redirect `HOME` as well** — that is the reliable method, and it makes both
agree:
```sh
env HOME=/tmp/iso-run LOCAL_OPERATOR_CONFIG_DIR=/tmp/iso-run/.local-operator ...
```
**And strip what a `lop` parent exports, because two of its prefixes are read by
the child product rather than only by a terminal.** `CMUX_*` is the one already
known here (a headless TUI that inherits `CMUX_WORKSPACE_ID` renames the
operator's real cmux workspaces). `LOP_*` is the other, and it is quieter:
`session/runtime/process.py` reads `LOP_MOBILE_CHILD_PROVIDER`, `_MODEL`, `_CWD`
and `_RESUME` to decide what a child runtime is, and sets
`LOP_RUNTIME_ADOPT_SESSION` for it; `session_factory` then reads that flag and
`LOP_RUNTIME_DEFER_MATERIALISE` to decide whether the session is adopted as-is or
materialised first. A cell run from inside
another session therefore inherits *that* session's provider and model (so it
silently runs on a provider the fixture never chose) and, with a deferral flag
inherited, a child can idle-exit with no work at all — a plausible-looking cell
that proves nothing. QA round 1 lost a cell to exactly this. Strip both prefixes,
then set the names the cell means to set.
This is worth spelling out because the failure is silent and it produces a
*plausible* wrong answer rather than an error. It has now cost two separate QA
rounds a false result in the same way: an "offline" cell resolved a provider
listing out of a catalogue cache that the same session's own earlier live calls
had written into the real home. Any cell whose point is a cold cache needs a
**fresh** `HOME` per cell, not merely a fresh config dir.
**And a redirected config dir is not automatically a redirected write path:
check the store's WRITE site, not just its read site.** A function can honour
an injected `config_dir` everywhere it reads and still default its writes to
the global path, so the isolation looks correct in every observable way while
the side effects land in the operator's real home. That is not hypothetical —
the analytics session-name backfill did exactly this and wrote 612 rows into
the live `~/.local-operator/analytics.db` from a sandboxed run. Production was
unaffected, because the only production caller passes the real config dir and
the two therefore agreed, which is precisely why nothing caught it. When you
isolate a run, verify where its writes actually go, not merely that its reads
are redirected.
**`lop browser install` used not to be isolated by `HOME` at all, and the
reason is worth keeping even though the hazard is now fixed.** `browser_bridge/
install.py` hardcoded `LABEL = "com.local-operator.browser"` and derived only
the plist *path* from `Path.home()`, while the launchd domain is `gui/<uid>`
from `os.getuid()`. A redirected `HOME` therefore wrote the plist somewhere
harmless and then `bootstrap`ed the *same global label* into the *same*
domain — and launchd resolves a plist to the `Label` INSIDE it, so the
`bootout` that precedes it evicted whatever held that label: the operator's
live bridge daemon. It happened: a sibling agent's test took the operator's
daemon down on 4099 for ~90s, and the only symptom he saw was his browser tool
going dead. Linux had the identical exposure through the fixed systemd unit
name.
The supervisor name is now derived per config root — `label()` and
`systemd_unit()` append a digest of the resolved root, so an isolated run
registers its own daemon instead of evicting the default one. Two constraints
that fix rests on, and that anything touching it must preserve: the DEFAULT
root's names stay byte-identical (a rename orphans every installed daemon,
leaving a live process the CLI can no longer stop), and "is this the default
root?" is answered against the **uid's passwd home**, not `Path.home()` —
`Path.home()` reads `$HOME`, so an isolated run would compare its root against
its own redirected home, conclude it is the default, and reuse the real label.
The advice below is therefore no longer load-bearing for safety, but it is
still the cheaper way to test: run the daemon as a **plain subprocess on a
non-default port** (`python -m local_operator.browser_bridge.daemon --port
<port>`, as `docs/design/browser-extension-evidence.md` does) rather than
calling `install` at all. If you do call `install` under a redirected `HOME`,
confirm it reported a suffixed supervisor name (`lop browser status` prints
`supervisor:` and `config root:` whenever it is not the default install)
before trusting that it left the operator's daemon alone.
### Read the committed ref, not the working tree
"Never `git stash` to get a before-frame" (in the visual-validation section
below) is about not *writing* into a checkout other agents are using. This is
the same hazard in the other direction, and it is easier to fall into because
nothing fails: **when you want to know what the repository says, read a ref,
not the files on disk.**
```sh
git -C ~/local-operator fetch origin main --quiet # REQUIRED: see below
git -C ~/local-operator show origin/main:AGENTS.md | grep -n 'thing you are checking'
grep -n 'thing you are checking' ~/local-operator/AGENTS.md # answers about SOMEONE'S DRAFT
```
**Fetch first, and pick the right ref.** `origin/main` is a local tracking
ref that only moves when something fetches, so reading it without fetching has
the *same* failure this section exists to prevent — a silent, plausible, stale
answer. Reproduced: against a tracking ref left where it was a few hours
earlier, `git show origin/main:pyproject.toml` reported `0.50.2` while `main`
was `0.50.3`, with no error. It bites hardest on "is this documented?"
questions, because anything added recently is invisible on an older ref.
And `origin/main` is not always the ref you mean. Asking whether *a PR's* tree
contains something means reading that PR's head, not `main`. Fetch it into a
**named ref** and delete that ref afterwards:
```sh
git -C ~/local-operator fetch origin pull/713/head:refs/lo-read/pr713 --force --quiet
git -C ~/local-operator show refs/lo-read/pr713:AGENTS.md | grep -n 'thing you are checking'
git -C ~/local-operator update-ref -d refs/lo-read/pr713
```
**Not `FETCH_HEAD`.** There is one `FETCH_HEAD` per gitdir, rewritten by every
fetch, and the `-C ~/local-operator` above means every session on this machine
shares the root checkout's copy. A peer fetching between your fetch and your
read silently redirects it: measured at **0 successes in 20 interleaved
trials**, returning `main`'s tree with no error, where the named-ref form
succeeded 8/8. That is this section's own failure — a confident wrong answer —
reappearing inside the command meant to prevent it.
Several agents routinely hold uncommitted work in the root checkout at the
same time, so a tracked file there is whatever the last writer left mid-task —
a half-finished edit, a section not yet written, or a revision from a branch
nobody merged. `grep` reports that state with no indication that it is not the
repository's.
The failure mode is a **confident false negative**: `grep` finds nothing, and
"nothing" reads like an answer rather than like a question about which tree you
just searched. It has already produced a wrong finding on a PR. A reviewer
checking whether the release procedure was documented grepped the working copy,
found no match, and reported that the procedure "does not exist in the repo's
markdown" — while `git show origin/main:AGENTS.md` matched it immediately at
`### What the release owner does`. A peer session was holding the file 398
lines shorter than `main`. The claim was reported, believed, and escalated to
the operator before anyone checked the ref, and the fix it implied was to write
a section that was already there.
So an absence found in the working tree is not evidence. Confirm it against the
ref before acting on it, and say which you read when you report it:
```sh
git -C ~/local-operator status --porcelain AGENTS.md # 'M'/'MM' => on-disk copy is not main's
```
This applies to any question about repository content — whether a rule is
written down, whether a helper exists, whether a test covers a case, what a
config declares. It does not apply to your own uncommitted work, which is the
one thing the working tree is authoritative about, and it is not only about
`grep`: a subprocess launched with the wrong cwd resolves the root checkout's
copy of a module rather than its own worktree's, which surfaces as failures
that look environmental and are.
When you need several files or a whole tree at a known revision, take a
throwaway worktree rather than reading the shared checkout and hoping it is
clean — and **remove it in the same task**, on a unique path, because a
leftover either blocks the next agent or gets silently reused at a stale
revision (`git worktree list` presently shows dozens of abandoned ones):
```sh
read=$(mktemp -d /tmp/lo-read.XXXXXX)
git -C ~/local-operator fetch origin main --quiet
git -C ~/local-operator worktree add --detach "$read" origin/main --quiet
grep -n 'thing you are checking' "$read/AGENTS.md"
git -C ~/local-operator worktree remove --force "$read"
```
The `-C ~/local-operator` is load-bearing on every command here: you are
usually standing somewhere else when you ask these questions, and without it
`git` fails with `not a git repository`.
## Releasing the stable `lop` runtime
Development and the global launcher deliberately use different installations:
- `uv run local-operator` and `.venv/bin/local-operator` execute the current
checkout. Use them while developing and validating source changes.
- `lop` executes the non-editable generation install that the launcher resolves
to: `~/.local/bin/lop` goes through `~/.local/share/lop/current/bin/lop` into
`~/.local/share/lop/generations/<stamp>-<sha-or-version>/`. It must remain
independent of the checkout so branch switches and uncommitted work cannot
break the global TUI.
A release here is a **combined release**: one version bump, one tag and one
GitHub Release covering every PR merged since the previous tag, cut by one
release owner. PRs do not carry their own bump, and merging is decoupled from
releasing. The sections below say why, then how.
### PRs do not bump the version; merging is not releasing
`pyproject.toml` stays at the **last released version** on every feature and
fix branch. A PR never touches it, and the reviewer round treats a version
change inside a feature PR as a finding.
**CI checks this**, in the `version-bump-guard` job. A pull request whose diff
changes the `version =` line fails unless its title starts with
`chore(release):`. Dependency and metadata edits to `pyproject.toml` are
unaffected — the guard reads the version line, not the file.
It makes the violation loud; it does not make it impossible. `main`'s ruleset
requires one approving review but configures **no required status checks**, and
admins hold a configured bypass (see "Who may merge: two tiers"), so an
`--admin` merge lands over a red guard. Treat a failing
`version-bump-guard` as a stop signal rather than an obstacle to route around:
the job is the reviewer's missing memory, not a lock.
**A release PR's green is narrow.** Change-scope gating classifies a diff
that is nothing but the `version =` line as a release bump, and a release
bump runs *only* `changes` and `version-bump-guard` — two checks, not a
matrix. That is the intended disposition (the classifier's `release_bump`
rule is asserted in `tests/unit/test_ci_hygiene.py`), but it means "CI is
green" on a release PR is evidence about the version line and nothing else.
The code being released was gated on the PR that landed it, not on this one.
Note that `gh api repos/<owner>/<repo>/branches/main/protection` answers
`404 Branch not protected` here. That endpoint reports only **legacy** branch
protection and 404s even while a modern ruleset is actively enforcing the
branch, so it is the wrong probe and reading it as "unprotected" is a mistake
this paragraph made once already. Query
`gh api repos/<owner>/<repo>/rules/branches/main` instead.
The job exists because the prose above was not enough. It was already written,
and it already told reviewers to treat a bump as a finding, when #701 and #706
each landed one inside a feature PR and both review rounds passed anyway. The
result was that `main` advertised `0.50.2` while the newest tag, GitHub Release
and PyPI artifact were all still `0.50.0`: `0.50.1` and `0.50.2` were consumed
without ever being built or published, and the v0.50.3 window had to skip both
numbers rather than reuse a version `main` had already claimed. A rule that
depends on every reviewer remembering to look is a rule that fails on the day
someone does not.
This replaces the earlier practice of each PR bumping its own patch, which
failed in a measurable way on 2026-09-05: with ten sessions each holding a
reserved patch number, `0.47.1` → `0.48.0` took close to five hours of agents
serialising behind one another (the tags land at 05:24, 05:58, 06:16, 06:50,
07:38, 08:27, 10:08 UTC — each one a PR that could not merge until its
predecessor had released). Every rebase across another session's bump was a
`pyproject.toml` conflict, and several were resolved into **dirty merge
states**. Two out-of-queue releases consumed numbers that other sessions had
already been told were theirs, so their PR bodies and reviews referred to a
version that no longer existed. None of that work needed a distinct version;
it needed to land.
So: **the owner of a PR merges it the moment its review rounds are clean and
fresh and CI is green** — and "green" means reading the run's
classification report: it names every flag, every changed path's category and
which jobs ran. **A skipped job is not evidence.** A guard nothing ran is
indistinguishable from no guard, which is why the `changes` job writes that
report into the step summary. No release queue, no waiting for a predecessor,
no handing the "next number" to whoever is behind you. A merged PR that has not
been released yet is the normal state of `main`, not a problem to fix.
### One release owner per window
Releases are cut by a single **release owner** for a **window**: the set of
PRs merged since the last tag that are ready around the same time (about an
hour). Nobody holds a merge to make a window.
**A PR that merges after the owner starts cutting does not reliably ride the
next window — it may ride *this* one, unlisted.** The tag names a SHA and
everything reachable from it ships, and the bump commit is not a barrier: in
v0.51.4 the bump landed at 07:16 on its own release branch and #731 merged to
`main` at 07:18, so the two sat on divergent branches and both were reachable
from the tagged merge. #731 was the ordinary merge to `main`, and it shipped
while being absent from the notes. So: **if you merge while a window is open,
send the owner your PR number and merge SHA at merge time**, not when you next
happen to talk to them. The owner cannot poll continuously, and a peer who
confirms a window list and then quietly merges into it has broken the protocol
even though every individual step looked correct. The owner's matching duty —
re-derive the window immediately before tagging — is in the release-mechanics
warnings below.
**The lock on a window is the open bump PR, not a message.** Two sessions
that both run `lop sessions` and both announce themselves in the same minute
each see "nobody owns it" and both proceed — which is precisely how two
out-of-queue releases happened. A `send` is not observable to a session that
was not listening, so it cannot be the lock. An open PR whose title starts
`chore(release):` — first `claim release window`, later
`bump version to X.Y.Z` — is observable to everyone through the forge, so it
is; the search below keys on the prefix, so the retitle does not drop the lock. The lock is opened *before* the number is known — the bump
is decided from the window's contents, which do not exist yet when the lock
is taken — so it starts life as a claim and becomes the bump once decided.
Before starting a release, every agent does the same things, in this order:
1. **Look for the lock.**
`gh pr list --search '"chore(release)" in:title' --state open`.
An open bump PR means the window is owned: its body names
the owning session's pid, so `send` that session (by `pid`) your PR's
number, merge SHA and `Release:` line, and let it aggregate. Do not start a
second release. (Quote the phrase — `gh`'s search passes it to GitHub,
which treats bare parentheses as syntax and returns nothing.)
2. **Take the lock by opening the claim PR.** If nothing is open, the
release owner's *first* act — before collecting anything — is pushing
branch `release-next` with one empty commit and opening a draft PR titled
`chore(release): claim release window`; its body names the owner's session
pid from `lop sessions` and an empty checklist of the PRs in the window.
Only then announce it with `send` to the other live sessions. Once the
window's bump is decided, the same single commit is amended into the
version change and retitled `chore(release): bump version to X.Y.Z`
(mechanics below) — the PR number, and so the lock, never changes.
3. **Tie-break by `createdAt`.** If two bump PRs are open, the earlier one
owns the window; the author of the later one closes it, deletes its
branch, and hands its window contents to the earlier PR's owner.
4. **Adopt a dead owner.** An agent arriving cold cannot know how long a
pid has been gone, so the clock is anchored on what the forge shows: if
the owner pid is absent from `lop sessions` *now* **and** the lock PR's
`updatedAt` and its last owner comment are both more than 15 minutes old,
any agent may adopt the window: comment on the PR that it is taking over,
put its own pid in the body, and continue from wherever the checklist
stopped. Nothing is reset. (An owner still working therefore keeps the
PR's checklist current; silence is what makes a window adoptable.)
The owner is a role for one window, not a standing job. Whoever cuts the
release is also responsible for telling every contributor in the window where
it landed.
### What the release owner does
1. **Collect** from each merger in the window: PR number, merge SHA, and
the PR body's `Release:` line. Every PR carries one, in this exact shape,
under its summary:
```
Release: <patch|minor> — <one-line user impact>
```
The bump is the merger's argument, the impact is the sentence the release
notes will use, and it lives in the body precisely so a merger who is no
longer running still contributes both. If a merged PR is missing the line,
the manager coordinating that PR adds it to the body before the window
closes; the release owner does not guess an impact from commit subjects.
2. **Pick ONE bump for the whole window** by the materiality rule below: a
minor only if some *single* PR in the window clears the step-function bar
on its own; otherwise a patch. Several patches in a window are still one
patch. The chosen version is `<last tag> + that bump`, never a number
someone was "promised" earlier.
3. **Land the bump PR** (the claim PR, now carrying the bump): one commit,
`chore(release): bump version to X.Y.Z`, touching `pyproject.toml` only.
This is a C0 change, but it is still an agent-authored PR, so the standing
review gate applies: an **independent reviewer subagent** — not the owner,
who is the author — posts `### Agent review — round 1` confirming the
diff is exactly one line in one file, the version is `<last tag> + the
chosen bump`, and no other PR in the window touched `pyproject.toml`.
The owner replies with the remediation comment and merges. A bump commit
that also carries code is a defect — the code belongs in a reviewed PR
of its own.
**Expect two checks on this PR, not a matrix.** A version-only
`pyproject.toml` diff classifies as a release bump, so only `changes` and
`version-bump-guard` run — the whole job set would be gating code this PR
does not touch. Do not read that short check list as a truncated pipeline,
and do not "fix" it by adding an always-run job: the narrow green is the
design (`scripts/ci_scope.py`, `release_bump`), and the code being released
was gated on the PR that landed it.
4. **Tag and publish** from the merge commit of that bump, then install and
smoke (mechanics below). The release notes cover **every PR in the
window**, grouped in the house style of the existing releases (a headline
sentence naming the version's theme, then `## Major` / `## Minor` /
`## Fixes` as applicable, then `## Install` and the full-changelog compare
link). A window's notes are written from the collected impact lines, not
from commit subjects.
5. **Post the refs** (tag, release URL, installed `.lop-source` revision) as a
comment on every PR in the window, and `send` them to each contributor
still running.
### Mechanics, in order
The bump branch lives in a throwaway worktree so the root checkout's branch
is untouched, but `lop-update` itself runs against `~/local-operator`: it
gates on `-d "$REPO/.git"`, and a worktree's `.git` is a *file*, so pointing
`LOCAL_OPERATOR_REPO` at a worktree fails with `local-operator repository not
found` (reproduced). The owner fetches and advances the root checkout's
`main` ref, then runs the drained install there (see "Installing over a live
fleet"). The order matters: the bump PR is opened
first as the lock, merged only after every PR in the window has merged, and
nothing is installed until the tag exists. The claim and the bump are ONE
commit on ONE branch: the owner amends and force-pushes with
`--force-with-lease` rather than stacking a second commit, so the scope check
stays "one line in one file".
```sh
# 1. Confirm the window: everything on origin/main since the last tag.
git -C ~/local-operator fetch origin
git -C ~/local-operator log --oneline "$(git -C ~/local-operator describe --tags --abbrev=0 origin/main)..origin/main"
# 2. Take the lock: an empty claim commit in a throwaway worktree, opened as
# a draft PR. (No open chore(release) PR was found in step 1 of "One
# release owner per window".) The number is not known yet.
git -C ~/local-operator worktree add /tmp/lop-release-next origin/main
git -C /tmp/lop-release-next checkout -b release-next
git -C /tmp/lop-release-next commit --allow-empty -m 'chore(release): claim release window'
git -C /tmp/lop-release-next push -u origin release-next
gh pr create --draft --base main --head release-next \
--title 'chore(release): claim release window' --assignee damianvtran \
--body 'Release window claimed. Owner session pid: <pid from lop sessions>.
Window (tick as each merges):
- [ ] #<n> — <Release: line>'
# ... collect the window, write notes, wait for the last PR in the window to
# merge; then, with the bump decided:
# 2b. Turn the claim into the bump: amend the SAME commit, retitle the SAME PR.
sed -i '' 's/^version = ".*"$/version = "X.Y.Z"/' /tmp/lop-release-next/pyproject.toml
git -C /tmp/lop-release-next commit --amend -am 'chore(release): bump version to X.Y.Z'
git -C /tmp/lop-release-next push --force-with-lease origin release-next
gh pr edit <claim-pr-number> --title 'chore(release): bump version to X.Y.Z'
gh pr ready <claim-pr-number>
# ... independent scope-check round, merge; then:
# 3. Advance the local main ref to the merged bump WITHOUT checking it out —
# after the compare, because nothing downstream refuses a stale ref. Both
# calls must print the SAME sha (`rev-parse --short` takes ONE revision, so
# the two refs cannot share the flag).
git -C ~/local-operator fetch origin
git -C ~/local-operator rev-parse --short main
git -C ~/local-operator rev-parse --short origin/main
git -C ~/local-operator update-ref refs/heads/main origin/main
# If main IS the checked-out branch, use this instead:
# git -C ~/local-operator merge --ff-only origin/main
# 4. Write the notes from the collected `Release:` lines, then tag + GitHub
# Release on the bump's merge commit. --target creates the tag on that
# exact SHA; the publish workflow triggers on the release.
$EDITOR "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/lop-release-X.Y.Z-notes.md" # headline, ## Major/Minor/Fixes, ## Install, compare link
gh release create vX.Y.Z --target "$(git -C ~/local-operator rev-parse origin/main)" \
--title 'X.Y.Z: <theme>' --notes-file "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/lop-release-X.Y.Z-notes.md"
# 5. Install and verify — with the fleet DRAINED: wait until no session
# reports `busy`, install, then re-engage what the swap displaced.
# See "Installing over a live fleet" below; a plain `lop-update` is only
# safe when nothing is running.
<install>
# The revision record lives INSIDE the installed generation, at `sys.prefix`:
# a git build writes "<sha> <ref>"; a PyPI install writes "pypi <version>".
cat "$(readlink ~/.local/share/lop/current)/tools/local-operator/.lop-source"
# Smoke the built command from outside the repository, naming the uv-tool build
# by path where the host has more than one `lop`: a bare `lop` can resolve to a
# different install and report a version that is not this one.
cd /tmp && ~/.local/bin/lop --version
# Two checks, two different questions — do not use one to answer the other:
# * `.lop-source` (above) says WHICH BUILD is installed (ref, or `pypi <v>`);
# * a symbol check in the installed tree answers a DIFFERENT question — what
# code that build contains. The glob must stay OUTSIDE the quotes, or the
# shell passes it through literally and grep reports "No such file or
# directory":
# g=$(readlink ~/.local/share/lop/current)
# grep -c _SurvivalIndex "$g"/tools/local-operator/lib/python*/site-packages/local_operator/redaction_shapes.py
# A docstring can quote a removed line verbatim, so a grep for a code string
# is not evidence either way; the symbol is.
# 6. Reclaim the worktree.
git -C ~/local-operator worktree remove /tmp/lop-release-next
```
`lop-update` archives the committed `main` ref, builds and installs that
snapshot, and records the exact source revision in the installed generation's
`.lop-source` (at `sys.prefix` — see step 5). It never packages the
currently checked-out branch or uncommitted files. A specific committed ref can
be installed deliberately with `lop-update <git-ref>`.
**It does NOT compare that ref against a remote.** The compare-and-refuse gate
belonged to the retired legacy installer; this build has no remote check at all
(the compare is step 3's, and the wrong-build measurement is below), the refusal
text survives only in
`~/.local/bin/lop-update.legacy-uvtool.bak` **as history**, and
`--skip-remote-check` is gone with it. So the fetch-and-compare is the release
owner's own step, never a backstop — that is why step 3 exists. It uses
`update-ref` rather than a checkout because the root checkout is usually on
another branch with uncommitted work, and `update-ref` moves the branch pointer
without touching the working tree: `update-ref refs/heads/main origin/main` is
safe while another branch is checked out, and under a checked-out `main` it moves
the branch without touching the index, so `git status` would then show every
merged change as a local modification — use
`git -C ~/local-operator merge --ff-only origin/main` in that case.
**There is no remote check to rely on — the fetch IS the protection.** The
current `lop-update` does not compare the ref against a remote at all: it takes
its ref from its argument (defaulting to the local `main`) and delegates to
`lop update --from-snapshot "$REF"`. The compare-and-refuse body that this step
used to contain was retired with the legacy installer. It survives only in
`~/.local/bin/lop-update.legacy-uvtool.bak` **as history**; its `REFUSING to
release a stale ref` message lives there (`:154-170`), nothing runs the script,
and the `--skip-remote-check` flag went with it. So nothing refuses a stale
local `main`: measured on 2026-09-21,
`lop-update` built **0.61.6 from `f6eaea3d`** while `origin/main` was several
releases ahead, and the wrong build was installed before anyone noticed. The
BEFORE-install check is therefore not a fallback to a gate; it is the only
protection there is: `git fetch origin main`, confirm `git rev-parse --short
main` equals `git rev-parse --short origin/main`, and when in doubt name the ref
explicitly — `lop-update <sha>` installs exactly that commit. The generation
directory name also carries the commit or version it was built from
(`…/generations/<timestamp>-<sha-or-version>`), so every build is auditable
after the fact, including when the marker is the `pypi <version>` form.
### Installing over a live fleet
**The install must not run against a busy fleet.** That is the rule; the command
that carries it out is the host's business. An install replaces site-packages
inside the shared uv-tool environment **under whatever runtimes are running**,
and a runtime that is mid-turn never checks for a newer build — by design, only
an idle runtime notices one — so it goes on importing from a tree that is being
deleted beneath it, dies torn, and writes no exit record. Measured on
2026-09-15 at 19:23, installing 0.55.8 took every running session on the fleet
down mid-turn; and because only sessions **watched** by a frontend re-engage
eagerly, the unwatched half — the `daemon`-kind sessions, woken by `lop send`
or a wake, as against the `attach` viewers a frontend is holding open — stayed
down until they were resumed by hand.
So the step is: **wait until no session reports `busy`, install, then re-engage
whatever the swap displaced** — turns that were in flight when it landed, and
the sessions no frontend is holding open. A host that has a drain wrapper for
exactly this may use it, and it is the convenient way to do those three things
in order; the wrapper is a host-local convenience, not part of the protocol,
and the release owner owes the behaviour above whether or not one exists.
Two cautions that hold wherever the install runs:
- **Never force an install past a busy fleet to save time.** Displacing
someone's turn is a real cost, and an expired drain budget means waiting and
running it again, not forcing it.
- **Verify the build; do not read it off an installer's own summary.** A
summary that resolves `lop` on `PATH` can describe a different install than
the one it just wrote: measured on 2026-09-15, such a summary printed
`v0.55.9` after installing `0.55.10`, because a bare `lop` had resolved to a
desktop app's managed environment. Read
the installed generation's `.lop-source`
(`$(readlink ~/.local/share/lop/current)/tools/local-operator/.lop-source`) and smoke the built
command from outside the repository for the answer.
It is still `lop-update` underneath, so the mechanics above about the committed
`main` ref and the source revision record apply unchanged — but the
remote-check gate does NOT exist in this build (see step 3): nothing compares
your ref against a remote, so fetch and compare it yourself before installing.
Warnings that still hold, each of which has already cost a release:
- **Derive the window from the commits, not from the commit shape.** Use a
plain `git log <last-tag>..origin/main` and read the PR references out of it.
**Never `git log --merges`**: GitHub's *merge* button produces a merge commit
that `--merges` sees, while its *squash* button produces a single-parent
commit that `--merges` silently drops. So the trap fires **per PR, according
to which button someone happened to press** — which means a window is not
reliably empty when it is wrong, it is **partially listed**, and a partially
correct window looks right and survives review. Check it rather than trusting
this paragraph:
```sh
git log --oneline --merges v0.51.0..v0.51.1 # 3 PRs
git log --oneline --first-parent v0.51.0..v0.51.1 # 4 PRs
```
The one only the second form lists is #720 (`78c20295e`), squash-merged; it
nearly shipped unlisted in v0.51.1 and was caught only because its author was
watching. `--first-parent` happens to work for *counting* the window because
both button shapes land on that chain, but do not adopt it as a general
"show me every PR" idiom: for a PR landed with the merge button it shows only
`Merge pull request #999 from feat` and never the substantive commit on the
second parent, so when a merge subject is uninformative the plain log carries
the real description and the first-parent walk does not. A release note that
omits a merged PR is a defect in the release, not a cosmetic miss — it is
the only record of what changed under a user who is about to update.
- **Re-derive the window immediately before `gh release create`, not once when
you claim it.** The bump commit is not a barrier: anything merged after it
and before the tag still rides the release, because the tag names a SHA and
everything reachable from it ships. This happened in v0.51.4 — #731 merged
*above* the bump commit `d1ce356ac`, shipped in the tag, and was absent from
the notes, even though the owner had polled every peer and derived the window
correctly when they started. Deriving it right once does not help if the
derivation is stale by the time you tag. The check is one command:
```sh
git log --oneline <last-tag>..origin/main # immediately before tagging
```
The other half of this is the merger's duty, and it is stated where a merger
will actually read it — see "One release owner per window" above.
- **Check `git diff <last-tag>..origin/main -- pyproject.toml` is empty before
tagging.** A non-empty diff means a merged PR carried its own version bump
and has silently consumed the number you are about to use. That is exactly
how 0.50.1 and 0.50.2 were burned — each consumed by a PR's own bump, neither
ever built, tagged or published, and the next owner had to skip both. The
`version-bump-guard` CI job now catches this on the PR, but it does **not**
block an `--admin` merge, because `main`'s ruleset configures no required
status checks and admins hold a bypass. The pre-tag check is the backstop.
- **A shard failure on your branch alone is not evidence your diff caused it.**
CI shards the unit suite with `scripts/shard_tests.py` — a
longest-processing-time-first (LPT) partition of
`sorted(glob('tests/unit/**/test_*.py'))` by the measured per-file weights in
`tests/durations.json` (`.github/workflows/ci.yml`). The positional
`i % 5` split this replaced balanced file *counts*, ignored what files cost,
and so shuffled the slow cluster between shards as unrelated files were
added. LPT **still moves files between shards** — it assigns by remaining
capacity, so one new weight cascades; measured when the split landed, one
inserted file moved 326 of 458 files. What it fixes is the consequence:
balance is invariant under insertion, so there is no saturated shard for the
slow cluster to migrate *into*, and a shard's runtime stops being evidence
about who added what. Derive the shard against the ref CI actually runs
rather than a stale local checkout — run it, do not read it:
```sh
python scripts/shard_tests.py --shard <I> --total 5 \
--out "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/shard_<I>.txt"
```
Run it once at `origin/main` and once at your branch (a worktree each, or
`git stash` between); it partitions the tree it is run in, and it sorts with
Python's `sorted()`, so it is locale-independent by construction. The
hand-rolled `git ls-tree | LC_ALL=C sort | nl` recipe this replaces computed
the retired positional index and is no longer valid — which also retires the
`LC_ALL=C` trap that recipe carried.
If the shard differs, the failing test ran in a different group than it does
on `main`, so a failure appearing only on your branch may have nothing to do
with your diff. **A differing shard is a reason to check attribution, not by
itself an explanation** — confirm either way by reproducing on clean
`origin/main` under the conditions CI uses (the shard job runs `pytest --cov`,
and some failures do not reproduce bare). The one case where the grouping
genuinely *is* the cause is cross-test pollution: if the test fails
reproducibly in its new shard and passes in its old one, the neighbours it now
runs beside are leaking or withholding state, and you have a test-isolation
bug to fix rather than a false alarm. Attributing one of these to a diff that
touched no related file has already cost a review round.
- **Several PRs going red at once is a shared cause, not several bugs.**
`cli-sanity` and `server-sanity` call a live model (both are marked
"Live-LLM job" and take `OPENROUTER_API_KEY`), so a third-party outage or
429 reddens every open PR simultaneously — including heads that touch
neither surface. Re-sharding (above) produces the same fleet-wide pattern
from an entirely different cause. Either way the tell is the same: before
debugging your diff, check whether sibling PRs went red in the same window,
and confirm *which* job failed. A shard failure and a sanity failure look
equally alarming on the checks list and have nothing to do with each other.
- **Never pre-create a bare tag** (`git tag vX.Y.Z && git push --tags`) and
then make a release from it. The publish workflow triggers on the *release*
being published, so a bare tag publishes nothing, and `gh release create`
against an existing tag will happily attach notes to whatever SHA that tag
already points at — which is how a release once shipped the previous
version's code under the new number. Let `gh release create --target`
create the tag.
- **There is no `--skip-remote-check` any more, and no gate for it to skip.**
The flag was retired with the legacy installer along with the compare-and-
refuse step, so `lop-update` installs whatever ref you name — including a
stale local `main`, which is how 0.61.6 was once built from an old commit
while main was several releases ahead. The protection is the fetch-and-compare
in step 3, not a flag; it exists because a stale local `main` was once
installed and reported as a successful release while `lop` silently downgraded
from 0.18.1 to 0.17.5.
- **Never repoint `lop` at the editable `.venv`**; doing so couples the stable
command back to in-progress work. Publication is always the separate final
step: merge, bump, tag, the drained install, verify `.lop-source`, then smoke
the built command from outside the repository — naming the uv-tool build by
path, because a bare `lop` can resolve to a different install.
Every agent asked to "update local-operator" or make a change available through
`lop` follows this protocol: merge the tested change when its rounds are clean,
then either hand it to the window's release owner or become that owner. It
does not bump the version on its own branch, and it does not release
one PR alone while other merged work is sitting on `main` unreleased.
## Releasing the browser extension to the Chrome Web Store
The extension and the runtime release on separate tracks. The runtime uses the
combined-release protocol above; the extension uses the Chrome Web Store's own
two-phase review model, driven by two workflow_dispatch workflows. Both live in
`.github/workflows/` and run against the `chrome-web-store` (stage) and
`chrome-web-store-production` (promote) protected environments. The store
credentials are environment-scoped variables — they are not readable from a
local shell or a workflow token, so there is no local path to the store API.
**The extension version in `extension/manifest.json` and
`extension/package.json` tracks the extension code, not the review queue.** It
must be bumped in the same PR that changes extension behaviour, so that every
submitted version identifies exactly one tree. A PR that changes extension
source without bumping the version has created an ambiguous artifact — the
version number no longer pins a tree. This happened with 0.1.9 (PR #798 changed
9 extension files including `protocol.gen.ts` without a bump), and the fix was
to bump to 0.1.10 before the next submission.
### Phase 1: Submit for review
```sh
gh workflow run chrome-web-store.yml -R damianvtran/local-operator \
--ref main -f ref=<origin/main sha> -f version=<manifest version>
```
This builds the extension from the named commit, validates the zip, and calls
the store's upload + `STAGED_PUBLISH` endpoint. The item enters Google's review
queue as `PENDING_REVIEW`. While an item is in review, the store refuses all
further uploads with `HTTP 400 FAILED_PRECONDITION / NOT_UPDATEABLE`. That
rejection is non-destructive and leaves the queued item untouched, so
re-dispatching the stage workflow is the practical way to read the queue state
(observed across runs 34184188091, 34211464472, 34259291271, 34316525125,
34385550911, 34452314995 while 0.1.8 sat in review). The script itself
deliberately does not translate error reasons into behaviour — it only prints
the store's body — so treat the rejection as an observation, not a contract.
Google publishes no SLA for review. Extensions using the `debugger` permission
with broad host access routinely draw extended manual review (the 0.1.8 review
took ~4.5 days). **Do not cancel a pending review to force a new submission** —
cancelling forfeits the accrued queue position with no visibility into how
close it was. Once the review finishes, promote the approved revision; that
clears the queue and the next version can be staged immediately.
### Phase 2: Publish an approved staged revision
When the store dashboard shows "Ready to publish" (or `fetchStatus` reports
`STAGED` at 100% deploy), publish it:
```sh
gh workflow run chrome-web-store-promote.yml -R damianvtran/local-operator \
--ref main -f version=<approved version>
```
This skips the build entirely — it reads the store's own `fetchStatus`,
refuses unless the submitted revision is STAGED at 100% for exactly that
version, then calls `publish` and polls until `PUBLISHED`. It is the automated
equivalent of the dashboard's Publish button, behind the same environment
guards and main-only ancestry check as a stage upload.
### Several extension installs paired at once, and the dev manifest key
The store build and a locally loaded (`Load unpacked`) build are DIFFERENT
identities, and the daemon accepts an allow-list of them rather than one pinned
id (`browser/pairing.json`: the legacy `{extension_id, token_sha256, paired_at}`
trio kept verbatim as the driver's record, plus an `identities` list with **one
token per identity**). Two installs may therefore be connected at once, and only
one of them DRIVES at a time — the other is a **standby** that receives no
command, holds no tab, and is promoted if the driver goes away. Everything that
changed here is described in `docs/design/browser-multi-identity-pairing.md`.
**The one rollout cost, stated rather than discovered.** Two installs can be
paired at once and exactly one drives; the other is told `standby` with a
`{event: "role"}` frame. A build that PREDATES this change (the released 0.1.10
store build, which cannot be side-loaded and will be on real profiles for
weeks) does not know that event: told to stand by, it keeps its
`chrome.debugger` attachments and its surface map, so it leaves "Local Operator
is debugging this browser" banners on tabs only it can release, and its popup
still reads as connected while the daemon routes commands to the other install.
No daemon-side fix exists for a build that cannot hear the frame. So: the
disclosure is in `lop browser status` (printed whenever a standby is listed),
in the PR body, and here. If an operator sees banners on tabs after loading a
second build, removing that build (or closing those tabs) releases them.
What that means when you are working on the extension:
- **`lop browser pair`** shows one line per WAITING install, named by the label
the daemon derived from its `hello` (browser + extension version), because a
user staring at two popups cannot otherwise tell which code is which.
- **`lop browser pair --list`** shows the authorised installs and which one is
driving; **`--revoke <id-or-label>`** removes exactly one and severs only its
socket; **`lop browser drive <id-or-label>`** moves the wheel by hand (the
incumbency rule is self-stable, so the install already driving keeps it).
- **`unpair`** in the options page is per-identity for the same reason.
- **`PROTO_VERSION` stays 1.** It must not be bumped for any of this: a bump
closes the released store build with 4001, whose popup reads as an unfixable
"update needed" card. Capability travels in ADDITIVE `HelloAck` fields (`role`,
`authorized_count`) plus one new daemon→extension `role` event. **Never add a
field to `Hello`** — it is validated with `extra="forbid"`, so every already
released daemon would close a new extension that did. An absent `role` must
always be read as "driver".
**A dev-only manifest `key` gives local builds a stable id.** A Chromium
unpacked extension's id is derived from its DIRECTORY PATH, so the same build is
a different identity in every worktree and the operator re-pairs on every
switch. `extension/manifest.dev.json` holds an RSA **public** key which
`build.mjs` merges into the manifest of every NON-store build (`--zip` never
gets it: the store assigns the published identity, and that file must stay
exactly as submitted).
Why committing it is fine, and precisely what it does not buy:
- `key` is the base64 of a **public** key. It is an identity, not a credential.
The private half is not in this repository (it was generated once and
deliberately discarded — it is only needed to sign a `.crx`, which this project
never distributes) and is not needed to Load unpacked.
- Anyone can copy it and build an extension claiming the dev id. That buys them
nothing: pairing still requires the 6-digit code that only the operator's
terminal prints (`docs/design/browser-extension.md` §6.2), and the daemon
needs a code from the DEV INSTALL for that id.
- Therefore **never auto-trust it**: a well-known id the daemon accepts without
pairing is exactly what would turn a public identity into a credential. The
dev key changes the ID a local build gets; it changes no authorisation rule.
- The dev id currently derived from that key is
`ijokelchmpajpdekmanpbhnnccbpoopl`. Verify it before relying on it:
`node build.mjs`, load `extension/dist` unpacked, and read the id off
`chrome://extensions` — the derivation is Chrome's, not ours.
### What to record
After every submission or promotion, post the outcome on the relevant PR:
the workflow run URL, the exact API response (not a summary of it), the
version, and the source commit. The release record (`docs/store/release-record.md`)
is updated with the artifact SHA-256, source commit, and protocol version once
the version is live on the public listing. Never claim a version is live from
a merged PR or a workflow's success — only from the public listing or a
successful publish call.
## What an agent may start: `exec` with the allowance, never a terminal
`lop exec` opens a TOP-LEVEL conversation — an ordinary session directory that
`is_user_session` reports as the operator's own. From inside an agent session
that is never what you want *by default*: the operator's session list and
desktop sidebar show it as a chat they opened, and it runs outside the job
manager that would let you see, steer, cancel or account for it. A subagent did
it out of the wrong belief that it was the only way to get a review run going:
```sh
lop exec --profile reviewer --background --name lo-1281-review < brief.md
```
That happened on 2026-09-18: two sessions (`lo-1281-review`, `lo-1281-qa`)
appeared in the operator's sidebar for a PR they had never asked about, because
the coder that owed the review round held no `task` tool (a role that does not
delegate is never handed `task`/`wait`/`wake` — see `harness.subagent`'s prune).
**The rule after the 2026-09-19 relaxation, and it has two answers on purpose.**
The guard in `local_operator/agent_shell.py` decides by the calling session's OWN
live inventory — it holds `task`, or it does not:
* `lop exec` **is allowed** to an agent session that holds `task`, because that
is the case an operator asks for when they ask for it: "spin up parallel
sessions and fully delegate this work", or a fan-out of independent long-lived
workstreams, is a shape a `task` child cannot be (one prompt, then gone). A
session that does NOT hold `task` is refused exactly as before — for it the CLI
is a way around a missing tool, not a way to delegate.
* the **interactive** path (`lop`, `lop --resume ID`, `--tui`) stays refused for
EVERY agent shell, delegating or not. An agent has no terminal, so that path
opens a front end on the operator's SCREEN — not the delegation shape above.
When you touch this guard, check both halves: a change that relaxes one
predicate and not the other is the defect this paragraph exists to prevent.
The allowance travels as `LOCAL_OPERATOR_AGENT_MAY_DELEGATE`, which the `bash`
tool signs from the session's live tool inventory (`ToolContext.may_delegate`,
set by `Session._build_tool_context` from `self._tools`) in three arms: `1` when
the session holds `task`; the EMPTY string only where the name is inherited from
the launcher and the session does NOT hold it, which is the clear that stops the
allowance outliving the session it described; and NOTHING AT ALL otherwise, so a
session that never had it is not handed the spelling — the name IS the mechanism
(`LOCAL_OPERATOR_AGENT_MAY_DELEGATE=1 lop exec` is a one-token self-grant). Both
an absent marker and an empty one read as "may not delegate", so a forgotten
export is a refusal the model reads rather than a chat in the operator's sidebar.
**Delegate with `task`, and reach for `exec` only when the user asked for
sessions.** "Spin up parallel sessions to fully delegate this" is that ask: open
them, and say what you opened. If you think you need a separate live session and
were NOT asked, say so in your report instead of starting one. A session that
holds no `task` does the work itself and reports a genuine blocker with `hub`;
work that belongs later is not a child's to arm either — `wake` is pruned from
EVERY child session — so it goes back to the session that delegated. `lop exec
--status JOB_ID` still polls a job that is already running.
WHO may delegate is the role's answer, never the depth's (operator, 2026-09-18).
A subagent whose role allows delegation is expected to use `task` at any depth —
a `manager`'s child is a manager too — and the tree it grows is navigable: the
TUI re-scopes its roster to the page you have open and climbs with `p`/`Esc`, and
the desktop UI walks the same edges with its breadcrumbs and back control. A role
that does not delegate gets no `task` at any depth and must do the work itself.
**A session either route opens is stamped `agent-shell`.** The QA escape hatch
and an allowed delegating `exec` both write `origin.json` =
`resume.ORIGIN_AGENT_SHELL` in `session_factory._prepare`, so the run stays out
of the `/resume` picker, the desktop sidebar and the phone's list. It is not
hidden and not a lock: `lop sessions` lists it, the id is in the run's own
output, and YOUR route back to it is `lop exec --resume <id>` — the bare
`lop --resume <id>` is the operator's, and the interactive path stays refused for
every agent shell (which is why this paragraph names the `exec` form). A
conversation a run merely RESUMES is never re-marked — that
is the operator's own work.
**`--workstream` is the ONE explicit exception, and it is opt-in per run.**
`lop exec --workstream` asks for the opposite disposition: a long-lived parallel
workstream the operator asked for is LISTED in the sidebar, `/resume` and the
phone list — by minting `resume.ORIGIN_AGENT_WORKSTREAM`, a second value
registered in `USER_ORIGINS`, which every listing in the tree already funnels
through — and it carries `opened_by` (the opening session's role, task label and
session id) so a machine-started row cannot be mistaken for one the operator
opened. It chooses visibility only and does NOT imply `--control`: every exec
run already publishes a discovery record, so the row is steerable either way,
and `--control` would change the approval posture. Absent the flag nothing changes: an
unflagged agent run is `agent-shell`, hidden and silent, and a hidden run raises
no completion banner of its own — its supervising session owns it, and the durable
records still make it findable. Reach for the flag only when the operator asked
for parallel sessions; a run that was not asked for is noise in their list. The
marker is written once, at creation, so a run started without the flag cannot be
promoted later by editing it — start it as a workstream instead — while
`lop exec --resume <id>` of a workstream does not re-stamp and therefore keeps the
row listed. `docs/EXEC.md` owns the flag table and the full semantics.
**Scripts that drive the real CLI must declare themselves.** A bench, an eval
driver or a pty harness runs `exec` — or the TUI — as a child of YOUR shell, so
it inherits the marker and every inner run would be refused, which reads as a
broken product instead of a guard. Call
`agent_shell.harness_child_env()` for the child's environment: it sets
`LOCAL_OPERATOR_ALLOW_NESTED_SESSION=1` (named here for a harness that needs it —
obscurity, not secrecy, and `docs/EXEC.md` documents the limits of the whole
rule). Use it against an isolated `LOCAL_OPERATOR_CONFIG_DIR`; the session the
child opens is stamped `agent-shell`, so it cannot be mistaken for one the
operator started. The helper also carries the notification gate
(`tui.notify.ENV_DISABLE`), because a harness's child is a session nobody is
watching: the benches seed the test hosting, whose only reply is the mock's own
sentence, and a notification's body is a snippet of the session's last
assistant line — so an un-gated child finishes a turn and puts that sentence on
the operator's lock screen. A rig that runs a session in ITS OWN process
instead calls `tui.notify.suppress_notifications_for_process()` before it
starts one; the two capture sandboxes (`probe_isolation`, `visual_capture`)
spell the switch literally, because they must act before any product import.
The same applies to a manual QA run: that escape is for
driving the real front end, never for opening a peer to hand work to.
**The test-hosting rule is a SECOND gate, and a test whose subject is the
notification path has to waive it.** `session_uses_test_hosting` suppresses any
session whose journal records the mock wire, in every process that reads the
store — the TUI observer, the machine-wide feed and the per-session bridge all
ask it — so a suite that asserts a `notification` frame over a fixture-built
store observes nothing until it sets
`session.model_selection.ENV_ALLOW_TEST_HOSTING_NOTIFY`. That is a test/QA seam
in the same shape as `ALLOW_NESTED_SESSION` above: it can only ever ENABLE (it
answers "not a test session", so it cannot silence anyone), and the process kill
switch still wins because every leg asks it first. Set it through
`tests/notification_opt_in.notification_path_opt_in`, which clears both gates
and restores them; the alternative — running those tests on a real provider —
would make the reply non-deterministic and need network and credit.
## Who may merge: two tiers
`main` is governed by a ruleset that requires **one approving review**, plus the
one extra approval GitHub's *Additional approval for unattributed Copilot pull
requests* adds when a pull request is not attributed to a person (on by default:
it raises the configured count, so an unattributed PR needs two approvals), and
carries a configured **bypass for the admin repository role** (`bypass_actors`:
`RepositoryRole` 5, `bypass_mode: always` — diff this sentence against
`gh api repos/<owner>/<repo>/rulesets/<id>` rather than trusting it). It no
longer sets `require_code_owner_review`, and `.github/CODEOWNERS` no longer
declares an owner for anything: the two were removed together on 2026-09-16,
because the owner list was what created a review request on every PR the moment
it opened (four people notified per PR, usually mid-remediation) and a
code-owner rule with nobody declared is an unsatisfiable control. Those two
settings together still encode a
deliberate two-tier policy, and this section exists so nobody "fixes" one half
without understanding what the other half is for.
**Tier 1 — the PR is the owner's.** When the agent is **acting for the owner** —
running on the owner's machine and under their account, which is
the normal case here — the standing agent review gate **is** the approval. A
clean, fresh, independent agent review round plus green CI is sufficient to
merge; the agent does not need to find a second human to click approve.
Green here is the *classified* green: read the `changes` job's summary
before treating the check list as evidence, because a job this diff skipped
appears there as a skip rather than as a pass.
**Tier 2 — the PR is anyone else's.** An outside contributor's PR needs **both**
an approving review **and** a clean agent review round. The approval is the
ruleset's `required_approving_review_count: 1` doing its job — it must come from
a collaborator other than the author (`bbqben`, `olafagbemi`, `sherman-tsui`,
`SanaKetabchi`, or `damianvtran`); with no code owners declared it is no longer
constrained to that list, and it is the reason this tier still holds. The agent
round is this file's standing gate. Neither substitutes for the other. This tier
is the reason the review-count requirement exists and must not be lowered: at 0
an outsider could land on `main` with nobody having looked at it.
**How the forge records tier 1.** GitHub prohibits approving your own pull
request (`422 Review Can not approve your own pull request`), and every agent
here pushes as the owner's account, so an agent-authored PR the owner created
can never be *clicked* approved by the account that opened it. The ruleset
anticipates exactly this: the admin-role bypass is the **sanctioned** way the
owner's reviewed PR completes, not a hole. So, concretely, for an agent acting
for the owner with a clean independent round and green CI (the `changes`
job's classification summary, not just the tick list — a skipped job is not
evidence): try the
normal merge first (a collaborator may already have approved); if the ruleset
refuses because nobody else has approved, complete it with `--admin` **and
disclose that on the PR** in the terms below. Do not sit on finished, reviewed
work waiting for a click that may never come — and do not pretend the click
happened.
This is a statement about *authority*, not about rigour. Every requirement in
this file still holds in full: an **independent** reviewer subagent (never the
agent that wrote the code), rounds repeated until no blocker or major remains,
review freshness against the current head, QA evidence from the real running
surface, and design/UX rounds for anything user-visible. Merging is authorized
by the review being genuinely clean, never by the merger being entitled to it.
Two things this does not license:
- **Never approve your own work to satisfy the rule.** The author and the
reviewer must be different agents. GitHub cannot tell them apart, because
every agent here pushes as the same account — so this separation is a
discipline the agents keep, not one the forge enforces.
- **`--admin` stays disclosed, always — and described accurately.** Whenever a
merge is completed with `--admin`, say plainly on the PR — and in the release
notes if it ships — that no human clicked approve, and that the merge
completed on the agent-review path with the review round as its approval. Say
**why** the bypass was needed, in the terms above: an account cannot approve
its own pull request, so an agent-authored PR the owner opened cannot be clicked
approved by the account that opened it.
Do **not** describe it as "bypassing a broken control": the control
is not broken, and that framing invites someone to remove it. What must never
happen is a tag that implies a *human* review it never had.
- **Never use `--admin` on a tier-2 PR.** An outside contributor's PR that
lacks an approving review from a collaborator is not finished, however clean
its agent round.
The bypass is for completing the owner's own reviewed work, not for
waving through someone else's.
An agent that is **not** acting for the owner prepares the PR, records the
review rounds, and hands it to the owner. The owner approves it as a human
first — that click is the tier-2 requirement and nothing else satisfies it —
and only then merges, or lets their own agent complete the merge.
**No automatic review requests: never notify a human by opening a PR.**
`.github/CODEOWNERS` declares nothing as of 2026-09-16, so GitHub requests
nobody when a PR opens — the four-handle ping that used to fire within a second
of every PR is gone, and PRs are opened **non-draft** again. (For an ordinary
PR the draft flag existed only to suppress that request; the release-claim lock
PR still opens as a draft for its own reason, see "Take the lock" above. A draft
cannot be merged, so for every other PR it now only delays the merge.) Two rules
replace the automatic routing:
- **Do not add reviewers to a PR, or tag a person in a comment, unless the
operator asks for reviewers on that PR.** A comment tag is what says a PR is
waiting on someone; `gh pr edit <n> --add-reviewer <handle>` is the deliberate
act when a human review is genuinely wanted.
- **A PR may be opened non-draft and merged as soon as its agent rounds are
clean and fresh.** Nothing is signalled by the draft flag any more. If you do
need to pause a PR in public (a stack that must not merge yet), a draft is
still the right marker — just do not read it as "the reviewers have been
notified".
### Scope round N+1 to the remediation delta
**A remediation is the most likely place in a PR for a new defect**, so the
round that follows one is scoped to `<previous-head>..<new-head>` rather than
re-reading the whole diff. Read that as a *detection* measure, not an
efficiency one: a whole-diff re-read spreads attention over hundreds of
already-approved lines, so the twenty that changed get the same share as the
rest. A remediation also arrives with social proof — a reviewer asked for it,
it is small, it is framed as closing something rather than opening anything —
and each of those lowers scrutiny at exactly the moment it should not.
This is not a hunch. Over one day, eight sessions reported this shape on
unrelated PRs — a fix reproducing its own defect at larger scale in a second
surface, a fix whose two halves drifted into describing different scopes, a
round-2 fix leaving a resumed conversation ~120 messages behind. The reports
are on the PR threads; what matters here is that the shape recurs often enough
to plan for.
Scoping to the delta is necessary but not sufficient. Three things make the
round conclusive rather than merely narrow:
- **Run the same probe against the previous head.** "Is this new?" is only
answerable by comparison — otherwise a reviewer finds a defect in the delta
and cannot tell whether the remediation introduced it or whether they missed
it twice.
- **Check the guard the remediation added can observe the defect it names.**
A fix can be right while its regression test is structurally incapable of
seeing what that same fix broke elsewhere — a real case closed one end of a
contradiction, opened its mirror at the other, and asserted only on the end
it had fixed.
- **Assert on what is painted, not only on the contract.** A round can verify
a contract is honoured and still miss that the resulting frame is wrong.
Two further corollaries. Do not re-open dimensions the delta cannot have
touched — a backend-only remediation keeps a design round valid, and
re-running it spends a reviewer on unchanged pixels. And when a rebase moves
the base, prove the content is unchanged (`git range-diff` plus byte-identical
`+`/`-` line sets) and then run **one** convergence round over
`<old-head>..<new-head>`, checking semantic conflicts only in files the
upstream also touched.
**Finally, a terminal approval is a property of a SHA, not of a PR.** Commits
landing after a clean round — including ones driven by a *different* stream,
such as a design fix after a terminal code round — make it stale, and the
merge gate needs a fresh round on the current head. "Everything else is green"
is exactly when that lapses. The one narrow exception: changes since the
review that are exclusively non-conflicting fixes from parallel approved
streams — a docs or nit commit from another round — leave it fresh.
## Security advisories
Any agent handling a reported vulnerability or a GHSA for this repository
**must read [`docs/security-advisory-runbook.md`](docs/security-advisory-runbook.md)
first** and follow its phases in order — in particular, the fix rides a normal
release window under "One release owner per window" above (the handler never
bumps `pyproject.toml`), the advisory is published only once that window's
version is on PyPI, and a CVE is requested immediately after publishing. A
published advisory is **not** the end of the task: GitHub's review for the
Advisory Database is asynchronous and unannounced, so the advisory is done only
when the runbook's propagation checks (GitHub Advisory Database, OSV, PyPI,
`pip-audit`) pass, or when the +3d/+14d follow-up wake and the escalation path
(PYSEC PR, Snyk disclosure) are recorded. `SECURITY.md` carries the public
commitment and the "Past advisories" table; update it in the fix's close-out.
## Versioning: choose the bump by materiality, not commit type
The version in `pyproject.toml` and the `vX.Y.Z` release tag are chosen by the
**user-facing materiality** of the change, **not** by its conventional-commit
type. A `feat:` commit is *not* automatically a minor. Using the commit type as
the version signal is how a run of bug-fix and reliability releases inflates the
minor number and drains its meaning — a minor should mark a step-function
improvement a user would notice and adopt, so that going from `0.N.x` to
`0.(N+1).0` still tells them something.
The bump is chosen **once per release window** by the release owner, for the
window as a whole (see "Releasing the stable `lop` runtime"). A PR argues for a
bump through its body's `Release: <patch|minor> — <impact>` line; it does not
apply one.
- **Patch (`0.N.x` → `0.N.(x+1)`) — the default; most releases are patches.**
Bug fixes, performance and reliability improvements (backoff, retries,
timeouts, tuning), refactors, internal cleanups, docs, and small
self-contained features that do not change what the product can fundamentally
do. A single small `feat:` commit is a patch. **When in doubt, patch.**
- **Minor (`0.N.x` → `0.(N+1).0`) — a material, step-function capability.**
Reserve it for a new surface or subsystem a user would notice and adopt: the
browser extension, the mobile relay, peer-to-peer session messaging. The test
is simple — if you cannot name the step-function capability in the release
title (`X.Y.0: <the new thing>`), it is a patch, not a minor. Several small
features bundled together are still patches unless one of them clears this
bar on its own — and that holds for a whole window: ten patches merged in
the same hour are one patch release, not a minor.
- **Major (`X.y.z` → `(X+1).0.0`) — only on explicit request.** Bump the
major version *only* when the developer explicitly asks for it, in the rare
case where the new version is considered a distinct product from its
predecessor. Never decide a major bump on your own judgement.
Because releases run frequently here, err toward patch: an under-called bump is
trivially corrected by the next release, while an over-called minor permanently
misreports how much changed.
## Visual validation: how to actually look at a UI change
This is a terminal UI. **A passing test is not evidence that a visual change
looks right**, and every spacing, layout, or animation change in this repo has
to be inspected as a rendered frame before it is claimed to work. The recipe
below is the one used for the usage-card spacing and the `/resume` picker; it
takes about a minute.
**When the app is available, the console is the first instrument, not the
emulator.** If the Local Operator desktop app is running and its console host
advertises, the `console` tool is present — `guide://console` is its playbook, and
`docs/CONSOLE.md` is the operator's view of the same thing — and the frame should
come from a console surface before the Textual compositor below is reached for.
Textual emulation has small visual and spacing differences from what people
actually see, and those differences are the size of the defects a still is taken
to catch: a reconstructed frame can be exactly right about a layout that looks
wrong on the user's screen. When the pane is displayed, the console photographs
the **real** pty in the app's own terminal — its font, its theme, its own grid —
and it always reports which
of the two it gave you: `rendered: "displayed"` is the app photographing its own
window cropped to the pane, while `"offscreen"` is a replay reconstruction from
the surface's record, faithful to that record but not a photograph of a live
screen. The tool's `keys` method sends named keys into the live program, so a
state that only exists under interaction is driven rather than simulated.
**Two of section 1's requirements move with the instrument, so carry them over.**
Isolate `HOME` and the config root before the app imports — section 1's isolation
step exists because a machine set to auto-capture otherwise yields a frame with
no prompt at all, and a console surface runs under whatever `HOME` it is given,
so that same silent emptiness would simply move onto the preferred path. And
check the size the frame came back at rather than the size you asked for: a
displayed surface is re-gridded from the pane's own rect (design §8.2) while an
offscreen one keeps its create-time grid (§8.4), and the screenshot result names
neither `cols` nor `rows`, so a published frame can be at neither the size you
requested nor one the result reports.
Sections 1-5 are **not replaced** by that. They remain the fallback when no app
is running — the tool is offered only while the app's discovery record advertises
a console, so a session built without the app has no tool at all, one that
outlives the app is told its surfaces ended rather than being handed a frame, and
a build that is neither displaying the surface nor carrying an offscreen capture
view refuses the screenshot with `capture_unavailable` instead of returning a
frame it does not have — and the instrument of record for what the console cannot
reach: a widget state you construct by hand, a CSS-less unit-test host. What a
frame can prove does not change either: a `read` is text and a `screenshot` is
pixels, neither is a design judgement, and the principles of sections 3-5 apply
to a console capture exactly as they do to an SVG — the procedures in them are
written for the compositor, which the console does not drive.
### 1. Render the screen to an SVG still
**This is the fallback path.** If the app is running with its console host
advertised, take the frame from a console surface instead (see above); the
helper below stays the instrument of record for what the console cannot reach.
**Use the faithful developer capture helper**, not a default Rich presentation
as a terminal-size measurement. All current shot scripts use
`scripts.visual_capture.save_capture(app, path)`: native 8x17px cells, system
monospace fonts, no fake window chrome or network font, plus `.geometry.json`
with screen/widget sizes and resolved font provenance. The grid and displayed
image scale are separate: a 150-column frame shrunk to a thumbnail cannot be
compared to a native 100-column terminal. See `docs/VISUAL_CAPTURE.md` for
calibration, explicit reference estimates, the finite gallery matrix, and
font/rasterization limits. Run `scripts/visual_gallery.py --list` before making
another sample script. Existing `app.save_screenshot` remains unchanged for
compatibility, but its default Rich geometry is not faithful visual evidence.
Four of these already exist and are worth reusing before writing a new one.
They capture the `ask` picker and the tool-approval prompt over a seeded
conversation, at any terminal size:
```sh
env -u NO_COLOR TERM=xterm-256color .venv/bin/python scripts/ask_shot.py out.svg 100x30
env -u NO_COLOR TERM=xterm-256color .venv/bin/python scripts/approval_shot.py out.svg 100x30
env -u NO_COLOR TERM=xterm-256color .venv/bin/python scripts/ask_user_repro.py out.svg 150x40 [ROW] [reveal]
env -u NO_COLOR TERM=xterm-256color .venv/bin/python scripts/ask_long_shot.py out.svg 150x40 [ROW] [reveal]
```
- `ask_shot.py` — the picker carrying SHORT descriptions, which is the shape
the card was originally measured against.
- `approval_shot.py` — the tool-approval gate, where each option's description
is the consequence of authorising the call.
- `ask_user_repro.py` — the user's reported frame: three options with
paragraph-long prose plus the free-text row, the reproduction for the
description-truncation report and for `ctrl+e`.
- `ask_long_shot.py` — a LONG question as well as long descriptions, which is
what makes the question's own wrapped lines compete with the option rows.
The last two take an optional `ROW` (arrow presses before the shot, so the
selected row's prose can be checked) and `reveal` (press `ctrl+e` first). Pass
both to check the property the reveal rests on: the card is the same height
whichever row the cursor is on, so two shots at different rows differ in their
text and in nothing else.
Both seed real transcript blocks first, which is what makes "does this surface
still let me read the conversation?" an answerable question rather than a
screenshot of an empty app. `approval_shot.py` takes a third argument, `focus`,
which puts focus in the composer before the shot — the state that used to send
the prompt's answer keys into the prompt buffer.
**The scripts isolate HOME/config before app imports** and approval-specific
fixtures force the approval gate on (`app._set_approve_all(False)`). Without
that isolation, the app reads the developer's own `tool_approval_mode`, so a
machine set to `auto` can capture no prompt at all. A new sample must call
`isolate_capture()` before importing app modules, not merely before the export.
For anything else, Textual can export exactly what it painted. Drive the app
with `run_test`, put it in the state you care about, and save a frame:
```python
# ${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/shot.py — `guide://scratchpad` explains the variable;
# env -u NO_COLOR TERM=xterm-256color .venv/bin/python "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/shot.py" \
# "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/out.svg"
import asyncio
import sys
sys.path.insert(0, "/path/to/local-operator") # repo root, so `tests.` imports resolve
from scripts.visual_capture import isolate_capture, save_capture
isolate_capture() # BEFORE app imports: isolate HOME, config and caches
from tests.unit.tui.test_app_pilot import FakeSession, _factory
from local_operator.tui.app import OperatorApp
async def main() -> None:
app = OperatorApp(lambda: _factory(FakeSession()))
async with app.run_test(size=(100, 30)) as pilot:
await pilot.pause()
# ... put the app in the state under test: press keys, push a screen,
# call a widget's show_*() directly ...
await pilot.pause()
save_capture(app, sys.argv[1])
asyncio.run(main())
```
**Use the real `OperatorApp`.** The lightweight hosts in the test files
(`_PanelHost` in `test_usage_panel.py`, `_PickerHost` in `test_session_picker.py`)
declare no `CSS_PATH`, so `local_operator.tcss` is **not applied** to them.
They are fine for asserting text content, and useless for judging padding,
colour, or placement — a still captured from one of them will not show a
stylesheet change at all.
### 2. Look at the image
An SVG is not something to eyeball as markup, and the reader does not render one
(it decodes raster images only). Render it and view it — open the `file://` URL of
the printed path in a browser tool and screenshot it, or render it to a PNG in the
scratchpad and read that back: a human or a vision-capable agent **sees the frame**.
### 3. Always capture before AND after
**Capture `before.svg` FIRST, before you touch a file.** The before-frame is
the cheapest artifact in this recipe and it only stays cheap while the tree is
still clean — write the shot script, capture, then start editing:
```sh
env -u NO_COLOR TERM=xterm-256color .venv/bin/python "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/shot.py" \
"${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/before.svg"
# ... now make the change ...
env -u NO_COLOR TERM=xterm-256color .venv/bin/python "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/shot.py" \
"${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/after.svg"
```
**Never `git stash` to get a before-frame.** Assume you are not alone in this
checkout: several agents routinely hold uncommitted work in it at the same
time, and a whole-tree operation is not a local undo — `stash` pockets every
peer's uncommitted work along with yours and hands it all back only if nothing
goes wrong in between. Nothing about the command tells you it did that. The
same applies to `git checkout -- <path>`, `restore`, `reset --hard` and
`clean`, and to any whole-file overwrite of a tracked file (`cp` over it, a
`>` redirect, an editor "revert"). Already edited and need a before-frame
anyway? Take it from a throwaway worktree, which cannot reach anyone's work
but yours:
```sh
git worktree add --detach /tmp/lo-before HEAD
ln -s ~/local-operator/.venv /tmp/lo-before/.venv
cd /tmp/lo-before && env -u NO_COLOR TERM=xterm-256color .venv/bin/python \
"${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/shot.py" "${LOCAL_OPERATOR_SCRATCHPAD:-/tmp}/before.svg"
git worktree remove --force /tmp/lo-before
```
The symlink is safe **here and only here**: a shot script starts with
`sys.path.insert(0, <repo root>)`, so it reads the tree it lives in rather than
the one the venv was installed from, and this checkout is deleted immediately.
Never copy that line into a feature worktree — see "Every feature worktree
owns its own venv".
**The same hazard has an additive form: `git add -A` and `git commit -a`.**
Every command named above takes something away, so the rule reads as being
about destruction — and an agent who had read it walked into this anyway,
because adding sounds safe. It is not: in a checkout other agents are working
in, both are whole-tree operations in exactly the sense above, sweeping peers'
in-flight work into **your** commit under **your** authorship. They differ only
in reach — `-a` takes every *tracked* modification, `-A` takes those and
untracked files as well — so `-A` is the wider hazard and neither is narrow.
Nothing useful in the output says so (`add -A` prints nothing at all;
`commit -a` prints a file count, never names or authorship), and the fabricated
authorship surfaces only later, out of `git log`. Stage explicit pathspecs —
`git add <the files you actually edited>`. The ban lifts where the destructive
one does: in a throwaway worktree nobody else touches, `-A` is fine, because
there is no other work for it to reach. Measured on `feat/move-command`
(PR #727), where two sessions shared one worktree: `5b1de59e1` at 10:59:36
swept a peer's in-progress `tests/unit/session/test_remote_move.py` into a
`fix(tui):` commit, and `type-check` failed at `test_remote_move.py:368:37`
(`CancelledError` not assignable to `Exception | None`) because the widening
that makes that test type-check landed 3m24s later in the peer's own commit
`06ee21d9e`. The PR's head was red for a defect its own intended change did
not contain.
**The undo for a mistaken edit is a reverse edit of your own hunk, never a
whole-file operation.** Everything above is written against a deliberate act —
clearing the tree to get a before-frame — so none of it intercepts the reflex
thirty seconds after you notice you edited the wrong thing, when the goal is
just to put it back and `git checkout HEAD -- <path>` is the first command to
hand. Same hazard, different mental context, and an agent who had read the rule
above walked into it today. What makes an operation unsafe is that **its unit
is the whole file rather than your hunk** — every command listed at the top of
this block, plus a `write` that re-emits a whole file. Treat the list as
examples and not as the rule, since enumerating is how `git add -A` stayed
unnamed until someone hit it. The ban lifts where the others do: in a worktree
nobody else is working in, or a throwaway, all of these are ordinary — the test
is exclusivity, not ownership, because a worktree can be yours and still have a
peer in it, which is exactly how #727 above happened. Where you are not alone,
undo your own change the way you made it — a targeted reverse edit of exactly
the lines you touched, leaving every other line as you found it — and better
still, avoid needing the undo by checking `git rev-parse --show-toplevel`
before a write, since the mistake that provokes the panic is usually aiming the
edit at the wrong checkout. The undo rule limits the blast radius; the path
check prevents the event. Today a subagent editing `.github/workflows/ci.yml`
in the shared checkout instead of its own worktree ran
`git checkout HEAD -- .github/workflows/ci.yml` to take it back, and destroyed
a peer's unstaged work with its own: on the order of 27 lines, from a diffstat
of roughly +23/−37 against HEAD of which its own edit was about +19/−10 —
figures reconstructed from the session log, not re-derived, because the content
is gone. Unstaged content is never hashed into the object store, so nothing
recovers it: `git diff --cached` on the path was empty, all 283 entries under
`.git/lost-found/other` were searched and held no candidate, and no commit
after the incident touches the path. That checkout held 1,611 uncommitted paths
from concurrent sessions when this was written, which is why a whole-file
operation there has a blast radius you cannot see before you run it. One probe
to avoid while investigating such an accident: `git fsck --lost-found`
**writes** those entries rather than reporting them, so it mutates the very
tree under investigation — plain `git fsck` lists the same dangling objects and
creates nothing.
Two stills side by side catch what a single "looks fine" never does. The
usage-card round found a **pre-existing** bug this way: the after-frame had a
scrollbar the before-frame did not, which turned out to be any tall overlay
pushing the screen's virtual height past its size — costing two cells of width
and reflowing the transcript behind the popup.
### 4. Check the numbers behind the frame, not just the pixels
Stills show you the symptom; the widget's own geometry tells you the cause.
Useful probes:
- `widget.size` (content box) vs `widget.styles.height` (border box — Textual
sizes **border-box**, so a widget that pins its own height must add its
padding back or it clips its own last rows).
- `app.screen.virtual_size` vs `app.screen.size` — if virtual exceeds actual,
something is making the screen scrollable, and on this app that is always a
bug (the transcript scrolls; the input is docked).
- `app.screen.show_vertical_scrollbar` — a scrollbar appearing is also a
silent two-cell width loss.
- The `render_lines_for_test()` helpers on `UsagePanel` and
`SessionPickerScreen` return the plain strings a user reads, which is the
right thing to assert in a test.
### 5. Animation and multi-frame changes
For anything that animates or settles, capture **consecutive** frames
(`await pilot.pause()` between saves) and compare them. If the first painted
frame differs from the settled frame, the layout is reflowing after paint —
that is visible to the user as motion, whether or not anyone intended an
animation. Frames should be identical once settled.
The SVG goldens under `tests/unit/tui/__snapshots__` are a local design aid,
not CI: Textual's SVG output is not byte-stable across interpreters or OSes,
so they are opt-in (`LO_RUN_SNAPSHOTS=1`) and regenerated with
`--snapshot-update`. Do not add a golden as a substitute for looking at the
change.
### 6. Capturing a browser surface: headless Chrome, never a window
Sections 1-5 are about Textual, which is the bulk of this UI. The browser
extension's popup and options page are the other user-visible surface here, and
they are captured out of a real Chrome rather than out of a pilot. **Any Chrome
an agent launches for testing, capture, or CDP work runs `--headless=new`.**
**This section is for people and agents developing the extension itself**, where
the artifact under test is `extension/dist` and it has to be loaded into a
browser that does not have it yet. It is not how anyone *uses* the browser tool:
a user — or an agent doing ordinary browser work in any repository — drives the
browser that is already paired, per `guide://browser`, and never starts a second
one. Keep the two apart in both directions. Do not send contributors to the user
guide for harness flags, and do not let this recipe leak into the guide and read
as advice to spin up Chrome for everyday browsing.
This is not a preference. On 2026-09-07 agent test harnesses driving the
extension launched *headful* Chrome, and the windows took focus from the
operator repeatedly while he was working in another application. The whole
browser story on this machine is built on never doing that — the extension
creates its tab with `active: false`, every cmux command passes `--focus false`,
and `guide://browser` carries a "Focus safety — never steal the user's focus"
section. A harness that pops a window on screen violates the same principle the
product spends real effort upholding, and "it is only a quick check" is exactly
the reasoning that produced the incident. **Never stealing focus is the general
rule for every browser this project starts, in any context**; headless is simply
how a harness satisfies it, and `open -g` is how a real-browser launch does.
New headless is not the old one: it is the same browser binary with no window,
so extensions, DevTools/CDP, screenshots and synthetic input all work. Measured
on this host (Chrome 152.0.7977.76, `--headless=new`), the built
`extension/dist` loaded, its popup rendered at an explicit 300x600 viewport, and
`Page.captureScreenshot` returned a 34 KB PNG showing the real pairing form;
`Input.dispatchMouseEvent` dispatched. Nothing appeared on screen.
The launch itself, with the flags this section requires:
```sh
# Throwaway profile, headless, no keychain prompts, and a port Chrome picks.
profile=$(mktemp -d /tmp/lo-harness.XXXXXX)
"/Applications/Google Chrome.app/Contents/MacOS/Google Chrome" \
--headless=new --user-data-dir="$profile" \
--remote-debugging-port=0 \
--use-mock-keychain --password-store=basic \
--no-first-run --no-default-browser-check about:blank &
chrome_pid=$!
# Chrome writes DevToolsActivePort asynchronously, so WAIT for it rather than
# reading it straight after launch: an immediate read gets a missing or empty
# file and the connect fails intermittently, which reads like a flaky harness.
until [ -s "$profile/DevToolsActivePort" ]; do sleep 0.2; done
port=$(head -1 "$profile/DevToolsActivePort")
```
Drive it over CDP on `$port` per the steps below, then tear it down — by PID,
because this snippet's Chrome is in your own process group:
```sh
# SIGTERM to the browser PID alone is NOT enough, and what it misses varies run
# to run -- measured on Chrome 152 across 11 runs it left anywhere from 0 to 5
# helpers behind, with no stable pattern by timing. That is exactly why a quick
# test "passes": some runs really do come back clean. Never conclude from one.
# Sweep your OWN profile afterwards, and assert the count rather than trusting
# it. The unique mktemp pattern is what scopes the sweep to your Chrome and
# never the operator's.
kill "$chrome_pid" 2>/dev/null
sleep 2
pkill -f "$profile" 2>/dev/null
pgrep -f "$profile" | wc -l # assert 0, every run
rm -rf "$profile"
```
**The process-group teardown below applies to a harness that owns its own
group, not to this shell snippet.** A backgrounded `&` puts Chrome in the
*caller's* process group, so signalling that group signals whatever is running
the script — verified here: the child inherited the caller's pgid and
`kill -TERM -$pgid` terminated the calling shell with `-15`. Kill the captured
PID from a shell; use the pgid form only from a harness that started Chrome
with `start_new_session=True` (Python) or an equivalent, which is what puts it
in a group of its own. `setsid` is not the escape hatch on macOS — it does not
exist there.
**Load the extension over CDP, not `--load-extension`.** Branded Chrome removed
that switch in 137 (`PSA: Removing --load-extension flag in Chrome branded
builds`, chromium-extensions), and the
`--disable-features=DisableLoadExtensionCommandLineSwitch` escape hatch is gone
too. It does not error — it is **silently ignored**, so a harness using it gets
a Chrome with no extension and an agent that concludes headless "does not
support extensions". Verified here: with `--load-extension=extension/dist` the
only `chrome-extension://` targets were Chrome's own built-ins and our manifest
name was absent; `Extensions.loadUnpacked` over CDP returned our id and the
popup drove normally. `docs/design/browser-extension-e2e.md` records the same
finding on Chrome 151, and it was re-confirmed on **Chrome 153.0.8010.53**
(2026-09-20, PR #1335's E2E): `--load-extension` loaded nothing, with or without
`--disable-extensions-except`, while `Extensions.loadUnpacked` worked first try.
The failure is worth recognising because of its SHAPE rather than its silence: the
extension's own pages still resolve at `chrome-extension://<id>/…`, so the harness
gets a real-looking options page whose `chrome.runtime` and `chrome.storage` are
both `undefined` — which reads as a broken manifest, not as an unloaded extension,
and cost one round of debugging. Check `chrome.runtime.id` on any extension page
before believing a manifest defect.
**Size the viewport with `Emulation.setDeviceMetricsOverride`, not
`--window-size`.** Headless inherits no real window, so it defaults to whatever
the platform picks — measured 756x469 at `dpr=1` on this host with no flag —
and a frame whose dimensions were assumed rather than set is not evidence of
anything. `--window-size` reads like the remedy and quietly is not: on Chrome
152 the width clamps at a 500px floor and the height loses 87px to chrome, so
`--window-size=300,600` produces **500x513** with no error. Measured, fresh
profile per run: `300,600` → `500x513`, `400,600` → `500x513`, `500,600` →
`500x513`, `1000,800` → `1000x713`, all at `dpr=1`. The CDP override returned a
true `300x600 dpr=2`, controls `deviceScaleFactor` (which the flag cannot), and
applies even to a browser started with the clamped flag — so it is the single
way to set a capture viewport here. Following the flag instead is exactly the
assumed dimension this paragraph forbids.
**Never pair a headful before-frame with a headless after-frame.** Device pixel
ratio and scrollbar presence differ between the modes, so a cross-mode pair
shows differences the change did not cause — the §3 trap, with the mode as the
hidden variable. Recapture both sides in the same mode.
The hygiene flags below are not decoration; each one is an incident:
- **A unique `--user-data-dir` per launch**, under `/tmp`. Never the operator's
profile, and never a shared path: a second Chrome on a live profile dir is
how a run corrupts the one the extension is paired to.
- **A port Chrome picks, not one you choose: `--remote-debugging-port=0`, then
read the real port from `"$profile/DevToolsActivePort"`.** A fixed port is a
shared path in the same sense as a shared profile dir, and this machine runs
many agent sessions concurrently. Measured with two harnesses on distinct
profiles both requesting 39222: the second started fine, wrote **no**
`DevToolsActivePort` and reported **no** error, while the port answered as
the *first* harness's browser — so the second agent would drive another
session's Chrome, capturing its pages and closing its targets. With port `0`
the same pair bound 57827 and 57829 and each drove its own. Chrome writes
that file only when it chose the port itself (under an explicit port it is
absent), so reading it is both the fix and the check that you applied it.
- **`--use-mock-keychain --password-store=basic`.** Real Chrome on macOS
otherwise reaches for the login keychain and raises modal "Keychain not
found" dialogs on the operator's screen — reported from a run of ~37
concurrent instances. These are on-screen dialogs, so they defeat headless on
their own.
- **`start_new_session=True` plus process-group teardown** (SIGTERM to the
pgid, SIGKILL if it does not settle) **and an `atexit` handler**. The reason
is not that `terminate()` fails when it runs — on Chrome 152 it reaped the
whole tree cleanly — it is that the harness does not always get to run it. A
harness killed mid-run leaves the browser reparented to PPID 1 holding its
profile dir: measured here, `SIGKILL` to the harness left the entire Chrome
tree alive with the browser at `PPID 1` (helper counts vary per run, so
compare against zero, never against a remembered number). A previous session
reportedly leaked 47 that way. Own the process group so the survivors are
addressable, and assert
zero afterwards (`pgrep -f <your unique profile prefix>`) rather than assuming.
- **Never touch the operator's own running Chrome.** Match on your own profile
prefix, never `pkill -f "Google Chrome"`.
One thing this rule does **not** cover: `open -g -a "Google Chrome"` in
`guide://browser`. That wakes the **operator's real paired browser** so the
extension reconnects — a different activity from launching a test harness, and
already background-launched by `-g`. It is correct as written; do not
"headless" it. The distinction is the profile: the operator's logged-in browser
is woken in the background and never given debug flags, while a harness browser
is a throwaway profile that is headless, debug-ported and torn down.
And it does not cover verifying a change *through* the extension once it is
loaded — a rendered page, a flow, a screenshot for a PR. That is ordinary
browser work: use the `browser` tool against the paired browser like any other
session. Reach for a harness only when you need a browser that does not yet
have the build you are testing.
### 7. Evidence goes on the PR, never into the repository
**Do not commit PR evidence artifacts** — before/after frames (SVG or PNG),
screenshots, terminal byte captures, measurement logs, review-round
transcripts, or a `bench/`, `docs/evidence/<change>/`, `docs/assets/pr-<n>/` or
`docs/pr-<n>/` directory of any kind. Those directories used to exist and
were removed in one sweep: they had grown to ~60 MB of frames that nothing in
the code or tests loaded, that every clone paid for forever, and that
described a UI several releases out of date. `bench/` was the same mistake in a
second place — 13 JSON dumps and the two hand-built markdown tables beside them,
written by the `scripts/bench_*.py` harnesses and read by nothing at runtime —
and was swept on its own: a measurement store is regenerable, so it belongs with
the PR that cites it, and it is ignored now (`.gitignore`).
The evidence still has to exist; it just lives where the review does. Attach
images to the PR description or the review comment (drag them into the GitHub
editor, or `gh pr comment --body-file` with the rendered markdown), paste
measured numbers and commands inline, and keep the capture *scripts* — the
reusable part — under `scripts/` when they are worth reusing. A PR reviewer
reads the PR; a future agent reads `git log` and the PR it links to; neither
needs the frames in the tree.
What DOES belong in the tree: the shot scripts in `scripts/`, the opt-in SVG
goldens under `tests/unit/tui/__snapshots__` (a design aid, see above), and
design/proposal documents under `docs/` that would be read on their own. A
document whose only purpose is to hold a PR's proof is a PR comment, not a
doc.
Comments and docstrings still cite a few of the removed directories by path
(`docs/evidence/cmd-chords/MEASURED.md`, the compaction-ruler measurements, the
`bench/*.json` pairs the analytics and info-snapshot code quote, and so on).
Those citations are kept as-is — the measurements they name are real and the
reasoning built on them still holds — and the files are one `git show` away.
The commit to retrieve each from:
| directory | `git show <sha>:<path>` |
|---|---|
| `cmd-chords`, `aside-chord` | `f3ae0441`, `1634a53b` |
| `compaction-ruler` | `9eb9bb33` |
| `compaction-advisor` | `c60cf8c2` |
| `fork-ux`, `fork-cache` | `a70e3460` |
| `browser-extension` | `39691ea0` |
| `sibling-modes-boot-layout` | `2c4ebc77` |
| everything else under `docs/evidence`, `docs/assets/pr-*`, `docs/pr-280`, `docs/performance` | `fc80a967` (the last `main` before the sweep) |
| `bench` | `ba225070` (the `main` this sweep branched from; the whole store is still present there — `git show ba225070:bench/README.md` retrieves the tables, `git show ba225070:bench/analytics-rollup-after.json` the numbers behind them) |
## Timing, flakes, and how to assert that something is fast
A test that spends a fixed budget waiting for asynchronous work, then asserts
as though the budget were a guarantee, is not a test — it is a bet on machine
load. On an idle dev box the budget is 20-50x more than the work needs, so it
looks solid; under `-n auto` contention on a 4-vCPU CI runner the work
stretches, the budget does not, and the suite goes red for reasons that have
nothing to do with the code under test. This repo has paid for that lesson
repeatedly (#122, #373, #461, #486, #496, #498, #499), so the rules below are
not style preferences.
### Wait on the event, never on the clock
**Do not** poll a wall-clock deadline and assert afterwards:
```python
await asyncio.sleep(2.0) # "surely it finished by now"
assert store.detail is not None
```
**Do** wait on a publication from the code under test.
`tests/unit/harness/test_comms.py` has the pattern: `ChangeSignal` subscribes
to notifications the code already emits, and `wait_for` re-tests the predicate
after each one. The subscriptions are what make it work, so build and release
it in full:
```python
signal = ChangeSignal().watch_comms(comms).watch_session(parent)
try:
await wait_for(lambda: comms.session_dir_of(job_id) is not None, signal=signal)
finally:
signal.close()
```
A bare `ChangeSignal()` subscribes to nothing, so `wait_for` blocks the full
`DEADLOCK_GUARD_S` and then reports "it is wedged, not merely slow" — a
confident diagnosis of the wrong thing. Watch the sources you need.
There is no elapsed-time comparison anywhere on the success path: the wait
lasts exactly as long as the work does. The deadline exists so a genuine hang
fails the run instead of blocking forever. It is a backstop, not the assertion.
When there is nothing to subscribe to — an in-process coroutine the test
itself scheduled — bound by **loop turns** (`MAX_PUMP_TURNS`) rather than
seconds. A turn count survives contention that a wall-clock budget does not.
If you catch yourself tuning a `sleep` until CI goes green, you are
calibrating a bet, not fixing a test.
### Prefer a structural invariant to a numeric one
The strongest version of "this work happened off the event loop" is not a
timing bound at all — it is thread identity.
`test_store_maintenance_callbacks_run_off_the_event_loop_thread` wraps the real
maintenance callbacks, records `threading.get_ident()` inside each, and asserts
none of them equals the loop thread. That assertion cannot flake: it is a fact
about where the code ran, not how long it took, and it fails deterministically
the moment someone drops an `asyncio.to_thread`.
This is not a stylistic preference — it is where this repo's timing bounds keep
landing. `tests/unit/session/test_launch_subagent.py` carries the full account
under "WHY THIS IS A STRUCTURAL SPY AND NOT A TIME BOUND": a flat 1.0 s wall
bound failed, a calibrated bound failed, converting to `time.thread_time`
removed the *load* sensitivity but not the *core-speed* sensitivity, and the
conclusion was that **no portable numeric bound exists** for that site — the
only window left was between CI's healthy 1156 ms and this box's 1315 ms
regression, which is too narrow to sit a bound in. It was converted to a
structural spy. Read that docstring before you reach for a number.
Reach for a number only when no structural fact expresses the property.
### If you must measure, measure CPU, not wall time
Wall-clock gaps between probe wakes conflate two unrelated things: the event
loop genuinely blocking, and the OS simply not scheduling the process. On a
loaded runner the second dominates — measured here at 525-668 ms of "stall"
with the loop idle and nothing blocked.
Use `time.thread_time()`, which is per-thread and excludes time asleep or
waiting on the GIL, so a sample is large only when the loop thread really ran
without yielding. Two probes exist and they are **not interchangeable**:
`LoopCpuProbe` (`tests/unit/tools/test_loop_liveness.py`) records CPU only;
`StallRecorder` (`tests/unit/test_tui_responsiveness.py`) records CPU **and**
wall, and asserts both. If you need the blocking-sleep backstop described
below, use `StallRecorder` or add the wall assertion yourself — reusing
`LoopCpuProbe` silently drops it.
The distinction is measurable, not theoretical:
| scenario | wall gap | CPU gap |
| -------------------------- | ----------- | ------- |
| 60 MB sync parse on loop | 96 ms | 96 ms |
| pure `time.sleep` on loop | 306 ms | 0.1 ms |
| loop idle, OS-starved | 525-668 ms | 0.0 ms |
Note the middle row: the CPU clock is **blind to a pure blocking sleep**. If
that shape matters at your site, keep a wall-clock assertion alongside it, with
a ceiling set for catastrophe (seconds) rather than for precision.
### Calibrate ceilings from CI, never from your laptop
This is the mistake that cost three PRs. Local measurements of the same
loaded-bar sites read 36-38 ms and ~0 ms; the CI runners recorded **206, 264,
321, and 413 ms** of entirely legitimate loop CPU at those same sites. Every
ceiling calibrated from the local numbers flaked a green tree within a day.
A slower core burns more CPU-seconds on identical work, so a bound calibrated
on a dev box is not a bound on CI — see the `test_launch_subagent.py` account
above, where that spread ended in abandoning the bound entirely. If you have
established a number is genuinely the only option:
- Take the healthy maximum from **CI logs across several runs**, not from one
local run and not from one CI run.
- Leave real headroom above it, and **write the dataset and the margin into the
docstring** so the next person is not re-deriving it from scratch.
- A number copied from an older comment is not evidence. The 549 ms figure in
this repo's history is a **wall-clock** measurement from a probe that no
longer exists; citing it to justify a CPU ceiling compares two different
quantities.
When a green tree trips the ceiling, **find out which it is before touching the
number**. Either the tree is not as green as it looks, or the ceiling is
miscalibrated — and "raise it" is only correct in the second case. Reproduce
the sample, check whether the same test fails on `main` and on unrelated
branches, and confirm the work under the probe is the legitimate kind. Widening
a bound because it went red is how a guard stops guarding.
### A pytest run that STOPS EARLY with exit 0 is not a pass
Check the count, not just the exit code. `main()` is called in-process by the
suite (`assert main() == 7`), so anything `main()` does to *its own process*
happens to pytest. A change that added an `os.execv` early in `main()` replaced
the running pytest with a fresh interpreter: the file printed dots to 57%, then
stopped, and the shell reported **exit 0**. No failure, no traceback, no
summary line — 42 of 98 tests simply never ran, and every gate looked green.
It surfaced only because the summary line was missing from the output. So:
`98 passed` is evidence; a bare exit code is not. The same shape hides behind
`| tail`, which reports the *pipeline's* status — see the `rc=126` note in the
Environment section, which is this hazard's twin.
Anything a process can do to itself — `execv`, `os._exit`, `chdir`, signal
handlers, `sys.exit` in a library path — needs a guard proving it is a real
launch before it fires. `procname.is_own_launch()` is that guard for the
branded re-exec, and its test is the one that pins this.
### Prove the test can still fail
A guard that cannot go red is worse than no guard, because it is believed. That
holds for every kind of test, not just the timing ones this section is about,
and it fails in more ways than a widened threshold. Four instances landed in a
single session: a test that asserted an assumption instead of behaviour; a test
named for a mechanism that still passed with the mechanism stubbed out; a QA
cell that passed *vacuously* because the thing it checked was absent rather
than correct (`cards.get(...) != "interrupted"` is true when there is no card);
and an entire suite of 36 that stayed green when the fix under it was reverted,
because the one test reaching that code stubbed its collaborators with
`lambda *a: object()` and discarded the value the fix changed. Each looked like
coverage on the checks list.
So whenever a test is the evidence for a claim — a review round, a QA cell, a
PR's "this is fixed" — **make it fail on purpose before you rely on it**: run
it against the unfixed tree, or revert the fix in place, or mutate the
mechanism it names. If it passes either way it is decoration, and the honest
next step is to fix the test or drop the claim.
When you change how a test detects a regression, reintroduce the regression and
watch it fail:
```sh
# put the synchronous call back on the loop, or delete the to_thread
ISO=$(mktemp -d)
env -i HOME="$ISO" LOCAL_OPERATOR_CONFIG_DIR="$ISO/.local-operator" PATH="$PATH" \
.venv/bin/python -m pytest tests/unit/test_tui_responsiveness.py::<test> -n0
# expect a failure with a message that names the real cause, then revert
```
This is not hypothetical. In `test_tui_responsiveness.py` the loaded bar was
raised to 500 ms to stop it flaking, which meant a 200-500 ms stall at the
boot and turn sites — whose only guard is that bar — would pass unnoticed.
That trade was made deliberately and on evidence: those sites recorded 321 ms
and 413 ms of *legitimate* loop CPU on CI, so a tighter bar failed green trees
instead. The strict 50 ms bar on the reconnect/connect sites was left alone
precisely so the 90-130 ms parse regression stayed catchable. When you widen a
ceiling, name what you just stopped catching — and record the measurements that
forced your hand.
Then check the other direction: run the file several times under load
(`for i in $(seq 1 5)` with a few CPU spinners) and confirm it stays green.
### A dead instrument returns a reading, not an error
"Prove the test can still fail" covers the guard. This is the half underneath
it: the instrument reading the guard can be dead too, and a dead instrument
does not raise, warn, or emit garbage — it emits a well-formed answer to a
question nobody asked. Its null result is indistinguishable from a real
reading in **both** directions, and care does not defend against that. A
canary does: before trusting anything an instrument says about the case you
actually care about, push a **known-positive and a known-negative through the
same instrument** — the tracer, the log parser, the grep, the render probe —
and watch it tell them apart.
Every instrument below succeeded, in the sense that none of it errored:
- A syscall tracer blind to `stat`/`lstat` on CPython 3.14, used to verify a
gate whose entire purpose was preventing an existence oracle.
- "Zero network calls", reported from a port collision that meant the request
log was never written.
- A mutation harness that scored every mutant CAUGHT against a suite that
never ran — `pytest-timeout` was absent, the run exited `rc=4`, and the
harness read the exit code as the mutant dying. Two sessions hit this
independently.
- A mutation that silently no-op'd because a formatter had rewrapped the
anchor line it patched, so the score measured an unmutated file.
- Fifteen planted-violation cells reporting PASS without their mutation ever
landing; the one apparent catch was a `SyntaxError`.
- A containment grep reporting "canary absent, len=2" — the `2` was `[]`,
serialised. An empty container greps clean.
- A footer legend certified at a width no caller can produce: the picker's
content box is capped at 74 cells and the hint row carrying the legend
wants 88, so the legend could only ever appear by displacing another hint.
Passing the function a width the terminal cannot reach tests a code path
that does not exist.
The reciprocal is equally real, and just as invisible — the instrument is dead
*to the thing you changed*, so it fails what should pass:
- A stale regex expecting a symbol a later round had renamed, reporting a
correctly-released binary as MISSING ITS OWN FIX — a hunt for a publishing
failure that never happened.
- A `MagicMock(spec=...)` that measured the negative branch, producing a
"cheap" figure for an expensive operation.
- A capture run under an env var that disabled the very effect being measured,
showing a defect "reproducing" after it was fixed.
- A UI frame captured with no factory wired: the command refused, the refusal
rendered as an ordinary notice, and the still showed a plausible frame of
the feature never opening.
- A `screen.render_line(y)` probe read outside a paint cycle returns blank
strips — 22 non-blank rows read as blank, a false BLOCKER against a correct
implementation.
Name the asymmetry, because it decides where your paranoia goes: a false pass
ships a defect; a false failure sends someone hunting a bug that does not
exist. The second is cheaper, but it is not free — and the reading itself
looks identical either way, which is exactly why the canary is the only
defence.
One species sits off that axis, being about what the instrument does to the
subject rather than which way it errs: **the instrument cures it.** A fixture
pinned `history_generation=1` against a snapshot at state 0, permanently
invalidating the follower, so every ordinary delta restarted a refresh — and
that refresh serviced the orphaned debt the test existed to catch. It passed
because the harness was broken in a way that repaired the defect.
That beats all three defences above: the instrument is live and
discriminating, the mutation lands, and the pre-fix tree goes green because
the cure operates there too. Both sides green reads as "already fixed" or "the
test does not discriminate", never "my fixture is repairing the bug".
The defence is a precondition on the precondition: **assert the fixture's
starting state is production-reachable**, at use time rather than at capture —
state drifts out of a failure condition as readily as it is mis-set into one
production never produces.
Two rules sit beside this one, each earned the hard way:
**When you fix something that raises or refuses, verify it still raises where
it should** — otherwise you have proven only that you removed a check. A
reauth fix correctly fell through when nothing was stored, but also fired when
the DELETE itself failed: it logged in over a surviving credential and
reported success for an account switch that never happened. The reviewer then
narrowed the fix to break it again, and all 520 tests still passed — the suite
was structurally blind to the distinction the fix depended on. A separate
session converged on the same rule from the opposite direction: an integrity
refusal that survived a heal.
**Quote the query, not just the result.** A `df` against `/` — which reports
how full the sealed system volume is, not the writable `/System/Volumes/Data`
that every byte you write lands on — and a `dict.get()` against the wrong
nesting level both return clean, confident, wrong answers. State the basis
alongside the number so a reader can check the *question*, not just the
digit. A disk figure taken against the wrong mount propagated through a dozen
sessions, each one confirming it back against the same wrong basis; every
confirmation strengthened a number that was never right.
And the corollary, for when the canary finally catches your own earlier
finding: **withdraw it, do not defer it.** A deferred wrong finding is a
landmine — the next reader treats it as unresolved-but-real and "corrects" a
correct value into a wrong one.
### When a test is already flaking
Reproduce and classify before touching a threshold. A single failure out of
~9900 on a different timing-sensitive test each run is a flake, not a
regression; the same test failing on several branches at once — check `main` —
is repo-wide and not yours. Re-run to confirm, then fix the measurement rather
than widening the bound. Do not merge a red head on the assumption that it is
"probably the known flake" without checking the log.
### Two things this section cannot do for you
**There is no `pytest-timeout` in this suite.** A test that waits forever hangs
its CI job until the workflow's `timeout-minutes` reclaims the runner
(`timeout-minutes: 20` for `test` in `.github/workflows/ci.yml` — quoted from
the file, and that ceiling exists because a job once held a slot for 3h38m).
So an unbounded wait is not merely slow, it is expensive for everyone queued
behind it — which is the other half of why `wait_for` carries
`DEADLOCK_GUARD_S`.
**A deadlock defeats every technique above.** If the failure mode is the event
loop freezing at the C level rather than running slowly, a Python-level probe
reports nothing — the coroutine that would record the sample never gets
scheduled either. Nor do the obvious fallbacks: `watchdog.py` documents, with a
measurement, that a `threading.Thread` watchdog needs a GIL the wedged thread
never releases, and `pytest-timeout`'s default `signal` method never fires
because a Python signal handler only runs between bytecodes. The pre-fix code
ignored a 20 s thread watchdog and had to be `kill -9`'d.
That class is covered by `tests/e2e/watchdog.py`, whose
`faulthandler.dump_traceback_later(exit=True)` is armed in C and fires from a
separate OS thread, so it survives a wedged interpreter and takes the process
down with a full thread dump. It runs as its own CI stage (`tui-e2e`, both
Linux and macOS) rather than in the unit run, since firing it under `-n auto`
would kill a worker carrying unrelated tests. If you are guarding against a
hang rather than a stall, that is the file to read.
## TUI conventions worth knowing before you edit a widget
- **Do not shadow Textual's API.** `Widget` already owns `query`, `visible`,
`render`, and `_render`; a property or method with one of those names breaks
focus, layout, or paint from inside your widget, and the traceback points
somewhere else entirely (`'str' object is not callable`,
`'Text' object has no attribute 'render_strips'`). Name list state
`visible_rows`, filter state `filter_query`, and renderers `_card_text`.
- **Overlays float; they must not disturb the layout beneath them.** Cards on
the `toast` layer are sized by the widget and positioned by an offset. Keep
`overflow: hidden` on `Screen` so a tall overlay cannot introduce a
scrollbar, and `event.stop()` in any mouse handler so one gesture does not
move both the card and the transcript.
- **Wrapping vs clamping.** Arrow keys wrap (a discrete, deliberate press);
wheel and page movement **clamp**. A scroll gesture that teleports to the
other end of the list reads as the list resetting itself.
**Documented exception — a list that IS the whole page clamps its arrows
too. Today that is `/settings` and nothing else.** The wrap rule is written
for a picker: a short list overlaid on a screen the user is still looking at,
where coming round is a shortcut to a row already visible. It does not
transfer to a full-page mode whose list is several times its viewport
(`/settings`: 60-odd rows against 14 at 100x30). There the bottom is a
destination the user travels to deliberately, and wrapping threw them out of
the section they were working in and scrolled the viewport with them, which
is what the report against v0.43.0 said. So `SettingsView.action_move`
clamps, and every movement on that page clamps with it — a page where `down`
clamps and `pagedown` wraps is worse than either rule applied uniformly.
Tab or pane CYCLING is outside both rules: a small closed set of tabs that
are all on screen (`←→` between the teams and agents panes) has no ends to
clamp against and nothing that scrolls, so it keeps cycling.
- **One gesture owns the viewport at a time.** A list inside a
`ScrollableContainer` has TWO positions that can each move the view: the
container's own scroll offset (wheel, scrollbar drag) and a cursor whose
"scroll the selection into view" recomputes the offset from the selected row.
Let both drive the same view from the same gesture and they overwrite each
other — on `/settings` the wheel moved the cursor and then re-derived the
viewport from it, so wheeling to the bottom and giving one more notch snapped
the view back to the top (reported against v0.43.10).
The split that holds: **the wheel and the scrollbar move the VIEWPORT and
leave the cursor alone; keys that move the CURSOR scroll it into view.** The
cursor is then allowed to go off screen, which is what every editor and list
UI does and the only arrangement in which the scrollbar thumb is telling the
truth.
Two traps when handing the wheel to the container. Textual stops the wheel
event on the container *while it can still scroll*, so a widget-level
`on_mouse_scroll_*` handler runs **only at the ends of the travel and outside
the container's region** — which is why this defect looked like "it bounces
when I reach the bottom" and why the same notch behaved differently over the
pane than over the list. And accumulate from `scroll_target_y` with
`immediate=True`, not `scroll_relative`: the default defers to
`call_after_refresh`, so notches arriving in one burst all read the same stale
offset and a fast flick collapses to a couple of rows.
Step the handler by the LIVE `app.scroll_sensitivity_y`, never by a constant
copied from it. It is a per-instance attribute set in `App.__init__`, so a
hardcoded step that matches today desynchronises the moment anything changes
it — measured at 4.0, the container applied 4 rows per notch over the list and
the widget handler 2 everywhere else, which is the position-dependence above
reappearing.
Finally, **the cursor being allowed off screen is a contract with the keys**:
once the wheel can leave it behind, every key that ACTS on the cursor has to
scroll it into view first, or it writes to a row the user cannot see and the
frame does not change. Reveal-then-act, not an interlock that makes the first
press a no-op — the press should still do what it says on the first try.
The trigger is "the list is the whole page", NOT "the list scrolls", and the
difference is load-bearing: `model_picker.move` windows a catalogue of
hundreds of rows and still WRAPS, correctly, because it is an overlay on the
conversation rather than a place the user has navigated to
(`command_picker.move` likewise). Do not read this exception as licence to
clamp them. `session_picker._move_to` already clamps, and it is a full
surface too.
- **Rows are load-bearing.** The welcome splash is content-sized and rests on
the input card, so anything that changes its line count moves the whole
block. Animated content must reserve its row even when it has nothing to
show.
- **Comments explain the why.** This codebase documents constraints and the
failure that motivated the code, not what the line does. Match that density;
a comment that restates the code is noise, and a change with a non-obvious
reason needs the reason recorded.
## Adding a configuration key
**Every new configuration key must also be added to the `/settings` registry
(`local_operator/settings_io.py`).** A key that only exists in the code that
reads it is invisible: `/settings` is the surface users browse to discover what
is configurable, and `lop config edit`/`config list` resolve names out of that
same registry, so an unregistered key can only be set by someone who already
knows it exists and edits `config.yml` by hand.
Adding one means, in the same change:
1. A `Setting` in `SETTINGS`, in a declared `Section`. If the section is new,
add it to `SECTIONS` too — and pick its `Scope` honestly. Scope is uniform
within a section by construction, so a key that takes effect immediately does
not belong in a section labelled "new sessions"; split the section instead.
2. **A real module-level default constant next to the code that reads the key**,
mapped in `_consumer_defaults()` in `tests/unit/test_settings_io.py`.
3. `path=` spelled deliberately. Almost every key is a genuinely NESTED tuple
(`("fork", "mode")`). The `display.*` flags are the exception — they are
literal dotted keys at the top level because that is what `tui/settings.py`
reads — and getting this backwards writes a key nothing reads while looking
like success from every angle.
`test_every_default_matches_its_consumer` is the enforcement, and it fails **by
name** for any setting with no consumer entry. There is an allow-list
(`_NO_SINGLE_VALUE_CONSUMER`) for keys that genuinely have no single-value
consumer, guarded by its own staleness test — it is for free-text keys whose
"default" is "unset means inherit", not an escape hatch for a key you did not
feel like wiring up. Using it to silence the failure defeats the guard: the
whole point is that a registry default disagreeing with the code's default is a
painted lie that nothing else reports.
## Credentials and the encrypted secret store
Full agent-facing guidance is `guide://credentials` (packaged at
`local_operator/guides/credentials/GUIDE.md`); the design is
`docs/design/secret-store.md`. This section is what someone CHANGING this code
needs to know.
**Four things are called credentials and they must stay distinct.** Provider API
keys (now rows in the encrypted store under the reserved provider namespace, read
by the model layer at boot); session credentials (`VariableStore._credentials`,
memory-only, injected into `bash`, never readable by the agent); ordinary
variables (denylist-filtered, not secret); and the encrypted long-term store
(`local_operator/secrets/`, on disk, reachable by the agent through `lop secret
get` and the eval library). A change that blurs two of them is a defect even when
every test passes.
**The plaintext `credentials.env` is RETIRED and its module is DELETED.**
`local_operator/credentials.py` is gone (PR2b): nothing reads the file as a
credential source, nothing creates it, and no module may import the deleted
module — `tests/unit/secrets/test_provider_namespace.py` and
`tests/unit/secrets/test_no_reader_resolves_the_plaintext_file.py` read the
package AST and fail on a writer or an import. Every writer (`lop credential
update`, `lop search setup`, the credentials route's PATCH,
`providers.key_prompt.prompt_for_provider_key`) files an encrypted store row
instead. **One reader survives, deliberately**: `secrets/legacy_env.py`'s
`read_credentials`, which exists for `lop secret migrate-env` alone and carries
two policies paid for in review rounds — it must not CREATE the file it is
emptying (`ENOENT` is `{}`, every other errno raises) and must not print a value
(values come back as `SecretStr`). It goes away in PR2c with the file.
**Provider keys live in a RESERVED NAMESPACE: `LOP_PROVIDER_<ENV_KEY>`.**
Provider-owned rows (a built-in API key the harness manages) are named under the
`LOP_PROVIDER_` prefix and carry `role="provider"`; an AGENT-class write or read
(default everywhere else) REFUSES that prefix, and a provider-class write refuses
a name outside it. The rule is enforced in the store on BOTH the write and the
READ path — the read check is the half that actually contains a value, since a
hostile caller wants a key that already exists — so an agent secret can never
shadow a provider key. Derive the name with `provider_secret_name(env_key)` and
never spell the prefix by hand. Resolution order for a provider key is store row,
then `os.environ` (the legacy plaintext `credentials.env` file is no longer a
rung — PR2a removed its reader legs): `provider_env_key(provider_id)` /
`provider_secret_value(env_key)` in `providers/registry.py` are the ONE readers,
imported lazily so the CLI startup path never pulls the crypto stack.
**The agent uses a secret it cannot read — preserve that inversion.** No surface
returns a value to the model. The `secret` tool's `retrieve` verb returns a
receipt naming the secret and stating how to reach it; the eval library hands
real bytes to the WORKER process, not to the transcript. Any new surface that
returns a value into a tool result is writing it into a transcript that is
persisted and replayed to the provider on every later turn.
**Redaction has three sinks and they are not interchangeable.** `redact_tool_
result` (the loop's choke point) covers finished tool results;
`_redact_tool_text` covers UIs that read output before a result exists;
`_PipeRedactor` covers live stream bytes and holds back a possible secret suffix
so a value split across two reads cannot leak. A value must be registered with
`VariableStore` before the output carrying it can be read — which is why the
broker writes its session notification and waits for the ack BEFORE answering
the retrieving child.
**Build a stream filter from `_stream_redaction_values`, never from
`credential_env()` alone.** That was a real hole: `credential_env()` holds only
the values this process INJECTED, so a value a child fetched with
`$(lop secret get X)` was scrubbed from the finished result and painted live in
the streaming card and in `jobs(op='peek')`. The helper unions the injected map
with the store's registered values, and **its failure path fails closed** — a
redaction sink that cannot be read withholds the live output rather than
publishing bytes it cannot vouch for, while still draining the pipe so the child
does not block forever on a full buffer.
**The eval worker scrubs through `sys.modules`, deliberately.**
`eval_worker._scrub_secrets` checks whether `local_operator.secrets.runtime` is
already imported instead of importing it. A kernel that never touched a secret
must not pay for SQLite and the AES binding, and the check is exactly correct:
if nothing imported the module, nothing obtained a value through it.
**Do not describe this store as a vault, in code comments, docs, or to the
user.** §9 of the design is explicit and the guide repeats it: it defeats the
"scan the disk for credential files" malware that a bad link drops, and anything
running as the operator that is willing to run `lop` can still read every
secret. A comment or a docstring that promises more than that is wrong.
## The QwenCloud console ticket (`lop qwencloud-ticket`)
Full agent-facing guidance — advising a USER who wants the personal Token Plan
window in `/usage`, including capturing the cookie — is `guide://qwencloud`
(packaged at `local_operator/guides/qwencloud/GUIDE.md`). This section is what
someone CHANGING this code needs to know, and it is the half that does not
ship: `AGENTS.md` is absent from `pyproject.toml`'s `package-data`, which is
why the guide carries the user-facing workflow rather than this file.
**What it is.** `login_qwencloud_ticket`, the QwenCloud console's browser
session cookie. Not a scoped API key — it is a FULL-ACCOUNT console session,
the broadest credential local-operator holds. Its value lives in the encrypted
`lop secret` store and `auth.db` holds only its metadata, which raises the cost
of stealing it without making it safe — see "Residual risk, stated plainly"
below.
**Why it exists at all.** The personal Token Plan window is invisible to the
BSS gateway the official CLI calls: for a live account that gateway answers
`IsGray: true` with an empty seat summary and zero instances on every
commodity. The console gateway the web UI itself calls does report the window,
and it authenticates on exactly one thing — this cookie. No login flow can mint
it, because a browser session cookie cannot be refreshed headlessly.
**Storing it.** Stdin only, never argv — a command line is readable by any
process running as you and lands in shell history:
```sh
printf %s '<TICKET>' | lop qwencloud-ticket set
lop qwencloud-ticket status # presence, length and age; never the value
lop qwencloud-ticket rm
lop qwencloud-ticket migrate # move a pre-existing plaintext ticket
```
There is deliberately no `get` verb. The one consumer is inside the process.
`status` is retrieval-free by construction: it reads the metadata row and calls
`describe`, never `get`, so it neither appends a `get` audit event nor stamps
`last_used_at`.
**The row is namespaced, not registered.** It is stored under provider id
`qwencloud-console`, which is deliberately absent from `PROVIDER_REGISTRY` —
the same trick `mcp-oauth` uses. A row under `alibaba-token-plan` would satisfy
`ProviderController.has_any_credential` (it matches the provider column with no
type or field filter) and local-operator would conclude it can run CHAT traffic
on a read-only console cookie. The metadata row carries `secret_name` and never
`data["key"]`, because the API-key cascade reads `key` and would hand a
full-account browser cookie to DashScope as an inference bearer. `data["ticket"]`
is the PRE-MIGRATION spelling of the value: still read, so an un-migrated
install keeps working, and what `migrate` removes.
**The symptom when it expires.** `/usage` simply stops showing the 7 Day
Credits window for alibaba-token-plan — no error, no "sign-in expired" note,
because this is not an OAuth row and cannot set `credential_invalid`. An
expired ticket also returns HTTP **200**, carrying
`data.errorCode == "BailianGateway.Login.NotLogined"`. The fix is one line:
capture a fresh cookie and re-run `set`.
**The ticket AUGMENTS a Token Plan credential — it does not replace one.**
This is the precondition that actually gates the feature, and it is invisible
unless you know to look. `/usage` only fetches a provider `can_report_usage`
accepts, which requires `is_usable` — and the ticket deliberately cannot
satisfy that, because a row that did would be the exact blast radius the
separate namespace exists to prevent. So with a valid ticket and NO
`alibaba-token-plan` credential row, `/usage` renders nothing at all: no
window, no error, no block.
The way you get there is ordinary: `lop logout alibaba-token-plan` removes the
credential row while leaving the ticket in place. `lop qwencloud-ticket status`
warns when no such row exists, and that warning is the only signal — do not
change `is_usable`, `has_any_credential` or `can_report_usage` to "fix" it.
**Cache lag.** The stored ticket is deliberately not part of the usage cache
fingerprint — hashing a full-account session cookie into a cache key is the
worse trade. The consequence is that a ticket swap changes NOTHING the cache
key observes (measured: byte-identical keys across two different tickets), so
`set` and `rm` call `_invalidate_cached_usage` explicitly, exactly as `lop
login` and `lop logout` do for the credentials they change. Without that call a
latched `usage unavailable` row was served for up to ~12.5 minutes
(`USAGE_UNAVAILABLE_RETRY_MS` 10 min, ±25% jitter) after the user had already
pasted a working cookie. Press `r` in the panel to force a refresh regardless.
**The value is validated at the door.** `set` rejects a ticket containing an
embedded newline, any other control character, or a non-latin-1 character, and
bounds the length at 4096 (the live cookie is 172 chars). This is not
fastidiousness: the value's only use is interpolation into a `cookie:` header,
httpx rejects such values LOCALLY, and the console fetcher swallows that as
`httpx.HTTPError` — so the panel showed its generic empty-result row ("no
usage — no quota endpoint, or no credential for one"), naming a missing
credential, with nothing linking it back to the paste. Copying the cookie out of
devtools is the documented workflow and the cookie expires roughly weekly, so a
multi-line paste is a recurring certainty rather than a corner case.
**The update hazard.** `lop /update` or `uv tool upgrade` reinstalls from PyPI
and silently reverts a locally built console fetcher while leaving the row in
place: nothing reads it, and the credential still looks healthy.
`lop qwencloud-ticket status` reports that state explicitly.
**Revocation must be provable, not assumed — and `rm` is not a full revoke.**
`rm` re-reads the store to confirm the row is gone from the API's view, and
exits **non-zero** saying the ticket may still be stored if it cannot prove
otherwise — a locked or corrupt store is an ordinary outcome with
`busy_timeout` at 5s on a busy machine. "Cannot read the store" and "nothing is
stored" are deliberately different answers (`TicketStoreUnreadable`):
collapsing them made `rm` report success with exit 0 while the plaintext
full-account cookie was still on disk, telling the user it had worked. `status`
reports UNKNOWN for the same reason, and both read with
`include_disabled=True` so a soft-deleted row cannot hide a cookie that is
still present.
Be precise about what that confirmation is worth, because it is easy to
overclaim. Two things `rm` does NOT guarantee, both verified:
- **The plaintext can outlive the row, on a pre-migration store.** A ticket
written by a build before the secret-store move is plaintext JSON in
`auth.db`, and after a successful `rm` it survives in freelist pages until a
`VACUUM`. Ordinary SQLite behaviour from `delete_credential`, not specific to
this credential — but it means `rm` is not a secure erase. A ticket written
by this build never put its value there, so only `migrate` and pre-migration
installs are in scope.
**Scanning `auth.db` alone is not the check, and this is a correction to the
guidance this file used to give.** `AuthStore._connect` sets `PRAGMA
journal_mode=WAL` (`auth_store.py`), so three things are wrong with
`strings auth.db`: (1) a freshly written value lives in `auth.db-wal`, not
the main file, so the main file **scans clean while the plaintext is
readable** — a verification built on it passes against a completely broken
migration; (2) `VACUUM` alone does not fix that, because the rebuild is
itself written through the WAL — `PRAGMA wal_checkpoint(TRUNCATE)` after it
is what removes the bytes (the pair measured at 0.63 ms); and (3) the
checkpoint reports a blocked run **by return value, not by raising** — its
first column is a busy flag, so `except sqlite3.Error` alone sees success and
lies. Scan `auth.db`, `auth.db-wal` and `auth.db-shm` together, and prove the
scan discriminates with a before/after pair (True before, False after)
instead of trusting a lone False.
- **The session stays valid server-side.** Deleting the local row ends local
use and nothing more. The cookie remains a live browser session until it is
signed out in the QwenCloud console.
So `rm` is not a substitute for revoking the session in the console, and it
says so on the SUCCESS path rather than only when it fails — success is the
moment a user worried about exposure stops looking.
**Residual risk, stated plainly.** The ticket's VALUE lives in the encrypted
`lop secret` store (AES-256-GCM, blind-indexed names) as
`QWENCLOUD_CONSOLE_TICKET`; `~/.local-operator/auth.db` keeps only metadata —
`captured_at`, `length`, `secret_name`, `project_id`. That raises the cost of
stealing it and removes the readable-on-disk failure, but it is **not a vault**:
in the default `keyfile` mode the master key is a file beside the store
(`<config>/secrets/master.key`), and anything running as the user that is
willing to run `lop` can retrieve the value exactly as local-operator does. This
cookie is still broader than every other row in `auth.db`. Do not describe it to
a user as safe, secure or protected without that qualification.
**Two stores means consistency states, and all of them are REPORTED, never
silent.** This is the operational knowledge a future agent needs:
- A **metadata orphan** — row in `auth.db`, no value in the secret store —
makes `status` print a WARNING that the ENCRYPTED VALUE is missing and name
`set` as the repair, and `/usage` render `no ticket — lop qwencloud-ticket
set`. `read_ticket_record` defaults `secret_present` to True so a
pre-migration row, whose value is still in `auth.db` itself, does not trip it.
- A **secret orphan** — value in the secret store, no row — is invisible to
this feature and shows up in `lop secret list`. `rm` clears the value even
when the metadata row is already gone, which is what keeps it from stranding
one.
- A **locked hardened store** is its own outcome with its own remedy, distinct
from both "no ticket" and "unreadable store": `TicketStoreLocked` (a
`TicketStoreUnreadable` subclass), message naming `lop secret unlock`, with
`status` exiting non-zero on UNKNOWN and `/usage` showing `locked — lop
secret unlock`. `retrieve_secret`'s own wording on that path is raw broker
wire text ("no lop session is registered with the broker"), so our code
catches `BrokerDenied`/`BrokerLocked` before the generic `SecretStoreError`
and supplies the actionable message. Never rely on the store's message
reaching the user.
`set` refuses to write when the METADATA store's directory is wider than 0700 —
scoped to metadata now that the value moved, because capture time and length
still disclose that an account has a live console session. The value store
enforces the same rule of its own accord: `SecretStore._open` calls `check_mode`
on every open. Both halves are covered by their own owners rather than by one
reaching across. `status` still flags a ticket older than about a week. Note
that `AuthStore._connect` re-chmods the db file to 0600 on every open, so the
file-mode branch of that check is a backstop for a store opened some other way,
not something the CLI path can normally hit.
**`migrate` moves a pre-existing plaintext ticket**, value-only: write the
secret first, confirm it with `describe`, only then rewrite the row without the
`ticket` key, then `VACUUM` + checkpoint. That order is deliberate — a crash
between the write and the rewrite leaves the value in BOTH stores, which is
recoverable and which re-running repairs; the reverse order could lose the
credential. The row is rewritten rather than deleted so `captured_at` (the
staleness clock) and `project_id` (what makes `_identity_key_for` upsert in
place instead of inserting a duplicate) survive. **A failed vacuum is not a
migration failure but must be reported**: the value is encrypted either way, and
the line claiming the plaintext is gone is conditional on the checkpoint having
succeeded.
## Usage analytics (`local_operator/analytics/`)
Every provider call across every session contributes to one shared, on-disk
ledger (`<config_dir>/analytics.db`, SQLite/WAL/0600). `/analytics /usage`
opens an Esc-dismissable screen summarising token consumption: authoritative
provider counts (input/output/cache, and the thinking-vs-generation split of
output), an *estimated* dollar **cost** overlay, and an *estimated* breakdown of
input across the system prompt, custom instructions (agent/team profiles), tool
inventory, tool schemas, environment, knowledge, conversation, and tool
results. The frame is responsive — it grows to `max-width: 140` on a large
terminal (see the `.analytics-panel` CSS and `AnalyticsScreen._card_width`,
which must stay in step), and the per-provider/per-session tables shed the cache
column below `_WIDE_TABLE_MIN` to keep the cost column.
Things that will bite you if you forget them:
- **Recording is off the critical path and best-effort.** The wrapper is
`SessionStreamFn._record_stream` in `model/configure.py`: it forwards the
provider stream unchanged and records ONLY after the stream is fully
consumed, so a turn's latency is untouched. The one on-loop cost is
`analytics.model.snapshot_component_chars` (character-length reads;
benchmarks under 0.4 ms even on a 340k-token context) — it must run on the
loop because the transcript mutates the messages after the call. Tokenising,
apportioning, and the SQLite write all happen on the recorder's background
thread. Nothing in analytics may raise into a turn; every path is guarded.
- **One writer thread, one write connection.** `AnalyticsRecorder` funnels both
call samples and session-name upserts through a single queue drained by one
daemon thread. Do NOT add a second thread that writes to the store: two
threads opening their first connection to a freshly-created SQLite file race
in a way that leaves the writer unable to see its own commits (this cost a
round of flaky tests). Reads open a fresh short-lived connection
(`AnalyticsStore._read_connection`) so a report always sees the newest commit
rather than a stale WAL snapshot.
- **Parallel-safe by the engine, not by us.** Several `lop` sessions write to
the one file at once; WAL + `busy_timeout` + a bounded busy-retry make that
atomic. The retry lives on the background thread, so accuracy under
contention costs a session nothing.
- **The component split is an ESTIMATE and is labelled as one.** The provider
bills one input total; the split is that total apportioned by character
length (largest-remainder rounding, so it sums exactly). Authoritative counts
and estimates must never be presented as the same kind of number — the screen
says "estimated split of context tokens" for a reason.
- **Cost is computed at RECORD time, not aggregation time.** Dollar cost is
priced in `SessionStreamFn._cost_micro` (`configure.py`) where the exact model
is known, via the shared `cost_for_usage` — so analytics can never disagree
with the status band. It is stored per call as `cost_micro` (USD × 1e6,
INTEGER so the `SUM` is exact) plus a `cost_known` flag. A model with no
published price records `cost_known=False` (rendered `$—`, never `$0.00`); a
scope where some calls were unpriced is a LOWER BOUND (rendered `$12.30+`).
Cost is an ESTIMATE (list price × tokens; it cannot see a plan, discount, or
free tier) and is labelled as one, same discipline as the component split.
- **Cost is priced at RECORD time under the tariff in force at the CALL's own
moment, and a stored figure is final.** A row whose prices vary by time of day
names its schedule (`ModelInfo.time_of_use` → `model/tariff.py`), the price
fields hold the PEAK list rates, and `price_snapshot` evaluates the schedule at
the call's own `ts_ms` rather than at read time — so a historical row is never
repriced when the window (or a later tariff change) moves, and the same row
reads the same on every replay. `Usage.at_ms` carries that moment for the
surfaces that price a REHYDRATED call (a restored session, an attached
receipt), where "now" would otherwise be off by up to 2x in either direction.
An AGGREGATE carries a stamp only while every call it folds agrees on one; a
fold across a window boundary leaves it unset and is priced at the clock, so a
row never claims a window that only some of its calls ran in. A
provider-reported dollar (`usd_cost`) is never scaled by any schedule: it is
the provider's own final figure.
- **Adding a component OR a stored column is a schema migration.**
`COMPONENT_KEYS` maps to one `c_<key>` column each in `store._SCHEMA`. A
database from an older release is upgraded on open by `AnalyticsStore._migrate`
(idempotent `ALTER TABLE ADD COLUMN`, since `CREATE TABLE IF NOT EXISTS` never
alters an existing table) — this is how the `cost_*` columns reach a
token-only ledger. Any new stored column needs a `_MIGRATION_COLUMNS` entry
with a DEFAULT so old rows read as a sane value, never NULL.
- **An index over a migrated column goes in `_OPTIONAL_INDEXES`, never in
`_SCHEMA`.** `_connect` runs `executescript(_SCHEMA)` as ONE unit *before*
`_migrate` adds the `_MIGRATION_COLUMNS`. So a `CREATE INDEX` on a migrated
column raises `no such column` on any ledger written before that column
existed — and because `executescript` aborts on the first error, every
statement *after* it in the script is silently lost too. Putting
`idx_calls_parent` beside `idx_calls_session` cost a pre-`parent_session_id`
ledger its `usage_daily`/`usage_monthly` rollup tables and latched `_broken`
for the process (reproduced both ways: rollup tables gone, `aggregate().calls`
0). `_OPTIONAL_INDEXES` pairs each index with the column it needs, runs after
`_migrate` under the same lock, and guards each statement individually, so a
failed index costs its own index and nothing else. Weigh the one-time build:
`idx_calls_parent` over a 475k-row ledger is a ~660 ms stall on the first open
after upgrade (on the recorder's background thread) and ~4.9 MB of growth.
- **`aggregate()` reads a maintained day rollup, behind a fail-closed gate.**
`session_daily` is a per-`(local day, session, provider)` accumulate-upsert
written in the SAME transaction as the ledger row (`record_batch`) — the
`usage_daily` mechanism at a second grain, so there is still one write path
and no separate hook to double-count against. `AnalyticsStore.aggregate()`
serves from it ONLY when the gate can prove the same numbers (day-aligned
bounds, coverage down to the window's first day, the same bucketing zone, a
rollup as new as the ledger's newest row, and a ledger bottom the prune has
not cut inside), and otherwise runs the original three ledger scans
unchanged, naming the refusal in a `debug` log. Measured on the operator's
342.8 MB ledger (1 155 845 calls), p50, both arms measured in one session at
load ~215-280: the panel's 30-day window goes from 4 868 ms wall / 3 179 ms CPU
to 187 ms / 166 ms CPU — both arms produced by `scripts/bench_panel_latency.py`,
and THOSE numbers are the canonical ones. They were stored in
`bench/analytics-rollup-before.json` / `-after.json`, which left the tree with
the rest of `bench/` (see "Evidence goes on the PR, never into the
repository"), so quote them from `ba225070` — the `main` this sweep branched
from, where the whole store is still present:
`git show ba225070:bench/analytics-rollup-after.json` (`-before.json` is the arm
it is compared against). Wall is not portable between hosts,
and neither is CPU to the same degree: the same fast path cost 121 ms of CPU at
load 38 and 166 ms at load 237 on this box, so quote the pair with its load and
never one arm alone. Three properties matter more than the mechanism:
- the gate FAILS CLOSED (a wrong fast-path number is far worse than a slow
one; `store.last_aggregate_source` says which path ran and every refusal
reason has a test, including the two that guard the day arithmetic and an
unreadable schema);
- the equivalence is STRUCTURAL (`_assemble_aggregate` builds the result for
both paths, so only the table differs);
- the gate's two recoverable refusals are RECOVERED FROM, not lived with. A
zone change (`TZ=`, travel) re-labels every day the ledger can still answer
and publishes the new zone only when that whole span is done; a stale
pre-rollup writer's rows land only on days from the previous pass's top
onward (`last_sweep_day`), so the pass re-derives from there. Neither is a
latch: treating `zone-changed` as permanent was a defect (it cost the
feature silently, forever), and a fixed newest-three healing window left a
mid-window hole served as exact.
The sweep that fills an existing ledger is `backfill_analytics_session_daily`,
on the store-maintenance thread, newest-first in bounded per-day transactions;
until it reaches a day, reads touching it stay on the ledger, so an upgrade
can cost latency but never a number. **The rollup keeps the SAME window as the
ledger** (`retention_days`, 90) and does not outlive it: a day the ledger has
dropped cannot be served, because the rollup's copy of it is whole while the
ledger holds only the pruned remainder. Raising that reach is the deferred
change that would serve all-time from the rollup — it needs the clamp relaxed
and the panel's window labelled.
- **Session parentage has exactly ONE rule: `store._PARENT_EDGE_SQL`.** Both
the `/analytics` per-session rollup and the `/session` subtree walk resolve a
session's parent through that constant. They used to derive it separately —
a per-row filter on one side, a lexical `MAX` on the other — and disagreed
about any session carrying more than one distinct `parent_session_id`,
including the ledger's real self-parent rows. Two surfaces answering the same
question differently is the defect the rollup exists to remove, so a third
consumer must call the shared rule rather than write its own. Note the
rollup's load-bearing invariant while you are there: the per-session column
must keep summing to the headline total on the same screen, which is why
children are reachable only as sub-rows of their root (listing them flat *and*
rolling up inflates the table by 12.7%).
## The tool-surface footprint ladder
Every core tool ships its schema on **every** API request. The tools array
rides in the same cache prefix as the system prompt (`tools/registry.py`
builds it in a stable order precisely so that prefix stays cache-stable), so
adding a core tool is a permanent per-call token tax on every session and every
subagent, paid whether or not the tool is ever called. The realized core surface
is on the order of a few thousand schema tokens — lean, and worth keeping that
way (the exact figure moves as `createIf`-gated tools drop out of a session that
cannot use them; `/context` reports the live number). Before adding a tool, take
the **highest (least-footprint) rung** that solves the problem:
1. **Extend an existing tool.** The capability is usually a variation of
`bash`, `read`, `edit`, `grep`, or `eval`. A new parameter or mode on a tool
that already exists costs no new schema. This is the default answer.
2. **A skill + `bash`.** Config, state, or infra work expressible as shell
commands belongs in a `skill://` guide the agent runs through `bash`, not in
a new tool. Zero model-schema footprint.
3. **A `createIf`-gated tool.** If the capability genuinely needs structured
params AND only makes sense when a prerequisite is present, add it to
`TOOL_BUILDERS` (`tools/registry.py`) as a factory that returns `None` when
the prerequisite is absent — the way `build_wake_tool` returns `None` with no
scheduler attached and `build_browser_tool` returns `None` with no browser
surface. A gated tool costs zero schema in every session that cannot use it.
4. **An MCP server.** If it is tool-shaped but not core-fundamental, put it
behind the MCP client: `mcp://` discovery is lazy, so its schema stays out of
the prefix until the session enables it. Prefer this over a new core tool for
anything integration-specific.
5. **A new core tool — last resort.** Only when the capability is fundamental,
useful to nearly every session, and unreachable via the rungs above. The
ungated core tools (`read`, `grep`, `eval`, `bash`) clear that bar; a niche
or setup-specific capability does not. (`browser` reads as core but is
actually rung 3 — `build_browser_tool` returns `None` without a browser
surface — which is the ladder working as intended.)
**Gating answers reachability or opt-in, not host identity.** A `createIf`
factory may ask "is the dependency configured or reachable?" — it must not
encode "which host spawned me" in a way that strips a tool a reachable client
could otherwise use. And when a tool's cost is in doubt, measure it: add it,
run `/context`, and confirm the schema delta is justified by the capability.
Adding a second gating convention beside `createIf` is itself a footprint
regression — extend the table, do not invent a parallel mechanism.