diff --git a/CLAUDE.md b/CLAUDE.md index 6b9f06d4..76f33d7b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -116,6 +116,8 @@ would double-load every global. | `config.py` | Cross-cutting paths + constants; `instance_file()` keeps prod/dev stores apart | | `log.py` | Logging + `_bg` / `_task` crash wrappers — every thread/task goes through these so crashes surface | | `pidfile.py` | Single-instance reclaim (reloader child counts as the same instance) | +| `forks.py` | Audit-hook fork counter + each poll's cost (`/api/state` `build_stats`, the resource popover's poll block; the full map on `/api/healthz`) | +| `gh.py` | Every `gh` call: token read every 10 minutes, telemetry and update check off | | `tmux.py` / `tmux_input.py` / `tmux_mirror.py` | Subprocess wrappers / persistent control-mode input client / control-mode pane mirror with reconcile frames | | `store.py` | `state.json` layer (`_STATE`, load/write, migrations) | | `channels.py` | In-process MCP server + tool implementations | diff --git a/docs/invariants.md b/docs/invariants.md index 7df2d535..c66f92ff 100644 --- a/docs/invariants.md +++ b/docs/invariants.md @@ -8,6 +8,10 @@ These are the non-obvious behaviors worth preserving: active window in its session, or when the user acts on it via the dashboard (focus/send). See `update_focus_from_windows`. + The window rows come from one `list-panes -a` (`tmux.list_panes`), one + row per window off its active pane (`#{pane_active}`); `active` still + reads `#{window_active}`. + 2. **Claude detection is anchored to the bottom of the pane.** A pane is Claude when its composer (`❯` between two full-width rules) has no more than a footer under it, or the status line is in the last 4 non-empty @@ -189,6 +193,14 @@ These are the non-obvious behaviors worth preserving: cross-poll residue; `grep "re-minting\|duplicate @periscope_id"` catches both. + `list_panes` carries `#{window_id}` and the window option + `#{@periscope_id}` on every pane row, so a stamp still targets the + window id. `list_windows` always forks a fresh `list-panes` — a stamp or + a new window must read back at once — and only `all_pane_ids` and + session_status's pane → pid map read the shared snapshot + (`tmux.panes_snapshot`: the poll's own rows inside `build_state`, + else under 1 s old). + 12. **Rebind eligibility (`_REBIND_TTL_S`, 15 min) is NOT GC retention (`_PID_TTL_S`, 30 days).** Rebind exists so persisted state reattaches when the tmux server restarts — window options are lost, so every diff --git a/docs/superpowers/plans/2026-11-02-fewer-forks-per-refresh.md b/docs/superpowers/plans/2026-11-02-fewer-forks-per-refresh.md new file mode 100644 index 00000000..28039b1c --- /dev/null +++ b/docs/superpowers/plans/2026-11-02-fewer-forks-per-refresh.md @@ -0,0 +1,2882 @@ +# Fewer Forks Per Refresh Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** `build_state` measures its own cost, forks one `tmux` and one `ps` per refresh instead of six processes, keeps git off the request path, and runs every `gh` without a keychain read (THI-450; spec `docs/superpowers/specs/2026-11-02-fewer-forks-per-refresh-design.md`). + +**Architecture:** A Python audit hook (`periscope/forks.py`) counts every `subprocess.Popen` by command over a rolling minute; `build_state` and the state hub record their timings there and `/api/state` reports them as `refresh`. One `list-panes -a` (`tmux.list_panes`, refreshed by every `list_windows`) and one locale-pinned `ps` (`agent_processes.process_snapshot`, executable from argv[0]) feed every per-refresh reader; git reads two commands per cwd behind a jittered stale-while-revalidate cache, and `gh` goes through one `gh.run` that carries a token read every 10 minutes. + +**Tech Stack:** Python 3.14 / FastAPI / pytest + pytest-mock; Preact + `@preact/signals`, Vite, vitest; tmux, git (porcelain v2), gh, macOS `ps`. + +--- + +## Conventions + +- **Worktree:** `/Users/thibaultdody/dev/worktrees/periscope/thi-450-fewer-forks` (branch `thibaultdody/thi-450-build_state-runs-too-many-subprocesses-per-tick-so-the`). Every path below is relative to it. Never touch the prod tree `/Users/thibaultdody/github/tomplex/periscope` or anything on `:17374`. +- **First, once:** the worktree has no `.venv` and the repo pins no Python, so uv would pick CPython 3.15 and fail to build watchfiles. Run `uv venv --python 3.14 && uv sync` (prod's `../main/.venv` is 3.14.7). It also has no `node_modules`: run `npm install` before any `npm`/`npx` command. +- **Read first:** the spec, `docs/invariants.md` (#1, #8, #11, #13, #14, #18, #19), `docs/testing.md`, `tests/conftest.py`. +- **Python tests:** `uv run pytest -q `; full suite `uv run pytest -q`. +- **Frontend:** `npm test`; build `npm run build`. +- **Gate:** `bin/check; echo "exit=$?"` — read the real exit code, never pipe it through `tail`/`head`. +- **Before each commit:** `uv run ruff check --fix && uv run ruff format ` on the task's Python files (sorts imports the steps add). +- **Commits:** one per task, single-line subject `area: what (THI-450)`, a blank line, then the implementing model's attribution line. Commit only the task's paths: new files need `git add ` first, then `git commit -m "" -m "Co-Authored-By: Claude Opus 5.5 (1M context) " -- `. Replace the Co-Authored-By line with your own model's if you are not Claude Opus 5.5. +- **Background work** goes through `periscope.log._bg` (invariant #8). Tests never start a real background thread for it: they patch the module's `_bg`, as `test_reviews.py` already does. +- **No test forks real tmux, ps or gh by accident.** `tests/conftest.py` seeds each new snapshot cache with an unexpiring empty entry (the `_config_dirs_cache` pattern `test_session_status.py` already uses); a test of the real path resets the seed itself. Two tests fork on purpose: the live-tmux oracle (Task 4) and the live-`ps` executable test (Task 3). + +## File structure + +| File | Change | Responsibility | +|---|---|---| +| `periscope/forks.py` | Create | Audit hook, rolling fork counter, build/gap timings, the `refresh` block | +| `periscope/app.py` | Modify (lifespan, first lines) | Install the hook | +| `periscope/routes/state.py` | Modify (L16, L69, L167–208) | Time `build_state`; add `refresh` to the blob | +| `periscope/routes/healthz.py` | Modify (imports, `healthz`) | The full per-command `forks` map | +| `periscope/state_hub.py` | Modify (imports, `_run_loop` L96–111) | Record the gap between broadcasts, reset on idle | +| `periscope/agent_processes.py` | Modify (`ProcessInfo`, `_PS_ROW_RE`, `parse_ps_snapshot`, `_take_snapshot`, `_is_codex_executable`) | `args`-only `ps` under `LC_ALL=C`; `exe` from argv[0] | +| `periscope/session_status.py` | Modify (L29–58, L128–241, L243–259) | `proc_table` / `_claude_procs` from `process_snapshot`; `_tmux_pane_pids` from `tmux.recent_panes`; `parse_etime` deleted | +| `periscope/window_view.py` | Modify (comment L164–171) | Comment names the shared snapshot | +| `periscope/tmux.py` | Modify (imports, `_run` L33–43, new section) | `list_panes` / `recent_panes` (`\x1f`-separated); `_run(env=)` | +| `periscope/panes.py` | Modify (L21, L385–449) | `list_windows` and `all_pane_ids` read `list_panes` / `recent_panes` | +| `periscope/git_pr.py` | Modify (L18–43, L65–128, gh call sites) | Two-command `git_state_for` + work-tree-less fallback, identity cache, SWR `cached_git_state`, `gh.run` | +| `periscope/gh.py` | Create | The one way periscope runs `gh` | +| `periscope/reviews.py`, `periscope/cleanup.py`, `periscope/projects.py` | Modify (their `gh` call) | Use `gh.run` | +| `static/src/store.js`, `static/src/poll.js` | Modify | `refresh` signal | +| `static/src/chrome/resources.js`, `static/src/chrome/ResourcePill.jsx`, `static/styles.css` | Modify | The popover's refresh line | +| `static/dist/*` | Rebuild | Committed bundle | +| `tests/conftest.py` | Modify | Seed tmux / ps / gh-token caches; fresh git caches | +| `tests/test_forks.py`, `tests/test_gh.py` | Create | Hook, counter, stats; gh environment | +| `tests/test_agent_processes.py` | Rewrite | Real-format `ps` rows, argv[0], locale, live `ps` | +| `tests/test_resources.py` | Modify (`_proc`, L67–84) | `ProcessInfo.exe` | +| `tests/test_panes.py`, `tests/test_tmux.py`, `tests/test_session_status.py`, `tests/test_git_pr.py`, `tests/test_reviews.py`, `tests/test_state_hub.py`, `tests/routes/test_state.py`, `tests/routes/test_healthz.py` | Modify | Cases per task | +| `static/src/chrome/__tests__/resources.test.js`, `resourcePillRender.test.jsx` | Modify | Refresh line | +| `docs/invariants.md`, `docs/testing.md`, `CLAUDE.md` | Modify | Snapshot note, conftest seeds, module table | + +`periscope/gitutil.py` does not change: the 10-minute identity cache lives in `git_pr.py`, because `resolve_repo` / `github_slug` have other callers (open_ops, reviews) that need a live answer. + +--- + +## Task 1: The fork counter + +**Files:** +- Create: `periscope/forks.py` +- Modify: `periscope/app.py` (lifespan, the first lines after `async def lifespan`) +- Test: `tests/test_forks.py` (new) + +- [ ] **Step 1: Write the failing tests** — create `tests/test_forks.py`: + +```python +"""forks — the audit hook that counts every fork the server makes, and the +per-refresh cost /api/state reports.""" + +import subprocess +import sys +from collections import deque +from types import SimpleNamespace + +import pytest + +from periscope import forks + + +@pytest.fixture(autouse=True) +def _fresh_counters(monkeypatch): + monkeypatch.setattr(forks, "_forks", deque()) + + +def _popen(*argv): + forks._on_audit("subprocess.Popen", (argv[0], list(argv), None, None)) + + +@pytest.mark.parametrize( + ("argv", "key"), + [ + (["git", "-C", "/repo", "status", "--porcelain=v2"], "git status"), + (["git", "--no-optional-locks", "-C", "repo", "diff", "HEAD"], "git diff"), + (["/opt/homebrew/bin/tmux", "-L", "periscope-x", "list-panes", "-a"], "tmux list-panes"), + (["ps", "-axo", "pid=,ppid=,lstart=,state=,args="], "ps"), + (["ps", "eww", "-o", "pid=,command=", "-p", "1,2"], "ps eww"), + (["gh", "pr", "list", "--head", "x"], "gh pr"), + (["/bin/sh", "-c", "echo hi"], "sh"), + ([b"/usr/bin/true"], "true"), + ], +) +def test_command_key_is_the_binary_and_its_subcommand(argv, key): + assert forks._command_key(argv) == key + + +def test_hook_ignores_every_other_audit_event(): + forks._on_audit("open", ("/etc/hosts", "r", 0)) + forks._on_audit("subprocess.Popen", ("x", [], None, None)) # no argv[0]: uncounted + assert forks.fork_counts() == {} + + +def test_hook_never_raises_into_the_audited_popen(monkeypatch): + # An exception escaping an audit hook aborts the Popen it audits. + class Broken: + def append(self, item): + raise RuntimeError("boom") + + monkeypatch.setattr(forks, "_forks", Broken()) + _popen("git", "-C", "/r", "status") + + +def test_counts_roll_off_after_a_minute(monkeypatch): + clock = [1000.0] + monkeypatch.setattr(forks, "time", SimpleNamespace(monotonic=lambda: clock[0])) + _popen("git", "-C", "/r", "status") + clock[0] += 30 + _popen("git", "-C", "/r", "status") + _popen("ps", "-axo", "pid=") + assert forks.fork_counts() == {"git status": 2, "ps": 1} + clock[0] += 31 + assert forks.fork_counts() == {"git status": 1, "ps": 1} + + +def test_installed_hook_counts_run_and_popen(): + forks.install() + subprocess.run(["true"], check=True) + subprocess.Popen(["true"]).wait() + assert forks.fork_counts()["true"] == 2 + + +def test_install_adds_the_hook_once(monkeypatch): + added = [] + monkeypatch.setattr(forks, "_installed", False) + monkeypatch.setattr(sys, "addaudithook", added.append) + forks.install() + forks.install() + assert added == [forks._on_audit] +``` + +- [ ] **Step 2: Run them and see them fail** + +Run: `uv run pytest -q tests/test_forks.py` +Expected: collection error `ImportError: cannot import name 'forks' from 'periscope'`. + +- [ ] **Step 3: Implement** — create `periscope/forks.py`: + +```python +"""Every fork the server makes, counted by command over a rolling minute. + +Counted by a Python audit hook on `subprocess.Popen` — the event +`subprocess.run`, `check_output` and asyncio's subprocesses all raise — so no +call site changes and background threads count too. It sees only the server's +own forks: what a child spawns (gh's `security`, `gh send-telemetry`) needs a +process-tree sample.""" + +import contextlib +import os +import re +import sys +import threading +import time +from collections import Counter, deque +from collections.abc import Sequence +from typing import Any + +_WINDOW_S = 60.0 +# The subcommand is the first word-like argument after the binary, so +# `git -C status` counts as `git status` and `ps -axo pid=,…` as `ps`. +_SUBCOMMAND_RE = re.compile(r"[a-z][a-z0-9-]*") +# Options whose value comes next and could pass for a subcommand +# (`tmux -L `, `git -C `, `sh -c