AGENTS.md · diff

git:20260924.e0aecd7 to git:20260924.bc1e247

2409 added, 2402 removed, shown as one replacement because the pair was too large to align, first 3,000 lines shown. Audit A to A.

# 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.
+ Python-level signal timeouts all fail to fire — verified, not assumed, and
+ *Python-level* is the narrow claim: a Python signal handler runs only between
+ bytecodes, which a wedged process never produces.
+ `faulthandler.dump_traceback_later` survives it, because it runs in a C
+ thread and needs no GIL. It is armed dump-only (`exit=False`): the C callback
+ writes every thread's stack to a file and ends nothing, and the fast-fail is
+ a kernel-level `SIGALRM` one grace later — disposition `SIG_DFL`, so nothing
+ has to be scheduled for it to fire. `python -m tests.e2e.watchdog`
+ prints the retained dump back.
+ - **It is why the stage is deselected and runs `-n0`.** `faulthandler`'s timer
+ is process-global, so under xdist a `bounded` block would take a worker's
+ stacks with it for the rest of the item, and a hang nothing in-process can end
+ would strand the run as an infrastructure error rather than a freeze.
+
+ 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