Skip to content

refactor(api): extract the command layer into worktrees-api — Server Edition Phase 0 - #508

Open
brethash wants to merge 32 commits into
mainfrom
feat/server-edition
Open

brethash wants to merge 32 commits into
mainfrom
feat/server-edition

Conversation

@brethash

Copy link
Copy Markdown
Collaborator

Why

Phase 0 of the Server Edition. app/src-tauri/src/lib.rs held 143
#[tauri::command] handlers and was the only thing in the repo that could
call them. worktrees serve needs the same 143 from something that is not a
window, so the bodies move to a new crates/worktrees-api and the app keeps
thin wrappers.

This PR opens no port and adds no dependency. Cargo.lock gains exactly one
package, our own. It should merge on its own merits: lib.rs goes from 11,288
lines to 2,090, and 158 handler-level tests now compile under a library
crate's cargo test rather than only when someone types cargo test -p app.

What moved

  • Ctx replaces AppHandle + State<_>. Measured before anything moved:
    60 AppHandle mentions resolving to the config dir, emit, and three
    window calls. Ctx { config_dir, host, watch, terminals, shells, viewer, docnames }; Host carries emit alone; the window calls stay in the shim.
  • ByteSink replaces Channel<InvokeResponseBody> for the two byte
    streams (tmux attach, dock shell). The typed progress channels
    (CloneProgress, SyncProgress) became &mut dyn FnMut(T), with
    dead-client suppression owned by the transport shim. Reader-thread behaviour
    on a dead client is byte-identical to main (is_err() → break).
  • The log is a process-global installable sink (worktrees_api::log),
    not a Host method — 150 applog call sites across 59 of 143 bodies made
    threading a Ctx into every helper that might warn pure churn. Precedent:
    worktrees_core::logsink::install.
  • Three process statics became fields on Ctx. MODEL_CACHE,
    CODEX_WATCH, PI_WATCH → watch::Watch. The nav-dot poll thread stays in
    the shim and drives watch::*_tick through the managed Ctx.
  • docserver.rs, viewer.rs, editors.rs moved whole: 7, 2 and 0 lines
    changed respectively (an applog → log::log, two fixture paths for the
    new CARGO_MANIFEST_DIR, one pub(crate) → pub).

What did not change — the 1:1 evidence

  • The generate_handler! list is identical to main: 143 names.
  • Every command's JS-visible argument list is identical to main.
  • app/src/ is untouched — not one file. The mock harness still talks to
    __TAURI_INTERNALS__, which has not moved.
  • All 143 wrappers are still async fn (the main-thread rule is about the
    handler, not the work).
  • Tests: 166 → 173 (158 api unit + 7 headless + 8 app lib). Zero warnings.

Guards added

  • app/scripts/api-boundary-check.mjs — six assertions, each shown to
    fail first: worktrees-api resolves in Cargo.lock; it reaches no tauri*
    crate (BFS over Cargo.lock, failures name the route); every command
    delegates except install_app_version / set_zoom; run() installs the
    log sink before the panic hook and the first applog; ctx() clones the
    managed Ctx rather than constructing one (a fresh Ctx per call compiles,
    passes every unit test, and silently gives the poll different registries
    from the handlers — the nav dots never light); exactly one Ctx is managed.
  • crates/worktrees-api/tests/headless.rs — seven integration tests,
    deliberately integration so they may touch only public API, which is the
    position worktrees serve will be in. The registry-sharing test asserts on
    the registry via a new Watch::watching(), not on a tick's output, because
    a tick for a lane with no session file answers "nothing" whether the
    registries are shared or not (the fixture-omits-the-thing trap).
  • Five drift checks repointed to where their constants now live:
    dnd-check, drop-check, mdpaths-check, termresize-check,
    usage-check.

Also in this PR

  • A sandboxed app no longer writes into your real app's log.
    sandbox.sh --app runs under .sbx but app.log was keyed to a
    compile-time constant. It follows the running identifier now; the AGENTS.md
    note moves from "open gap" to closed, citing tauri-2.11.5 path/desktop.rs:238.
  • release.yml's docs-server gate pointed at the wrong crate after the
    move: under -p app the --exact filter matched nothing, printed
    0 passed, and the count check would have blocked the release at the
    app-bundle job — in a workflow PR CI never runs. Fixed to
    -p worktrees-api; proved both ways (0 passed → 4 passed).
  • CI never ran cargo test -p worktrees-api. Added on both OSes, no
    --lib so tests/headless.rs runs. The Linux half of proc_cwd lives in
    that crate now, so this is the only place it is executed.
  • Decision recorded in the crate doc: one engine process per host. The
    desktop app becomes a client of a local worktrees serve, the same way the
    plan already makes a second desktop one. Two processes each building a
    Ctx would mean two nav-dot polls, two sets of dock shells the other
    cannot see, two docs servers, and two writers to the shared files.
    Ctx would mean two nav-dot polls, two sets of dock shells the other
    cannot see, two docs servers, and two writers to the shared files.

Known, and deliberately left

  • Six manual cases still need a window, a pty or a browser: terminal
    replay after a place switch (the Channel → ByteSink swap — highest
    remaining risk; 2RR0;276;0c11;rgb:… on the prompt is the failure), dock
    scrollback across a restart, live clone/sync progress and cancel,
    worktrees open <file> raising the window, dock shells dying with a closed
    place, and UI responsiveness with the PRs panel open.
  • Two intermittent single-test failures, each on the first cargo run
    right after a scripted rewrite, never reproduced across 36 subsequent runs.
    Recorded, not claimed fixed. Likely same-second mtime fingerprinting.
  • R1 (per-object authorization) is not addressed and nothing here should
    be read as addressing it.
    guard_under_projects asks "under SOME
    registered project," which is the right question for one trusted desktop
    user and the wrong one for a device whose capability names a subset.
    Phase 2.
  • viewer::ISSUED is still a static though Viewer is on Ctx;
    Phase 1 moves it, the same seam Watch got.
  • Client-local commands (copy_text, open_editor, open_terminal,
    detect_editors, open_docs_viewer) moved into the api
    and execute on
    the host. Correct for 1:1; the Exposure table has to land in Phase 1 rather
    than Phase 2 because of it.

