From ede3f2fedda5d01137e48563ebfae21a5866a6fb Mon Sep 17 00:00:00 2001 From: John Ky Date: Mon, 7 Sep 2026 13:02:18 +1000 Subject: [PATCH] docs(docs): record #1602 context menus and #1605 reaping in CLAUDE.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #1602 listed three docs to keep in sync — `docs/worktrees-service.md`, ADR-0072 §6, and CLAUDE.md's worktrees paragraph. The first two were updated; this one was missed across all four phases. It matters more than a normal docs gap because CLAUDE.md is loaded at the start of every session, so it is where a future session looks first: without this, `menu.rs` is invisible, and so are two grep guards, which is how guards get deleted. Adds what a reader needs and cannot infer: `menu.rs` holds per-surface contents as pure functions (the tree menu excepted, so it cannot drift from the `a` menu); `draw_menu` returns the region it drew, so the hit map and the frame cannot disagree; `popup_mouse_enabled` names the one popup kind that takes the mouse, the confirm modal staying keyboard-only because its `y`/`n` gates destructive actions; contract §6 and §7; why `alt-m` is primary and `F10` is deliberately not a `ChromeKey`; and the clipboard-read guard as the other half of `Osc52::OnlyCopy`. Also corrects the §5 sentence in the same bullet, which called the host's escape hatch "shift-drag". The #1602 spike measured that as terminal-specific: Ghostty keeps shift-drag, iTerm2 keeps option-drag and forwards shift-drag, the VS Code terminal keeps neither. The mechanism stands; the modifier is not ours to encode. Finally records #1605's process-group reaping and the residual #1611 stall, with the operational consequence a future session needs: run this module's tests with `--test-threads=1`, and never by skipping them — that is how a never-run test reached CI in #1608. --- CLAUDE.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CLAUDE.md b/CLAUDE.md index 58bed2b0..33d26298 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -201,7 +201,7 @@ The daemon's **third** service tracks the repos/worktrees open across **every** - The **`push`** op (#1443, [ADR-0061](docs/adrs/adr-0061.md)) is the other half of `rebase`: a rebase diverges every already-published branch it touches, and this publishes them again. Two-phase and `push_lock`-serialized like `rebase`, but its plan phase **contacts no remote at all** — classification is against the local `refs/remotes//`, which is exactly what the lease is checked against, so the plan the user confirms and the lease `git` enforces agree by construction (a fetch here would *renew* the lease). Outcomes: `UpToDate` / `WouldFastForward` (ahead only — a plain push, **no force flag**) / `WouldForce` / `WouldCreate` (`--set-upstream`) / `Skipped`; a dirty tree is deliberately **not** a skip, since a push publishes commits rather than the working tree. Two rules are load-bearing. **(1)** Every force goes out as `--force-with-lease --force-if-includes`, never `--force`: bare `--force-with-lease` leases against the local remote-tracking ref, so any background fetch (VS Code's `git.autofetch`) silently renews it, and per `git-push(1)` `--force-if-includes` is *not* implied and is a no-op unless `--force-with-lease` is **valueless**. There is no force escape hatch anywhere — not an engine option, not a CLI flag, not a wire field — so the daemon *cannot* overwrite a tip it has not seen. **(2)** The repository's **remote default branch is never force-pushed**, whichever worktree holds it; the gate is the **branch name**, not `is_main`, which inverts [ADR-0060](docs/adrs/adr-0060.md) precisely because a rebase is local and reflog-recoverable while a force-push publishes the rewrite (a *fast-forward* onto the default branch stays allowed). Both guards live in `crate::git::worktree_push`, not the UI. The tree cue is a transient `pushing` set mirroring `rebasing` but with **no durable half** — a push writes no on-disk state, so `clear_pushing` on every exit path is load-bearing; a *completed* push is observable as `upstream_sha` moving (#1344). The VS Code action is the only batch git action offered on **repo rows**, expanded client-side by `tree.ts::expandToWorktrees` (no new op). - The **`reload`** op (#1417) signals the selected worktrees' windows to run `workbench.action.reloadWindow` — the batch form of `Developer: Reload Window`. Deliberately **not** two-phase like `close`: a reload creates, modifies and destroys nothing and VS Code's hot exit preserves dirty editors, so it follows the `reposition` precedent of firing and reporting, with nothing to undo. Addressed by **window key** like `reposition` rather than by path like `close` (a reload acts on a *window*, and one path can be open in several), which is also why it needs no `requester_key` — the invoking window reloads itself directly rather than waiting a heartbeat for its own directive, so the daemon never learns who asked. Cross-window targets are only *signalled*: the directive rides the ~10s heartbeat, and a reloaded window re-registers under the same key, so there is no completion to observe and the reply claims only what was signalled. Adds no capability (strictly weaker than `close`), so no ADR. - `src/cli/worktrees.rs` — `omni-dev worktrees list [--json]` / `tree` read-only clients plus `focus ` (raises a worktree's VS Code window via the daemon's `open` op — the tray-only focus action made CLI-reachable, #1113) and `reposition --reference […] [--dry-run|--undo]` (#1407 — paths are mapped to window keys via a `list` call, since the op addresses *windows*; `--dry-run` is the title-matching diagnostic and the AX calls happen in the **daemon's** process under the daemon's grant, not the terminal's); `--socket` override. `worktrees rebase` is the one subcommand that stays **entirely local** (its own `crate::git::worktree_rebase` engine, no socket) — since #1415 that is a *choice*, not the credential necessity ADR-0055 claimed: it keeps a batch rebase working with no daemon running and keeps `--all`/`--onto` out of the wire protocol. #1361 closed the CLI/extension parity gap, so the full subcommand set is now `list`/`tree`/`focus`/`close`/`rebase`/`push`/`merge-queue`/`reposition`/`reload`/`show-closed`/`register`/`heartbeat`/`unregister` — including the companion **feed** ops (`register`/`heartbeat`/`unregister`), exposed for scripted/headless companions and integration testing even though the real client, the VS Code extension, still speaks NDJSON to the socket directly. Note `register`'s repository-name flag is **`--repo-name`**, renamed in #1420 because `--repo` collided with the global `-C/--repo` on clap's *arg id* (a panic, not an error); a structural test now walks the command tree asserting no subcommand re-uses a global arg id. -- `src/cli/worktrees/ui/` — **`omni-dev worktrees ui`**, a full-screen `ratatui` terminal UI and the registry's **second first-party client** (#1585, [ADR-0072](docs/adrs/adr-0072.md)). A read-only daemon client that deliberately does **not** register as a window (the daemon's `open`/`reposition` assume a VS Code window): `wire.rs` (hand-mirrored, `sanitize_for_terminal`-at-the-boundary DTOs), `supervisor.rs` (reconnect-with-backoff over `subscribe`, permanent polling fallback for an old daemon that replies `ok:false` and holds the connection open), `hub.rs` (the actor merging both feeds + local state into one `WorktreesViewModel`), `actions.rs` (the parity commands and the two-phase `close` fan-out — `close` isn't batched on the wire), `terminal/` (Phase 3: one embedded terminal tab via `alacritty_terminal` in-process, one PTY-reader OS thread per tab feeding the async loop over one channel). Two load-bearing PTY invariants, regression-tested against real children: `TERM`/`COLORTERM` set per spawn, and every `Event::PtyWrite` written back to the child. A `claude` tab launches through `omni-dev claude-wrap`, so those sessions report *authoritative* state (ADR-0057). Focus decides who gets a key — a focused terminal takes everything verbatim except `Alt`-chords, no tmux-style prefix; keys are legacy-xterm encoded and the kitty keyboard protocol is deliberately not advertised (`Config::kitty_keyboard` off). **PTY contents are never logged** — enforced by the `no_pty_content_is_ever_logged` grep-guard test, not convention; the emulator stays at `Osc52::OnlyCopy` so no clipboard→child path exists. Row colours are a local unsynced store (`~/.omni-dev/worktrees-ui-row-colors.yaml`, `0600`/`0700`) until `rowColors` moves into the daemon. Phase 4 completes the surface: `mouse.rs` (the §5 selection contract — a per-frame `RegionMap` hit-tests each event to one region and clamps a drag to where it began; **only a terminal grid has a selection model**, so chrome cannot reach the clipboard; a child that requested mouse reporting gets the encoded event unless `⌥` is held; the host's shift-drag escape hatch is implemented by *omission* and must stay that way), `layout.rs`/`panes.rs` (a `Vec` stack with weights and a 3-cell splitter — a tab strip out-ranks the splitter overlapping it, or no group below the first could be clicked; strip hit-spans come from the same width computation that renders them), and `glyph.rs` (**every glyph is one cell wide in both unicode and `--ascii` modes**, asserted for every variant — East-Asian *Ambiguous* characters are excluded however well they read, since `unicode-width` calls them 2 in an East-Asian locale and 1 elsewhere). `rebase`/`push`/`merge-queue` landed in Phase 4d as two-phase clients of the **daemon's** ops, never the CLI's local path (ADR-0072 §9); the UI adds no safety logic of its own because the lease lives in the daemon, and what it owes is the *absence* of a bypass — pinned by the `no_force_escape_hatch_exists_in_the_ui_surface` grep-guard test, so no `force`/`remote`/`onto` field can be added to a request without failing the build. Phase 5 closes #1585: `session_layout.rs` persists the pane *shape* only (`~/.omni-dev/worktrees-ui-layout.yaml`, same `0600`/`0700`/atomic-rename convention as the row-colour store) — a restored tab is a **new** child (ADR-0072 §2), and restoration is silent and lossy by design so a convenience file can never fail a startup; the session picker labels rows with `relocate::transcript_preview` (head-of-file only, schema-tolerant, never logged) instead of a UUID; and `alt-⇧c` retires the module's last `dead_code` allow. +- `src/cli/worktrees/ui/` — **`omni-dev worktrees ui`**, a full-screen `ratatui` terminal UI and the registry's **second first-party client** (#1585, [ADR-0072](docs/adrs/adr-0072.md)). A read-only daemon client that deliberately does **not** register as a window (the daemon's `open`/`reposition` assume a VS Code window): `wire.rs` (hand-mirrored, `sanitize_for_terminal`-at-the-boundary DTOs), `supervisor.rs` (reconnect-with-backoff over `subscribe`, permanent polling fallback for an old daemon that replies `ok:false` and holds the connection open), `hub.rs` (the actor merging both feeds + local state into one `WorktreesViewModel`), `actions.rs` (the parity commands and the two-phase `close` fan-out — `close` isn't batched on the wire), `terminal/` (Phase 3: one embedded terminal tab via `alacritty_terminal` in-process, one PTY-reader OS thread per tab feeding the async loop over one channel). Two load-bearing PTY invariants, regression-tested against real children: `TERM`/`COLORTERM` set per spawn, and every `Event::PtyWrite` written back to the child. A `claude` tab launches through `omni-dev claude-wrap`, so those sessions report *authoritative* state (ADR-0057). Focus decides who gets a key — a focused terminal takes everything verbatim except `Alt`-chords, no tmux-style prefix; keys are legacy-xterm encoded and the kitty keyboard protocol is deliberately not advertised (`Config::kitty_keyboard` off). **PTY contents are never logged** — enforced by the `no_pty_content_is_ever_logged` grep-guard test, not convention; the emulator stays at `Osc52::OnlyCopy` so no clipboard→child path exists. Row colours are a local unsynced store (`~/.omni-dev/worktrees-ui-row-colors.yaml`, `0600`/`0700`) until `rowColors` moves into the daemon. Phase 4 completes the surface: `mouse.rs` (the §5 selection contract — a per-frame `RegionMap` hit-tests each event to one region and clamps a drag to where it began; **only a terminal grid has a selection model**, so chrome cannot reach the clipboard; a child that requested mouse reporting gets the encoded event unless `⌥` is held; the host's own selection escape hatch is implemented by *omission* and must stay that way — **which modifier it is varies by terminal**, measured in #1602: Ghostty keeps shift-drag, iTerm2 keeps option-drag and forwards shift-drag, the VS Code terminal keeps neither, so the mechanism is ours to leave alone and the modifier is not ours to encode), `layout.rs`/`panes.rs` (a `Vec` stack with weights and a 3-cell splitter — a tab strip out-ranks the splitter overlapping it, or no group below the first could be clicked; strip hit-spans come from the same width computation that renders them), and `glyph.rs` (**every glyph is one cell wide in both unicode and `--ascii` modes**, asserted for every variant — East-Asian *Ambiguous* characters are excluded however well they read, since `unicode-width` calls them 2 in an East-Asian locale and 1 elsewhere). `rebase`/`push`/`merge-queue` landed in Phase 4d as two-phase clients of the **daemon's** ops, never the CLI's local path (ADR-0072 §9); the UI adds no safety logic of its own because the lease lives in the daemon, and what it owes is the *absence* of a bypass — pinned by the `no_force_escape_hatch_exists_in_the_ui_surface` grep-guard test, so no `force`/`remote`/`onto` field can be added to a request without failing the build. Phase 5 closes #1585: `session_layout.rs` persists the pane *shape* only (`~/.omni-dev/worktrees-ui-layout.yaml`, same `0600`/`0700`/atomic-rename convention as the row-colour store) — a restored tab is a **new** child (ADR-0072 §2), and restoration is silent and lossy by design so a convenience file can never fail a startup; the session picker labels rows with `relocate::transcript_preview` (head-of-file only, schema-tolerant, never logged) instead of a UUID; and `alt-⇧c` retires the module's last `dead_code` allow. **#1602 adds right-click context menus** on all four surfaces (tree rows, tab strips, terminal grids, splitter/global chrome). `menu.rs` holds the per-surface contents as **pure functions of small inputs**, so "the strip menu hides *Move to Group Above* with no group above" is a direct unit test; the tree menu is the exception, built from `applicable_action_groups` so it cannot drift from the `a` menu and `:` palette. `popup.rs` owns `place` (anchor + flip + clamp, pure) and `draw_menu`, which **returns the `PopupRegion` it just drew** so the hit map and the frame cannot disagree — the invariant `GroupRegion::tab_spans` already keeps. `popup_mouse_enabled` is the single predicate naming the one popup kind that takes the mouse; the confirm modal stays keyboard-only because its `y`/`n` gates destructive actions. Contract §6 and §7 are added to `mouse.rs`: **the right button opens a menu and never inspects modifiers** (four terminals, four conventions — Terminal.app forwards both, Ghostty plain-right, iTerm2 shift-right, VS Code both plus its own menu — so one non-rule is correct everywhere *because* it does not branch; a `Shift` test in either direction breaks exactly one), and a menu is modal over what it covers, its dismissing click consumed. `alt-m` is the **primary** gesture since no right-click gesture works everywhere; `F10` is handled tree-side in `app.rs`, deliberately not as a `ChromeKey`, because `encode_key` already sends it to a child and Alt-chords are the only chrome keys. *Paste* is absent by decision, pinned by the `no_host_clipboard_read_exists_in_the_ui_surface` guard — the other half of `Osc52::OnlyCopy`, so neither a child nor the UI can read the clipboard. **#1605**: `TerminalTab::shutdown` reaps the child's whole **process group** (`SIGHUP` → 250ms → `SIGKILL`) *before* joining, because `tty::Pty`'s drop `wait()`s with no deadline and the runtime waits for blocking tasks — so a child ignoring `SIGHUP` could keep the process alive after the terminal was already restored. A rarer stall in the reader thread's own join remains (#1611): **run this module's tests with `--test-threads=1`**, and never by skipping them. - `editors/vscode/` — the companion VS Code extension (TypeScript; the daemon's first non-omni-dev client, #1111): a thin per-window reporter plus the tree-view UI. `src/socket.ts` recomputes the socket path (`dirs::data_dir()`) and holds the NDJSON client + envelope builders (including `closeCheck`/`close`; no `vscode` import, so it is unit-tested with `node --test`); `src/extension.ts` is the activate→register / ~10s heartbeat (**close self on `close:true`**, else re-register on `known:false`) / deactivate→unregister lifecycle. The tree view adds two context-menu commands (#1277) routed by an enriched `contextValue` (`worktreeContextValue` encodes `is_main` as `.main`/`.linked`): **Close Worktree** (linked; phase-1 check → conditional modal confirm → phase-2 execute in `withProgress`) and **Close Window** (main tree; close-only, never a delete). #1407 adds **Reposition Windows to Match** (the `4_layout` group, gated `viewItem =~ /worktree\.open\./` so it never offers the invoking window, plus a `canReposition` context key set from `process.platform === "darwin"`) and a palette-visible **Undo Reposition Windows**; the handler reuses `partitionByWindow`/`partitionSelfLast` (no new tree code), fires with **no modal**, and toasts a summary carrying an **Undo** action, branching on the reply's `trusted` field to offer an "Open Accessibility Settings" deep-link. #1415 turns **Rebase on main** (`4_git`, every worktree row) from a terminal shell-out into a socket client — `rebaseCommand.ts` is now a thin `vscode` adapter over the pure, `node --test`-ed `rebaseReport.ts` (modal detail, skip/summary text), taking `send`/`windowKey` injected from `extension.ts` since those are module-private there; `omniDev.ts` (the CLI binary resolver + argv/command-line builders) is **deleted**, so a daemon-down rebase now errors like every other action and names `omni-dev worktrees rebase` as the way out. #1438/[ADR-0060](docs/adrs/adr-0060.md) drops the linked-only gate and the client-side main-tree pre-filter, so a main working tree row is offered and rebased like any other target. #1430 adds **Copy PR URL** (`3_copy@2`, beside Copy Directory and gated identically on `viewItem =~ /^(repo|worktree)/` — *not* the `/github/` gate the two Open commands use, since a non-GitHub or detached row's truthful answer is the placeholder): `prClipboard.ts` is the pure, `node --test`-ed line builder and `prCommands.ts::copyPullRequestUrls` the thin adapter. Its one structural difference from `selectPullRequests` is that it reports per selected **row**, not per discovered PR — discovery stays per *scope*, so the outcomes are joined back onto the rows by the newly-exported `prScopeKey` — which is what lets a PR-less row emit a `#`-commented placeholder instead of vanishing, and a *failed* lookup emit a distinct comment so it can never read as a settled "no PR". URLs dedupe across the block, placeholders never. #1442 adds **Open GitHub Repository** (`0_open@2`, beside Open Worktree — an *open* action, not a PR one, so deliberately outside `1_pr` — gated `viewItem =~ /^repo\.github/`, the `repoContextValue` prefix, so it is offered on GitHub **repo rows only**): `github.ts::repoWebUrl`/`repoWebUrlsForNodes` are the pure builders and `repoCommands.ts::openGithubRepository` the thin adapter. It is the cheapest action in the view — the URL is a pure function of the snapshot's `github` identity, so alone among the GitHub actions it costs **no `gh`, no subprocess, no network and no daemon op**, and needs no progress notification or confirm. A worktree row in a multi-selection still resolves to its parent repo and dedupes by URL, since a `when` clause is evaluated against the *clicked* row only. Bundled with esbuild → `dist/extension.js`, packaged to `.vsix` by `.github/workflows/vscode-extension.yml` (path-filtered on `editors/vscode/**`). The Rust build never descends into `editors/` — it is single-crate and `editors/` is in `Cargo.toml`'s `exclude`, so `cargo publish` never ships it. Marketplace/Open VSX publishing is a deferred follow-up (needs a publisher account + CI secrets). **Three destructive ops, one new OS trust surface:** requests ride the daemon's existing `0600` Unix socket; **no secret is persisted** (in-memory only, the close directive and the rebasing set included); the companion is the daemon's first non-omni-dev client. The `rebase` op **rewrites git history** and spends the user's ambient `ssh-agent` credentials ([ADR-0059](docs/adrs/adr-0059.md)) — same-user-bounded, daemon-re-validated on execute, rewriting only branches the client names (main working tree included, since [ADR-0060](docs/adrs/adr-0060.md) — the `is_main` guard is `close`'s alone now); its one genuinely new residual is that `keep_conflicts` lets a socket request leave a worktree **mid-rebase** (intended, git-recoverable, and cued durably in the tree). The `push` op ([ADR-0061](docs/adrs/adr-0061.md)) is the first to **write** to a remote with those ambient credentials — the largest escalation so far, since it publishes rewritten history — but the mitigation that carries the weight is not ours: the lease is enforced by `git`, and with no force escape hatch in the surface the daemon cannot overwrite a remote tip it has not seen. The `close` op deletes files and closes windows — a real threat-model escalation ([ADR-0049](docs/adrs/adr-0049.md)) but same-user-bounded and guarded in the daemon (`git2`-enforced real-linked-worktree-only, main-tree removal refused). The `reposition` op adds no socket capability but needs the macOS **Accessibility** grant — coarse enough that the capability the daemon *holds* far exceeds the one it *uses*, so the narrowness (titles/geometry/flags read, position+size written, own-user processes named by pid, never input synthesis or a raise) is enforced in our code, in one `#[allow(unsafe_code)]` module behind a trait ([ADR-0058](docs/adrs/adr-0058.md)). The grant is opt-in, cannot be self-granted, needs a `daemon restart` to take effect, and is **invalidated by every upgrade** because omni-dev installs unsigned. Keep [ADR-0040](docs/adrs/adr-0040.md), [ADR-0048](docs/adrs/adr-0048.md), [ADR-0049](docs/adrs/adr-0049.md), [ADR-0058](docs/adrs/adr-0058.md), [ADR-0059](docs/adrs/adr-0059.md), [ADR-0060](docs/adrs/adr-0060.md), [ADR-0061](docs/adrs/adr-0061.md) and the operator guide + companion contract [docs/worktrees-service.md](docs/worktrees-service.md) in sync, and run the [`update-snapshots`](.claude/skills/update-snapshots/SKILL.md) skill on any CLI-surface change (see Code Changes §5).