Reviewing

Commit by commit is the honest order — each moves one cluster and keeps every
gate green. b4462b9 is the crate skeleton, 0c8eada the log seam,
a42f90b the one non-mechanical cluster (the registries), 06a8959 the
boundary gate, 26e0451 the headless harness.

Phase 0 of the server edition (plan §III.0): the 143 `#[tauri::command]`
bodies in app/src-tauri/src/lib.rs are the only thing in the repo that can
drive the engine, and a server needs the same 143 from something that is not
a WebView. This is the crate they move into — foundation only; no handler has
moved yet and the app is untouched.

`Ctx` replaces `AppHandle` + `State<_>`, `Host` replaces `app.emit`/`applog`,
`ByteSink` replaces `Channel<ArrayBuffer>`.

Measured on lib.rs before moving anything, because the plan's estimate
deserved checking: 143 handlers, 11,288 lines, and 60 `AppHandle` mentions
that resolve to just six distinct uses — `app.path()` x11, `app.emit()` x9,
`app.clone()` x3, and three window calls. The handle is a config dir plus an
event sink plus a client-local remainder, which is exactly what `Ctx` is; the
window calls stay in the shim, since a phone has no window of ours to address.

Two things reading the code changed, recorded in the module header:

  - `Ctx { config_dir, events }` was not enough. `import_app_projects` needs
    `applog`, and so will every handler reporting a background failure, so the
    seam carries logging. One trait, not two — a host implements all of it or
    none of it.
  - `snapshot()` is not pure core, which its import list hides: it reaches
    `watch_lane`, which writes the process-global codex/pi watch registries.
    Handlers touching those cannot move until that state has a home. It is the
    one cluster in this extraction that is not mechanical, and it is cheaper
    to know now than at handler 90.

The crate may not depend on `tauri` — that is the coupling being removed —
nor on a transport, since the caller owns the socket. Verified rather than
asserted: `cargo tree -p worktrees-api` has zero tauri nodes.

The unit test builds a `Ctx` against a recording host and asserts an emit and
a log land, which is the point: a moved handler becomes testable without a
window, in a crate whose `mod tests` actually compiles under `cargo check`.
Three things, in the order they were found.

The log is not per-call state. A first cut put `log` on `Host` beside
`emit`, because `import_app_projects` needs `applog`. Measuring killed
it: 150 `applog` call sites, 59 of the 143 handler bodies, plus the
helpers under them. Threading a `Ctx` into every function that might
warn is churn in the signature of nearly everything to carry something
none of them vary by — a log belongs to the PROCESS, and `worktrees
serve` has one just as the app does. So `worktrees_api::log` is an
installable sink, which is what the free `applog` already was, and
`Host` carries events alone. `worktrees_core::logsink::install` is the
same shape one layer down, which is the precedent.

That makes the rest of the extraction incremental: `applog` stays a
free function at its 150 call sites and is now one line through the
sink, so a handler can move without its logging moving with it.

`log_dir()` derived its path from `APP_IDENT`, a const, so a sandbox
launched under `…worktrees.sbx` wrote into the REAL app's log — the gap
AGENTS.md records under `sandbox.sh`. The sink reads a runtime `IDENT`
instead, set in `.setup()` from the live config. Not yet claimed as
fixed: lines above that point still use the default, and which
identifier `--config` actually reaches has to be measured in the
sandbox before the note in AGENTS.md comes out.

Moved: `meta` (log_info, log_event, get_changelog, log_tail) and
`profiles` (12 handlers over profiles + the skill store), plus
`util::fmt_utc`, which three call sites still ahead of the extraction
share. `include_str!("../../../CHANGELOG.md")` resolves identically
from `crates/worktrees-api/src/` — same depth as `app/src-tauri/src/`.

Bodies are sync; the Tauri wrappers stay `async`. The AGENTS.md rule is
about the HANDLER — a sync one runs on the main thread — and a sync
body called from an async wrapper is still off it, while leaving the
server free to pick its own blocking strategy.

cargo test -p worktrees-api 3, cargo test -p app --lib 163,
cargo tree -p worktrees-api: zero tauri nodes.
…statics

The cluster the plan called the one non-mechanical part of the
extraction: `snapshot()` reaches `watch_lane`, which wrote three
process-global `static`s, so every handler above it was pinned to this
file until they had somewhere else to live.

They live on `Watch` now, which `Ctx` owns. Per-process is still the
right lifetime — one machine, one set of tmux sessions, one model cache
— but a `static` ALSO makes every test that touches one
order-dependent, which AGENTS.md records biting `viewer::ISSUED`: the
suite passed on an accident of alphabetical order. The relaunch test
said so out loud in its own comment ("the map is process-global and
shared with the drift test's `pi_tick`") and now owns its registry.
Both drift tests — `harness_feed` and `watch_lane` must know every
provider — moved with it and still fail for a harness nobody wired in.

`drain_inbox` is the one split. It emits `app:open-doc` through the
host, and the caller raises the window, because a phone has no window
of ours to raise (§III.4.B classes window calls client-local). It now
returns how many it opened and the shim focuses once for the batch
rather than once per request: `show`/`set_focus` are idempotent and the
batch drains in a tick, so that is the same behaviour, not a trade.

`guard_under_projects` moved too, ahead of the file-browser section it
belongs to, because `drain_inbox` is one of its callers. It is the
containment boundary and it was reachable only through a Tauri command
— exactly what R2 is about: a check on the far side of the IPC is a
check a remote client skips.

17 tests moved out of the app and into the crate that now owns the
code. 163 + 3 before, 146 + 20 after — same 166, none dropped.

core 795, cli 68, app lib 146, api 20, zero tauri nodes in api.
`snapshot`, `list_workspace` and the eleven project handlers around
them. This is the slice the plan called the seam-prover, and it is only
reachable now because the watch registries moved first.

Two shape changes, both forced by what the code turned out to be.

`clone_project`'s `Channel<CloneProgress>` is a TYPED progress stream,
not a byte stream, so `ByteSink` does not fit it and inventing a second
transport trait would be premature. The command layer takes a plain
`&mut dyn FnMut(CloneProgress)`; Tauri's channel and the server's
framed JSON are each one of those. The wrapper also keeps the one thing
a transport knows and the engine does not — that the client has gone
away — so the dead-channel suppression moved out of the handler and
into the shim, logged once and then quiet, as before.

That handler also used to put only the clone on `spawn_blocking` and
finish on the async executor. The body is sync here, so the wrapper
wraps the whole call, which moves `add_project`'s git fan-out off the
executor too. That is the direction AGENTS.md's async-handler rule
points, but it is a real change and not a no-op, so it is said out
loud.

R2 has started, in `.planning/r2-validation-inventory.md`:
`guard_under_projects`, `under_roots`, `project_roots`,
`valid_project_name` and `expand_home` are engine-side now. The first
is the containment boundary and was reachable only through a
`#[tauri::command]`; `valid_project_name` already carried the sentence
that makes the case — the frontend checks the same shapes and "a text
box is never evidence" — which is the identical argument one layer out.
Four checks already engine-side are recorded as verified, no action.

Six more tests moved with the code. 140 + 26 = the same 166.

Gates, all from the repo root as CI runs them: bats 518/0, core 795,
cli 68, app lib 140, api 26, lint clean, 40 frontend checks, tsc clean,
zero tauri nodes in api. (This worktree needed both fresh-worktree
bootstraps first — the bats submodule and `corepack pnpm install` —
which is what the frontend check's first failure was.)
27 more handlers, which takes the extraction past a third: 54 of 143,
plus the poll cluster that was blocking everything above it.

`copy_text` lost a `spawn_blocking`. It was the only body here that
had one, and it wrapped a `pbcopy` writing a single buffer; the rule it
was serving is the Tauri handler rule, and the wrapper is still async,
so the work is still off the main thread. Same reasoning as
`clone_project`, one size down.

`close_place` deliberately did NOT move. It sweeps the dock's owned
shells, and that registry is still `State<'_, Shells>`; splitting the
handler now would mean writing the split and then rewriting it when
`Shells` lands in `Ctx`. It is marked where it sits and follows the
PTY section.

Two tests moved with their subjects. 138 + 28 = the same 166.

app lib 138, api 28, zero tauri nodes in api, and the only warning
left in the app crate is the pre-existing `viewer.rs:422 claim is
never used`.
Six more handlers and the thirteen tests that cover them, which is 60
of 143.

`install_app_version` stays in the shim. It drives
`tauri_plugin_updater` against one release's own manifest — the app
replacing its own bundle — which §III.4.B classes client-local and
which means nothing to a server whose clients are browsers. The tag
validation and the release list it leans on DID move, so the check
that decides what is installable is engine-side even though the
install is not.

Two more `spawn_blocking`s went, same reasoning as `copy_text`:
`claude_usage` and `codex_usage` are a file read each.

One thing I cannot explain and am not going to paper over: during this
move a single `cargo test -p app --lib` run reported 126 passed / 1
failed, and the output did not name the test. It has not recurred in
26 consecutive runs since, at the same commit, including 20 in a row
run specifically to catch it. The suite is green and I could not
reproduce it; if it comes back the first thing to check is a test
keyed on a shared temp path, since several here build one from
`std::process::id()`.

api 39, app lib 127 — the same 166.
18 more handlers — the shared CLI config the app only reads, the
Claude MCP install, the per-project `.worktrees.toml` surface, and the
headless one-shot status report. 78 of 143.

The test-mover used to split items on a naive brace count and
swallowed 669 lines in one go, because braces inside strings and
comments count too. It scans properly now — strings, raw strings,
chars, line and block comments — and the bad attempt was reset rather
than patched up.

api 42, app lib 124 — the same 166. lib.rs is down from 11,288 to
7,380.
27 more handlers — 105 of 143 — and `editors.rs` moves wholesale,
because it had no Tauri in it at all: a stat per known bundle name and
a PATH walk.

`sync_apply` gets the `clone_project` treatment — a typed callback
instead of Tauri's `Channel`, with the dead-client suppression in the
shim where the transport knowledge is.

`diagnostics` keeps its resource lookup in the shim and takes the
resolved path. Where a bundled file lives is a host fact (Tauri's
`resource_dir`); the report built from it is not.

Nine more `spawn_blocking`s went the way of `copy_text`'s.

On the two intermittent app-test failures: both happened on the FIRST
cargo run immediately after a scripted rewrite of lib.rs, and neither
recurred — 10 more clean runs here, 26 after the first. The likely
cause is cargo's same-second mtime fingerprinting, which AGENTS.md
already records in its `mv`/`cp -p` form. Recorded rather than
asserted; I did not reproduce it deliberately.

api 50, app lib 116 — the same 166. lib.rs 11,288 → 6,335.
`viewer.rs` and `docserver.rs` had FIVE and ONE mentions of tauri
between them, every one of those in a comment. They were already
window-free; what kept them in the app crate was only that nothing had
moved them. The three handlers that drive them need `viewer::Viewer`
in `Ctx`, so they go now.

That puts hyper and tokio under `worktrees-api`, which its own
dependency comment appeared to forbid, so the comment now says what it
actually meant. The COMMAND transport still belongs to the caller —
nothing here opens or speaks the socket a client connects on, and
`worktrees serve` brings its own. The docs server is a different
thing: a product feature that serves a place's documents to a browser
on loopback, and that has embedded hyper since it replaced `mo`
(GHSA-6pff-wf7m-6f5h). A phone wants it as much as the desktop does.
The one invariant that is checkable stays checkable and stays zero:
`cargo tree -p worktrees-api | grep -c tauri`.

hyper, hyper-util, http-body-util, tokio, sha2 and getrandom are gone
from the app crate — all six were the docs server's, and no file under
`app/src-tauri/src` names one now. Checked rather than assumed.

Two drift tests read `app/viewer/*.tsx` through `CARGO_MANIFEST_DIR`,
which is a different directory now; they were red for exactly that
reason and are repointed. They are the mirror checks for the page's
router and mount point, so losing them silently would have been the
bad outcome — the relative path is what made them loud instead.

api 109, app lib 57 — the same 166.
The remainder in one pass, because they were one knot: the file
browser needed `DocNameIndex`, the dock needed `Shells`, the tmux
attach needed `Terminals`, and all three registries had to reach `Ctx`
together. `close_place` moved with them — it had been waiting on
exactly that since the ops batch, which is why it was parked rather
than split.

140 of 143 handlers now delegate to `worktrees-api`. The three that do
not are not leftovers:

  install_app_version  tauri_plugin_updater — the app replacing its
                       own bundle
  set_zoom             WKWebView setPageZoom
  tmux_check           a wrapper; the body moved to `hostenv`

`fixup_gui_path` and `fixup_gui_locale` went with it. Despite the
names they are not window work — they repair the PROCESS's PATH and
locale, and a launchd-started `worktrees serve` needs both for the
same reason the app does: no homebrew on the inherited PATH means no
tmux, and every place this program manages lives in one.

The registries are no longer `.manage()`d one by one. They are fields
on the single `Ctx` built at startup, so a wrapper resolves them from
one object. Only `winstate::Tracker` still stands alone, because it IS
the window.

Five frontend drift checks were reading rules out of lib.rs and had to
be repointed: dnd, drop, mdpaths, termresize, usage. Four of the five
failed LOUDLY, which is the behaviour they were written for. The fifth
(dnd) threw a TypeError on a regex that matched nothing — right
outcome, wrong message — so it now asserts the match before indexing
it, and `mdpaths` says which function it could not find instead of
silently testing an empty string.

The app crate lost portable-pty, libc, hyper, hyper-util,
http-body-util, tokio, sha2 and getrandom. All eight belonged to the
PTY host or the docs server; no file under `app/src-tauri/src` names
one now.

R2's inventory is complete in `.planning/`, including what was left in
the shim and why, and an explicit note that R1 is NOT addressed here —
`guard_under_projects` asks "under SOME registered project", which is
the wrong question for a device with a per-project capability.

Gates from the repo root, as CI runs them: bats 518/0, core 795, cli
68, api 158, app lib 8 (= the same 166), lint clean, 40 frontend
checks, mcp check clean, tsc clean, zero warnings in the app crate,
zero tauri nodes under worktrees-api.

lib.rs: 11,288 → 2,090.
`worktrees-api`'s Cargo.toml says it must not depend on tauri, and
every commit so far has checked that by hand with `cargo tree`. A
comment is a claim about a config file and a config file can be
edited, which is the argument `viewer-boundary-check.mjs` already
makes for mermaid — so this is the same shape, one crate over.

It reads Cargo.LOCK rather than Cargo.toml, because the toml says what
was asked for and the lock says what was got, and a transitive edge is
exactly the one nobody notices. The walk keeps its path, so a failure
names the route in.

The second half guards the other direction: a command in lib.rs that
has a BODY rather than a wrapper is the extraction coming undone one
handler at a time. `install_app_version` and `set_zoom` are listed as
the deliberate exceptions, which also means adding a third has to be a
decision somebody writes down.

Both halves shown to fail first, as AGENTS.md requires — a fake
`tauri` edge spliced into the lock produced 14 named routes, and a
`set_pin` wrapper re-inlined by hand produced the second. Restored by
editing back and `touch`ing, never `mv`.

41 frontend checks, `cargo build -p app` clean.
AGENTS.md recorded the sandbox log as an open gap — `--app` moves the
config dir but `app.log` goes to the plain identifier, so a sandbox
writes into your real app's log. It is closed, and the note now says
why the two ever disagreed: `app_config_dir()` is
`config_dir().join(&config().identifier)` (tauri-2.11.5
`path/desktop.rs:238`), so the config dir always followed the runtime
override, while `log_dir()` read the `APP_IDENT` const. They read the
same field now.

Not claimed further than that goes: the `startup v… PATH=…` line is
still written under the default identifier, because it is emitted
before any app handle exists to ask, and that is said in the note
rather than left for someone to find.

`CHANGELOG.md` is deliberately not in `paths-ignore` — it ships in the
binary via `include_str!`, which still resolves from the api crate at
the same depth. One `### Changed` under `[Unreleased]`, counted.
Automating what the manual 1:1 pass would otherwise have to catch by
eye. Three of the twelve cases are now gates; the rest genuinely need
a window, a pty or a browser, and saying which is the point.

`crates/worktrees-api/tests/headless.rs` is an INTEGRATION test on
purpose. It links the crate from outside and may touch only what is
public, which is the position `worktrees serve` will be in — so it
passing is a statement about the seam, not about the code behind it.
Seven cases: a `Ctx` built with no window, settings round-tripping
through its config dir, ui-events appended and read back as usage, the
CHANGELOG still resolving through `include_str!` from a crate three
levels down, logging staying silent with no sink, every provider wired
into the poll, and the registry-sharing one below.

That last one is the nav-dot failure this refactor could most easily
have introduced: the registries were three process-global `static`s
and are now fields on `Ctx`, so a clone that stopped sharing them
would have the snapshot record a lane into one map and the tick read
an empty other — no error, no log line, dots that never light.

Its first version asserted on `pi_tick`'s OUTPUT and failed, and the
failure was the test, not the code. A tick can only report state for a
lane with a real session file behind it, so a nonexistent path answers
"nothing" whether the registries are shared or not — it would have
passed for the wrong reason, which is the fixture-omits-the-thing trap
AGENTS.md records. It asserts on the registry now, through a new
`Watch::watching()`; the maps themselves went back to private, where
they belong, because `pi_watch_set` keeps startup memory across a
re-list that a direct write would lose. (My blanket pub-fields pass
during the extraction had exposed them.)

`api-boundary-check.mjs` gains the three facts about `run()` that no
Rust test can reach, because they are about the order and arity of
calls rather than any function's behaviour — and all three fail
silently in the running app:

  - the log sink is installed before the panic hook and the first
    applog (install it late and both vanish)
  - `ctx()` CLONES the managed Ctx rather than constructing one
    (constructing compiles, type-checks and passes every unit test,
    and kills the nav dots)
  - exactly one Ctx is managed

Every new assertion shown to fail first: the sharing test against a
hand-written `Clone` that hands out a fresh `Watch`, and the three
wiring checks against a moved install, a constructing `ctx()`, and a
doubled `.manage`. Restored by editing back and `touch`ing.

api 158 + 7 integration, app lib 8, 41 frontend checks, zero warnings.
…that holds them

The four docs-server boundary tests moved to `worktrees-api` with the
command layer. `release.yml` still ran them with `-p app`, where the
`--exact` filter matches nothing, prints `0 passed`, and the count check
blocks the release — fail-closed, but at the app-bundle job, after the
CLI artifacts have shipped, and in a workflow PR CI never runs. Shown
both ways before the change: `-p app` → 0 passed, 8 filtered out;
`-p worktrees-api` → 4 passed.

`ci.yml` never ran `cargo test -p worktrees-api` at all, so 158 unit
tests and the headless harness ran only when someone typed the command
— including the Linux half of `proc_cwd`, which the comment above the
app step still called the only place it was ever executed. It runs on
both OSes now, without `--lib`, so `tests/headless.rs` runs too.
AGENTS.md's gate list and `.claude/close-out.md` gain the line.
…a local serve

The plan drew the app and `worktrees serve` as alternatives and never
both on one machine, which is the normal case. Two processes each
building a `Ctx` means two nav-dot polls, two sets of dock shells the
other cannot see, two docs servers, and two writers to the shared
files. Decision recorded in the crate doc, where Phase 1 will read it:
the desktop app connects to a local server the way the plan already
has a second desktop do, and a version handshake becomes mandatory.
…e scheduler

`project::tests::a_forced_ask_queued_behind_a_fetch_shares_it` failed on the
macOS runner of #508 (`left: 2, right: 1`) and passed on ubuntu. It slept
40ms and expected to land inside a 150ms fetch on another thread; a slow
runner spawned that thread late enough that it did not, and the failure read
as the PR cache fetching twice. Locally it passed 40 of 40, and this session's
transcript shows it failing once mid-extraction on a first run after a
rewrite — one of the two intermittent failures recorded then.

The rule is about three stamps (asked, started, done), so `answer_with` takes
the clock and the test scripts them; a `Barrier` keeps the shape — the second
caller at the door while the first concludes — without deciding the outcome.
Shown to fail first by stamping the conclusion at `started`. AGENTS.md gains
the note.

Four warnings the TEST builds surfaced, none visible to `cargo check`. In the
api crate: the project test module's glob import was unused while its one
test imported explicitly (it relies on the glob now), and `viewer::claim` was
a private function with no callers — on `main` too; the live path is
`place_key` → `place_key_in` → `claim_in`. Removed, and `claim_in`'s doc now
names the lock that actually exists. In the shim: `mod tests` kept a glob
import and a `v` helper that only the 155 moved tests used, plus the blank
lines the mover left where they were. All gone; the 8 tests are unchanged.
…link is a later opt-in

Records the 2026-10-17 choice of option B (the app's sidecar `worktrees`
starts `serve` and nothing else; `install.sh` keeps the CLI on PATH) over
option A (a link from `~/.local/bin` into the bundle), with the three reasons
and the shape A should take when it comes: an opt-in Settings offer once
`serve` is stable, like the MCP setup offer. Signing is noted as not a factor.
…nary files

docs/proposals/server-edition.md did not exist, though the worktrees-api
crate doc cited it. It now holds Part III of the plan with its section
numbers intact, so existing citations resolve, preceded by the decisions
made since: one package with the UI always a client, one engine process per
host, a Tauri iOS and iPadOS client instead of a PWA, Apple signing for iOS
only, the CLI as a private sidecar, and the loopback listener's own rule
set. Dated notes mark where those override the plan's text. The R2
validation inventory, until now in a gitignored planning file, is its
appendix. The crate doc now points at the sections that exist.

AGENTS.md gains a trap. Inside Claude Code, grep is a function running a
bundled ugrep with -I, which silently skips any file holding a NUL byte.
projcfg.rs has one in a comment, so a recursive grep for USER_ONLY_KEYS
finds every citation and not the definition, and a lane concluded the const
no longer exists. It does, at projcfg.rs:51; git grep finds it.
projcfg.rs held a literal NUL in a test comment where TOML's \u0000 escape
was meant, and an archived session summary held NUL, 0x1f and 0x7f where
its regex text \x00-\x1f\x7f was meant. Claude Code's grep wrapper runs
ugrep with -I and skips any file containing a NUL, so a recursive search
for USER_ONLY_KEYS found every citation but not the definition. git
rendered the summary's diffs as binary. Both now hold the escape text, and
no tracked text file contains a NUL.

The AGENTS.md note now describes the trap in the past tense, names
git grep and /usr/bin/grep as the second witness, and adds the cause: a
backslash escape inside an ordinary Python string literal is written as
the raw byte. git is no reliable check, because it sniffs only the first
8000 bytes of a file.
… loaded last

set_codex_permissions and set_pi_trust are pushed on every page load into
process-global overrides that outrank the env var and user config. Under one
engine per host that lets any client widen Codex's permissions for every lane.
Queued for a ruling before Phase 2; found by the transport lane driving serve.
… managed

tauri's Builder::setup replaces the stored closure (`self.setup =
Box::new(setup)` in 2.11.5) rather than chaining. run() called it twice,
so the first closure never ran. Since #358 that closure only installed
core's log sink, so the usage readers' lines were silently dropped. #508
moved IDENT and the one Ctx into it too, so every ctx() panicked
"state() called before manage()". The real app.log holds 50 of them from
a dev build of this branch. CI was green because nothing runs the app.

Fixing that surfaced a second panic of the same kind: the poll still
asked handle.state::<viewer::Viewer>() for a registry that #508 folded
into the Ctx. It now uses pollctx.viewer.

api-boundary-check.mjs gains two guards, both shown failing on the old
file: exactly one .setup( in run(), and no Tauri state lookup except the
Ctx and the window tracker. After the fix, the same dev build ran ~100s
with zero panics in app.log.

Reported by the fix/host-policy lane.
…ne engine per host

All 61 statics in the api, core and shim crates, read with their writers and
readers, plus serve's own state on feat/worktrees-serve and every load-time
setter in app/src. 31 host facts, 5 host policies, 0 client preferences,
2 per-launch identities, 23 const-like or test-only.

No static holds a client preference. The five host policies (Codex
permissions, pi trust, scrollback persistence, per-tab history, fetch
interval) all reach the engine through an App.tsx effect that pushes the
page's settings into an in-memory static. The two security settings are
recorded with their fix in flight on fix/host-policy (OWASP review M2);
the other three take the same shape. Only the terminal-history
pre-hydration push regresses in Phase 1.

Also corrects §III.6's fourth item (ISSUED already has its seam and stays in
the shim) and adds a fifth: a dock shell has one sink. The two Queued bullets
now point here.
The command layer handed client strings to core's cmd_* parsers bare, and
those parsers tell a flag from a positional by its leading `-`: relink's
`slug: "--all"` relinked every place (with --force) and came back OK, and
new_place's base `--no-fetch`/`-r`/`--brief=...` set those flags.

- core: every parser the api reaches honours `--` (argv::split_options).
- core: the git calls that carry a client positional end their own options.
  Required, not tidy: cmd_switch used to refuse a `-`-led base only as an
  unknown flag, and do_switch's `git fetch origin <base>` with
  `--upload-pack=<cmd>` RUNS <cmd>.
- api: every cmd_* argv is built by argv::Argv (static flags, glued values,
  positionals after `--`); a test fails on any other call to a cmd_*.
- boundary: MCP's safe_arg moves to core and is shared; a branch is held to
  git's check-ref-format --branch rule (oracle-tested against git), also
  after core's origin/ strip. Refusals are failed CmdResults, so the
  new-worktree form keeps what was typed.
- MCP: same validators, and `--` before its positionals.
… never sets them (OWASP M2)

App.tsx pushed `set_codex_permissions` and `set_pi_trust` from two
useEffects on every hydration and every settings change, from the window's
own ui-state.json, into core's process-global overrides, which outrank env
and user config. Under one engine per host, the client that loaded last
decided what Codex may run in every lane, so opening a stale page set to
`full` widened every lane.

Now both are HOST policy:

- `worktrees-api::hostpolicy` keeps them in `host-policy.json` in the config
  dir. It is written through core's `config::edit_user_config` (lock and
  rename). Unknown keys survive a write, and a corrupt file is refused rather
  than clobbered.
- The setters persist, then apply. An unknown value is refused and nothing is
  written. They are called only from the Settings click.
- The two load effects are gone.
- `get_codex_permissions` (new) and `pi_status.trust` are what the controls
  render. Each set is followed by a fresh read, and only the newest read
  lands, so out-of-order replies cannot leave the control on the loser.
- `hostpolicy::boot` runs once in the app's setup, beside `Ctx::new`. It
  migrates once from ui-state.json, READ only, copying a value only if it is
  chosen (not the default) and only where the host lacks the key. Then it
  loads the file into core.
- The CLI and MCP-launched lanes never boot it, so they keep env and user
  config as before.
- The two `Settings` fields are dropped and stripped from the blob on load.
- The mock persists host policy across a reload and records each setter call
  (`__mock.hostPolicySets()`). `?hostpolicyfail` drives the rollback.

`app/scripts/hostpolicy-check.mjs` walks the TypeScript AST and requires
every way to either setter to end in an onClick. It fails on the pre-fix
App.tsx. It also requires `hostpolicy::boot` inside the one `.setup` closure
tauri keeps, and fails on a lib.rs that puts it in a dead one.
`set_term_history_opts` was pushed from an ungated effect, with
`.catch(() => {})`. Before hydration `settings` holds DEFAULTS (both on), and
its comment called that early push a no-op because "that is what the backend
already assumes". That is true only of a FRESH backend. A webview reload, and
every client of a long-running serve, reaches one that already holds the
user's "off", and the push turned persistence back on until hydration
landed. A 15 s flush in that window writes every dirty dock shell's ring to
term-history/, whatever a command printed. Measured in the mock with
?slowsettings=800 and a stored "off": every load and every reload pushed
{true, true} twice before the user's {false, false}.

The effect now opens with `if (!hydratedTick) return;` and lists
`hydratedTick` in its deps. A refusal goes through `fail()`. The comment
states the real reason for the gate.

hostpolicy-check.mjs gains rule 5: every push must sit directly inside an
effect gated that way. It fails on the base.

Out of scope, per Appendix B: persisting PERSIST_SCROLLBACK, PER_TAB_HISTORY
and FETCH_INTERVAL_SECS on the host, and the fetch-interval effect, whose
pre-hydration 0 is harmless.
…nd R3 closed

Decision 6's first-frame auth is superseded by the mutual HMAC exchange
(OWASP H1). Decision 7 records that only the engine reads Codex
permissions and pi trust, that CLI and MCP-created lanes keep env and
config, and that an unset host value now follows config, which can
widen app lanes. The CHANGELOG bullet now says so too. R3's invariant
moves from Queued to closed, with the out-of-scope items found by its
lane listed for Phase 2.
…traction

main added Kimi Code as a fourth harness against the OLD 11k-line lib.rs.
This branch moved every command body into worktrees-api, so lib.rs is kept
from this side and main's ~360 Kimi lines are ported into the extraction,
each beside its pi twin:

- kimi_status / KimiStatus, kimi_version, set_kimi_allowed -> api project.rs
  (kimi_version's wrapper keeps spawn_blocking: it starts a kimi process)
- kimi_mcp_status, kimi_mcp_set -> api config.rs
- KimiWatched, kimi_watch_set, kimi_tick, kimi_tick_in, and the kimi arms of
  harness_feed / watch_lane / live_model -> api watch.rs; the registry is
  Watch.kimi on the Ctx, so main's KIMI_WATCH static does not come across
- kimi_paste_refusal, KIMI_OPENER_PENDING, and the kimi arms of plan_prompt /
  plan_ask_main / ask_main_refusal -> api files.rs
- set_pi_allowed's warn-on-not-a-repo log from main -> api project.rs
- lib.rs gains five thin wrappers and their invoke_handler entries (149)

Tests from main's lib.rs mod tests, moved with the code:
- agent_sessions_match_the_two_provider_rule_they_replaced -> workspace.rs
- kimi_lanes_feed_busy_waiting_and_done_through_the_tick -> watch.rs
- a_kimi_paste_needs_an_empty_composer_and_no_prompt_open -> files.rs

Conflicts: SettingsSheet.tsx takes both imports (KimiPanel, CodexPermissions);
the mock keeps the host-policy block and adds main's Kimi state beside it.
main's `doctor --kimi` branch scanned the raw argv, so `doctor -- --kimi` (a
place NAMED --kimi) was taken as the flag and refused with "takes only
--json (got '--')". It now scans split_options' opts and rest exactly as
the --pi branch beside it does. The R3 test covers --kimi beside --pi; it
was red on the merged code before this change.
- CHANGELOG: the extraction moved 148 commands, not 143 — main's five Kimi
  commands moved with the rest (names and client arguments checked against
  main's lib.rs).
- server-edition.md Appendix B: rows for kimi::KIMI_TAIL (host fact, the
  wire-log twin of PI_TAIL) and hostpolicy::tests SERIAL (test-only), both
  landed after the f308062 audit; counts become 32 host facts and 17 + 7.
- adding-a-harness.md: the dot-poll item pointed at app lib.rs; harness_feed
  and watch_lane live in worktrees-api watch.rs now, registry on Watch.
- AGENTS.md, api-boundary-check.mjs: drop a command count that rots.
… extraction

#511 added a `kimi_usage` command to the old lib.rs. It is ported the way
the other usage readers were: the body is worktrees_api::status::kimi_usage,
and lib.rs keeps a thin wrapper that runs it on the blocking pool, like
kimi_version, because it starts `kimi web` and waits. 150 commands are
registered: main's 149 plus get_codex_permissions.
brethash added a commit that referenced this pull request Oct 20, 2026
… the app and serve

Every engine process needs the same preamble, the same background work and
the same exit sequence: the log sink before anything can want it, the panic
hook, the launchd-bare PATH and locale repairs, core's log, the startup line;
the viewer sweep, the pre-registry project merge, the 3 s poll that lights
the nav dots and flushes the dock rings; and on the way out the cwd and ring
flush, the shells, the docs server, in-flight clones, the derived trees. The
desktop app's run() had grown all of it one hard-won line at a time, and
`worktrees serve` needs every line — which is the shape of the bug AGENTS.md
records for pi: a correct core, and a host that forgot one wiring step.

So the wiring moves into `worktrees_api::host` as `boot(sink)`,
`start(ctx, hooks)` and `stop(&ctx)`, ported from the shim without
behavioural change (every comment travelled with its beat). The shim's edit
is mechanical: three calls, and the one window-only step — raising the app
when the inbox delivers a `worktrees show` — becomes the `on_inbox` hook,
which a server leaves empty. `hostenv` gains `APP_IDENT` and
`default_config_dir()`, Tauri's `app_config_dir()` computed without Tauri.

`api-boundary-check.mjs` now asserts the shim makes all three calls — boot
first, start exactly once on the managed Ctx, stop in the Exit arm — and
that nothing in run() installs a sink, a hook, a thread or an exit step of
its own. Shown to fail first: 13 failures against the pre-edit shim.

Rebased onto #508's one-`.setup` fix (bbfed64): tauri's Builder::setup
replaces rather than chains, so the shim's single `.setup` now holds IDENT,
the config dir, `hostpolicy::boot`, the managed Ctx and the window state,
and then `host::start`; `host::boot` replaces the preamble, core's log sink
included, so the `.setup` no longer installs it. The check keeps the base's
two new rules (exactly one `.setup`; Tauri state only for the Ctx and the
window tracker) and gains one: `host::start` must come after the Ctx's
`manage()` — shown failing with the call moved three lines up.
brethash added a commit that referenced this pull request Oct 20, 2026
…ack WebSocket

Server Edition, Phase 1, slice 1 (the server half). `worktrees serve` hosts
`worktrees-api` on 127.0.0.1 and speaks the frontend's own shape: a request
`{id, cmd, args}` is answered `{id, ok}` or `{id, err}`; a `ByteSink` or a
progress callback becomes ordered channel frames `{channel, seq, data}` on
the connection that asked; `Host::emit` is `{event, payload}` to every
authenticated socket. One unbounded queue per connection and one writer
task make frame order a property of the code (AGENTS.md: a replay answered
as input came from an out-of-band payload). Requests dispatch concurrently
on the blocking pool, as Tauri's async commands do, so a clone cannot hold
up a keystroke.

The loopback rule set (`serve/policy.rs`), every refusal a closed socket
with nothing written: `Host` must be loopback on our port (the docs
server's rule, reused — it is what stops DNS rebinding); `Origin` must be
on the allow-list, and a MISSING Origin is refused — deliberately the
opposite of `docserver::origin_ok`, whose own page sends none, so the
divergence is written down where someone would "align" it; the first frame
must be `{"auth": "<token>"}` within 3 s. The token is 256 bits from
getrandom, rotated per launch, written to `<config_dir>/serve.json` with
`{port, pid, version}` at mode 0600 under flock + temp-with-mode + rename;
a second serve on a config dir whose file names a live pid that still
listens is refused (one engine per host). The auth reply carries the cargo
version for the app's mismatch check.

Dispatch is ONE macro table (`serve/dispatch.rs`): 150 rows, each
`name: Exposure => body`, generating both the exhaustive `(name, exposure)`
slice and the match arms, so neither can drift from the other. `ClientLocal`
rows (copy_text, open_editor, open_terminal, detect_editors,
open_docs_viewer, set_zoom, install_app_version) are refused with
`err: "client-local"` before their body is looked at — copy_text would write
the SERVER's clipboard. `HostAdmin` rows are tagged and logged, allowed on
loopback. Args are read by key as Tauri reads them: camelCase as the
frontend sends, snake_case accepted too, missing = null, the error naming
the key. `err` carries what the Tauri `Err` serializes to — a string for
149 commands, the `CloneError` object for clone_project, whose dialog
branches on `kind`. A client that disconnects takes its tmux attaches with
it; dock shells stay, because the next attach replays them.

Stack: hyper parses the upgrade (already compiled in for the docs server)
and a refusal is the service returning Err, which hyper answers with a
closed connection and no bytes — tungstenite's own accept_hdr writes an
error response, which is why it is not used; tokio-tungstenite 0.30
(default-features off, `handshake` for derive_accept_key only) owns the
frames over the upgraded socket. futures-util is a trait import, already in
the lock. Thirteen crates new in the lock, listed in Cargo.toml.

The log is `~/.cache/worktrees/serve.log`, the CLI's cache location, with
`host::boot`'s `startup v…` line first — a long-lived server outlives the
binary that replaced it on disk. SIGINT/SIGTERM run `host::stop` and remove
serve.json if it is still ours.

Tests, each shown to fail first by mutation (12 mutations, 12 red):
the connect-witness over raw sockets, served case FIRST (a 101 with the RFC
6455 worked-example accept value), then foreign Origin, missing Origin and
forged Host each closed with zero bytes; serve.json 0600 and rotated;
the version in the auth reply; a silent client cut at the deadline; requests
answered by id with Tauri's shapes; events to every authenticated socket
and none to an unauthenticated one; and `tests/serve_bin.rs`, which drives
the real binary under a sandbox HOME/XDG: serve.json, `list_workspace` over
an empty registry, the log's first line, a refused second serve, SIGTERM
cleanup, token rotation. The second-serve step is a bounded spawn/poll/kill
because its first fail-first run HUNG under the mutation rather than failing.
Mode assertions were measured on macOS 27.0 and in Linux 6.12 containers
(Alpine 3.24, Debian 12) under three umasks; a pid outside `pid_t` reads
dead without being cast into a process-group probe.

`app/scripts/serve-dispatch-check.mjs` holds the table against the shim's
generate_handler! in both directions and pins the client-local seven; fails
with a row removed, a handler removed, or a row retagged.
`app/scripts/server-stack.sh` runs a serve on an isolated config dir, tmux
prefix and port for the frontend lane and prints the serve.json path.

(143 rows at the extraction. The rebase onto #508's merges of main added
host policy's `get_codex_permissions` (Engine) and Kimi's six commands,
each classed as its pi or codex twin — kimi_status, kimi_version and
kimi_usage Engine; set_kimi_allowed, kimi_mcp_status and kimi_mcp_set
HostAdmin — and `set_codex_permissions` / `set_pi_trust` now take the Ctx.)
@brethash
brethash added this pull request to stack #517 October 20, 2026 14:54

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant