revdiff
umputun/revdiff/CLAUDE.md
TUI for reviewing diffs, files, and documents with inline annotations, built with bubbletea. Architecture: see docs/ARCHITECTURE.md for system design, data flows, interfaces, and design decisions.
CLAUDE.md901 starsChanged 26 days ago
- Installs packages
# revdiff
TUI for reviewing diffs, files, and documents with inline annotations, built with bubbletea.
**Architecture**: see [docs/ARCHITECTURE.md](docs/ARCHITECTURE.md) for system design, data flows, interfaces, and design decisions.
## Commands
- Build: `make build` (output: `.bin/revdiff`)
- Test: `make test` (race detector + coverage, excludes mocks)
- Lint: `make lint` or `golangci-lint run`
- Format: `make fmt` or `~/.claude/format.sh`
- Generate mocks: `go generate ./...`
- Vendor after adding deps: `go mod vendor`
## Project Structure
- `app/` - all Go source
- `app/revdiff/` - composition root (`package main`), split by concern: `main.go` (entrypoint + `run()`), `config.go` (options/parsing), `stdin.go` (stdin mode), `renderer_setup.go` (VCS wiring), `themes.go` (theme CLI + adapter), `history_save.go` (session save)
- `app/diff/` - VCS interaction (git + hg + jj), unified diff parsing, VCS detection, Mercurial + Jujutsu support. `compare.go` — `CompareReader` renderer for `--compare-old/--compare-new` (runs `git diff --no-index`, no repo required)
- `app/ui/` - bubbletea TUI package. Single `Model` struct with state grouped into sub-structs (`cfg`, `layout`, `file`, `modes`, `nav`, `search`, `annot`), methods split across files by concern (~500 lines each). Each source file has a matching `_test.go`. See `app/ui/doc.go` for package docs, `docs/ARCHITECTURE.md` for file-by-file breakdown. Does not import `app/theme` or `app/fsutil` — theme operations go through the `ThemeCatalog` interface
- `app/ui/style/` - color/style resolution: hex-to-ANSI, lipgloss styles, SGR tracking, HSL math. Types: `Resolver`, `Renderer`, `SGR`. Also `display.go` - package-level `SanitizeFilenameForDisplay` and `TruncateLeftToWidth`, the shared helpers every filename-rendering surface must route through
- `app/ui/sidepane/` - file tree + markdown TOC components with cursor/offset management
- `app/ui/worddiff/` - intra-line word-diff: tokenizer, LCS, line pairing, highlight insertion
- `app/ui/overlay/` - layered popups: help, annotation list, theme selector, file picker. Manager enforces one-at-a-time
- `app/highlight/` - chroma syntax highlighting, foreground-only ANSI
- `app/keymap/` - configurable keybindings (`Action` constants, parser, defaults, dump)
- `app/theme/` - Catalog-centric theme system: `Theme` (data + serialization) and `Catalog` (discovery, loading, installation, gallery). Zero standalone functions — all logic as methods. Files: `theme.go` (Theme struct), `catalog.go` (Catalog struct + all operations). 7 bundled + community gallery
- `app/annotation/` - in-memory annotation store and structured output
- `app/editor/` - external `$EDITOR` invocation for multi-line annotations: temp-file lifecycle, editor resolution ($EDITOR → $VISUAL → vi), and `Command()` API returning `*exec.Cmd` + completion func for `tea.ExecProcess`. Consumed by `app/ui` via the `ExternalEditor` interface
- `app/handoff/` - prepares user-configured post-flush shell commands. Annotation snapshots are provided on stdin; stdout is suppressed so helpers cannot overwrite the TUI. Consumed by `app/ui` via the `PostFlushHook` interface and `tea.ExecProcess`
- `app/history/` - review session auto-save to `~/.config/revdiff/history/`
- `app/fsutil/` - filesystem utilities
- `app/ui/mocks/` - moq-generated mocks (never edit manually)
## Architecture Principles
- **Decouple OS/external concerns from UI**: OS-level work (exec.Command, os.CreateTemp, env lookup, network calls) does not belong in `app/ui/`, even for small helpers. Extract to a dedicated package (e.g. `app/editor/`) even when there's only one production caller. The cost of a new package is lower than keeping `app/ui/` entangled with OS boundaries.
- **Minimize exported surface**: first pass of a new package almost always over-exports. Before finalizing, ask "which of these does the caller actually need?" and unexport the rest. For stateless helpers, prefer unexported methods on the grouping struct over top-level unexported functions — matches the global "prefer methods over standalone utilities" rule.
- **Consumer-side interfaces for external deps**: when UI uses an external subsystem, the default shape is: interface in `app/ui/` (consumer side), concrete type in the subsystem package, injection via `ModelConfig` (nil defaults to concrete). Direct import of a concrete type from an external package into `app/ui` is a smell — the interface documents the dependency direction and leaves room for alternate implementations, not just test mocks.
## Config
- Config file: `~/.config/revdiff/config` (INI format via go-flags IniParser)
- Precedence: CLI flags > env vars > config file > built-in defaults
- `--dump-config` outputs current defaults, `--config` overrides path
- `no-ini:"true"` tag excludes fields from config file (used for --config, --dump-config, --dump-theme, --list-themes, --init-themes, --version)
- Themes dir: `~/.config/revdiff/themes/` with 7 bundled themes, auto-created on first run
- `--theme NAME` loads theme; `--dump-theme` exports resolved colors; `--list-themes` lists available; `--init-themes` re-creates bundled
- Theme precedence: `--theme` overwrites all 23 color fields + chroma-style, ignoring `--color-*` flags or env vars
- Theme values applied via `applyTheme()` in `themes.go` which overwrites `opts.Colors.*` after `parseArgs()`. `colorFieldPtrs(opts)` is the single source of truth for color key → struct field mapping — adding a new color requires changes in `theme.go` colorKeys + options struct + `colorFieldPtrs()` in `themes.go`
- Theme ownership split: `app/theme.Catalog` owns discovery/loading, `app/ui.ThemeCatalog` interface consumed by UI, `app/revdiff/themes.go` wires adapter composing catalog + config persistence
- `ini-name` tags ensure config keys match CLI long flag names
- Keybindings file: `~/.config/revdiff/keybindings` (`map <key> <action>` / `unmap <key>` format)
- `--keys` overrides keybindings path, `--dump-keys` prints effective bindings
- **CLI flag description style is minimal and atomic** — match `--staged` ("show staged changes") / `--blame` ("show blame gutter"). Never include "at startup", "on startup", "(mirrors X toggle)", "(same state as X)", or cross-references to runtime toggle keys in the struct tag description, README/docs.html/plugin config.md table rows, godoc, or usage example comments. The flag description states what the flag does; users discover runtime toggles via the keybindings table or status-bar legend. This rule applies to every surface that describes a flag.
- **Mode-gating pattern for CLI flags with mode-dependent applicability**: when a flag is meaningful in some modes (working-tree, single-ref, `--staged`) but not others (two-ref, `--stdin`, `--compare-old/--compare-new`), gate it at the composition root via a method on `options` that returns the resolved bool, parallel to `options.ref()`. Example: `options.startupUntracked()` returns `false` in two-ref mode (`a b` or `a..b`) so working-tree state doesn't leak into historical diffs. `main.go` then wires `ShowUntracked: opts.startupUntracked()` into `ModelConfig`. The Model takes the resolved bool — it does NOT re-derive from CLI options or refs. Composition-root gating composes cleanly with the Model's own capability gate (e.g. `cfg.ShowUntracked && cfg.LoadUntracked != nil`) which handles the stdin/compare modes where the loader function is nil.
## Website
- Static site in `site/` (index.html, docs.html, style.css), deployed to revdiff.com via Cloudflare Pages
- Cloudflare Pages strips `.html` and 308-redirects `/docs.html` → `/docs`. Canonical tags, `og:url`, and `sitemap.xml` entries for documentation pages must use the extension-less URL (`/docs`), not the source filename (`/docs.html`), or Google indexes a redirect and tanks CTR
- `site/docs.html` must stay in sync with README.md - when adding features, flags, keybindings, or modes, update both
- `site/index.html` landing page should reflect major new features in the features grid and plugin sections
- **CRITICAL: After each release, update the version badge in `site/index.html`** (search for `hero-badge` div) and `softwareVersion` in JSON-LD
## Claude Code Plugin
- Plugin lives at `.claude-plugin/` with `plugin.json`, `marketplace.json`, and `skills/`
- Skills path in `plugin.json` is relative to repo root, not to `.claude-plugin/`
- **CRITICAL: Version bumps happen at release only — never per-PR or per-change.** Do NOT prompt to bump `plugin.json` / `marketplace.json` after a plugin file change; the bump is done as part of the release process.
- When bumping at release, keep every marketplace entry synchronized with its plugin manifests. For `revdiff`, update `.claude-plugin/plugin.json`, `plugins/codex/.codex-plugin/plugin.json`, and its version in `.claude-plugin/marketplace.json`. For `revdiff-planning`, update `plugins/revdiff-planning/.claude-plugin/plugin.json`, `plugins/revdiff-planning/.codex-plugin/plugin.json`, and its version in `.claude-plugin/marketplace.json`.
- **CRITICAL: Defer plugin version bumps when the change depends on a new binary feature.** If a plugin/launcher change relies on a `revdiff` binary feature, flag, env var, or exit code that is not yet in a tagged release, do NOT bump `plugin.json` / `marketplace.json` / `package.json` on the feature branch. The plugin (marketplace) and the binary (brew / `go install`) version independently — bumping the plugin early ships an updated launcher to users still running an old binary, causing a hard mismatch (e.g. the launcher passes an unknown flag, the old binary exits 1, every plugin-triggered review fails). Bump plugin/package versions as part of the binary version release, after the binary is tagged.
- Reference docs at `.claude-plugin/skills/revdiff/references/` — keep in sync with README.md:
- `install.md` — installation methods and plugin setup
- `config.md` — options, colors, chroma styles
- `usage.md` — examples, key bindings, output format
- **Adding a new CLI flag requires SKILL.md updates, not just reference docs.** `references/config.md` and `references/usage.md` document the flag's *existence*; `SKILL.md` teaches AI agents *when* to pass it during automatic launches (e.g. "pass `--untracked` when the recent change likely created new untracked files"). Without a SKILL.md entry, AI agents using the plugin will not know to pass the flag even though it's documented. Apply the same update to `plugins/codex/skills/revdiff/SKILL.md` (keep in sync with `.claude-plugin/skills/revdiff/SKILL.md`) and to `plugins/pi/skills/revdiff/SKILL.md` (which lists user-facing command examples). The launcher scripts (`launch-revdiff.sh`) pass `"$@"` through, so no script changes are needed beyond updating the usage-comment header for documentation parity.
- **Launchers must parse under bash 3.2** (`/bin/bash` on stock macOS, where `#!/usr/bin/env bash` resolves unless a newer bash is on PATH). Its parser scans a `$( )` command substitution for quotes **before** it processes a heredoc inside it, so an odd number of apostrophes in a heredoc opened **inside** one opens a quote that never closes and the whole script fails to parse — reported a hundred lines later, on an unrelated valid line. Two heredocs per launcher are nested that way, the `GHOSTTY_TERM_ID=$(osascript ...)` and `ITERM_NEW_SESSION=$(osascript ...)` captures; the close-pane heredocs beside them are plain commands and are unaffected. Keep the nested bodies apostrophe-free; balancing them is not a fix, since a later edit to one word re-breaks it. No parse check can guard this in CI — ubuntu and every Homebrew bash accept the broken form — so `TestLauncherNestedHeredocsHaveNoApostrophes` guards it textually instead, and `/bin/bash -n` is the direct check on macOS. Introduced by #309, reported in #314.
- **shellcheck is pinned in CI** (`koalaman/shellcheck:v0.11.0`, digest-pinned in `.github/workflows/ci.yml` because a docker tag is mutable and a retag would change the checker silently) because the runner image's version drifts and a `# shellcheck disable=` is version-sensitive: `SC2317` ("command appears unreachable") was split into `SC2329` ("function never invoked") in 0.11, so a suppression naming only the code your local shellcheck emits passes locally and fails CI — which is exactly how #344 broke. Name **both** codes. The one place this bites is `herdr_cleanup_unlaunched`: it is the only trap in the launcher that calls a *function* rather than inlining its commands, and shellcheck cannot see invocations inside the trap's quoted string. The warning is a false positive — mutating the function's body fails three `TestHerdrSignalPaneOwnership` cases — so suppress it, don't inline the function to appease the linter. Reproduce CI exactly with `find . -name '*.sh' -not -path './.git/*' -not -path './vendor/*' -print0 | xargs -0 docker run --rm -v "$PWD":/mnt:ro koalaman/shellcheck:v0.11.0@sha256:61862eba1fcf09a484ebcc6feea46f1782532571a34ed51fedf90dd25f925a8d`.
- **Launcher override chain**: both Claude plugins resolve their launcher script via `resolve-launcher.sh` through `user → bundled` layers (first executable wins). The planning plugin's user layer is `${CLAUDE_PLUGIN_DATA}/scripts/<launcher>` under Claude and `${PLUGIN_DATA}/scripts/<launcher>` under Codex. There is **no project-level (`.claude/...` or `.codex/...`) executable layer by design** — the planning hook fires automatically in any repo, and a repo-controlled launcher would run on routine agent actions. The Pi extension and manual Codex diff-review skill do not use this plugin-data override.
- **Overlay stderr relay**: `launch-revdiff.sh` appends `2>$ERR_FILE` to `REVDIFF_CMD` once, right after the argument loop, so every backend captures revdiff's stderr without per-backend edits (the redirect stays the trailing token through `write_rc_cmd` / `write_fifo_rc_cmd`, the heredoc launch scripts, and the later `/usr/bin/env` prepend). `print_output_and_exit` replays the file on any exit code other than 0 or 10 — those two are successes, and revdiff writes ordinary warnings to stderr, so relaying them would put noise on every successful review. All ten `EXIT` traps in `launch-revdiff.sh` list `$ERR_FILE` — an override that omits it leaks the file into `$TMPDIR`, which `TestShellLaunchersPreserveAnnotationExitCode` catches by asserting no `revdiff-err-*` survives the run. The stderr expectations live in that test because a second launcher-by-backend pass puts the main package over the timeout in `make race` (now `-timeout=180s`, matching CI); a new launcher-wide behavior belongs in the same matrix, gated per launcher via `relaysStderr` (`launch-plan-review.sh` has no relay). The sourced agent-deck window backend (`agentdeck-window.sh`) is the eleventh execution path and carries no relay code of its own: it builds its command through `write_rc_cmd`, exits through `print_output_and_exit`, and installs no `EXIT` trap by design, so it rides the base one. It is the only backend absent from `launcherBackends()`, so the relay tests do not cover it. `plugins/revdiff-planning/scripts/launch-plan-review.sh` has no relay: its per-backend exit tails are duplicated inline with no shared helpers.
- **herdr pane-scoped overlay (`REVDIFF_HERDR_PANE=1`)**: opt-in, off by default; unset, the `herdr tab create` + `pane run` block is reached byte-identical, because pane mode is a block ahead of it that exits on its own rather than a flag threaded through the shared path. The tab block itself is byte-identical, but the EXIT trap and the generated launch script ARE shared — the ownership machinery there is inert in tab mode only because `HERDR_TARGET` stays empty, so treat both as pane/tab common code when editing. **`$HERDR_PANE_ID` is a name collision** — herdr injects it into every managed pane and the tab path uses it as its own local, so the caller's id is copied to `HERDR_CALLER_PANE` first; losing it types the launch command into the agent's own shell. Ownership is `HERDR_TARGET` (non-empty = a pane owes a close); `herdr_close_pane` clears it before shelling out so a re-entrant call cannot close twice. **Cleanup is decided by evidence from the pane, not by `pane run` returning** — herdr may start the review before that call returns, or not after it. The dispatched script's first line touches `$SENTINEL.started`, and `HERDR_DISPATCHED` is set to 1 immediately before `pane run` is called; `herdr_cleanup_unlaunched` preserves the pane while **either** signal says a review may exist and the sentinel says it has not finished, closing and cleaning up otherwise (never started — holds only a shell; already finished — done). `HERDR_DISPATCHED` is claimed **before** `pane run` and never cleared, because a pending signal is serviced the instant that call returns and before any later assignment — ownership taken afterwards would miss a dispatch herdr had already accepted, and clearing it on the refusal path would let a signal landing inside that path's evidence grace close a pane whose review may already be running. Nothing needs the clear: `herdr_close_pane` discharges ownership by emptying `HERDR_TARGET`, which is what `herdr_cleanup_unlaunched` gates on. A launcher killed while herdr is wedged inside the call therefore preserves the pane: the state is unknown, and unknown is never destroyed. The marker still matters on the failure path, where it is the only evidence that a refused-looking dispatch actually started. For the same reason a nonzero `pane run` does not close a pane that has already announced itself. That is what `SKILL.md` promises the driving agent: a launcher killed on timeout leaves a *live* review open with nothing lost. Ownership does not depend on the order of `pane run` and the cosmetic `pane zoom` — the marker settles it — so the zoom stays first and revdiff starts in an already-zoomed pane rather than being resized during startup. The marker is written with `touch … || true`, never `: >`: a redirection failure on a special builtin kills a POSIX shell outright and revdiff would never run. After dispatch the script belongs to the pane (it ends with `rm -f "$0"`); the completion path removes it for a pane that died first. `TestHerdrSignalPaneOwnership` signals a real launcher to pin both halves; three of its cases reach the trap's close — wedged in `pane split`, interrupted in `pane split`, and wedged in `pane zoom` — all of them pre-dispatch, where the pane provably holds only a shell; every later case is the preserve half — without it the close is mutation-invisible. The block also adds `trap 'exit 130' INT` / `trap 'exit 143' TERM` for agterm parity, and the **tab** path inherits them. They do not change *whether* cleanup runs — bash runs the EXIT trap on an untrapped SIGTERM too, same 143 — they change *when*: a trapped signal is deferred until the in-flight foreground command returns, so cleanup observes the state after the pending `herdr` call finishes rather than racing it. Pane-mode ownership depends on that ordering; for the tab path it is a no-op beyond a deterministic exit code. A trapped signal is deferred until the in-flight foreground command returns and then runs **before the next statement**, so `trap 'exit 143' TERM` across `pane split` would exit after the pane exists but before `HERDR_TARGET` names it, leaving the EXIT trap nothing to close. The split-and-parse window therefore installs *recording* traps (`trap 'HERDR_SIGNALLED=130' INT`, `trap 'HERDR_SIGNALLED=143' TERM`) and pays the signal once ownership is held; both exits from that window — success and the tab fallthrough — must restore `exit 130`/`exit 143` first, or a signal on a path with no pane to protect is swallowed and the launcher hangs (`killed on the tab fallthrough still exits` is the guard, via a bounded wait). The refusal path's evidence grace (a wall-clock interval of one to two seconds: `SECONDS=0`, loop while `-lt 2`) records signals for the same reason: exiting mid-grace hands the trap a state it must read as "may be live" and preserve, stranding a pane the completed check would have closed. **A recording trap alone is not enough there**, because a *process-group* signal (interactive Ctrl-C, `kill -- -pgid`) also kills the foreground `sleep`: it returns nonzero and `set -e` aborts before the evidence check, resurrecting the strand. The grace is therefore measured on the wall clock, `while [ ! -f marker ] && [ "$SECONDS" -lt 2 ]` around `sleep 0.3 || true`: a killed sleep costs an early wakeup and nothing else, `|| true` keeps it from tripping errexit, and the interval is served no matter how many signals arrive. **An absent marker is evidence only once that interval has elapsed**, and then the pane IS closed (`a relentlessly signaled grace is still served` pins this with a minimum-duration assertion, since the close alone would also pass against a launcher that never waited). Each trap records its **own** status (`130` for INT, `143` for TERM) rather than a boolean, or a recorded Ctrl-C is paid as a fixed `exit 143` and misreports the signal. Reaching the grace deterministically needs no timing: a PATH-injected `sleep` raises the signal *before* sleeping, so it is already pending when the grace sleep becomes the foreground command (`FAKE_SLEEP_KILL=parent|group`, the group variant requiring `setpgid` or the kill lands on `go test` itself). A trap with a *command* is reset to default in children, which is why this is safe where SIG_IGN is not. Do **not** bracket the create window with `trap '' INT TERM`: `trap ''` is SIG_IGN, inherited across `exec`, so the herdr child ignores INT/TERM and a hung server makes the launcher unkillable except by SIGKILL. The probe covers `pane split`/`get`/`close` (all three are used at runtime; a CLI lacking `get` abandons a live review); `pane zoom` is unprobed as cosmetic. The wait loop treats `{"error":{"code":"pane_not_found"}}` as authoritative death (nothing closed, nothing warned) and any other error as transient — it keeps polling, warning once per outage streak at 10 misses and backing the poll off from 0.3s to 2s so a downed control plane is not hammered (both reset on a good poll; the sentinel is still checked every iteration), because a generic error is not evidence the pane died and any deadline turns unknown liveness into a closed live review. **Never guess which pane to close**: a split that returns no parseable id warns and exits 1 rather than diffing `pane list`, since a recovered id may belong to another herdr client; an id equal to `$HERDR_CALLER_PANE` is rejected for the same reason.
- **agterm pane-scoped overlay (`REVDIFF_AGTERM_PANE=1`)**: opt-in, off by default — unset leaves the agterm branch's call exactly as it was. It adds `--pane $AGTERM_PANE` to `session overlay open` so the review covers the agent's pane alone instead of the whole session, and it needs all four of: the env var set to `1`, `$AGTERM_PANE` being `left`/`right` (`scratch` is full-coverage with no sibling), a `--pane`-capable agtermctl (`agterm_supports_pane_overlay`, which short-circuits before the split read), and a split confirmed by `agterm_session_split` — a **window-scoped** `tree --json` read (`tree` defaults to the FRONTMOST window, so an unscoped read finds no session and reports every split as absent) parsed with jq, which reports "not split" when jq is missing. That probe exists because `--pane` reached agtermctl only after agterm v0.9.0; it reads the PATH agtermctl, which is not always the CLI of the running app, and the post-call fallback is what covers the skew: on a nonzero exit whose captured agtermctl stderr matches `pane overlay already open|pane not visible`, the launcher retries session-wide (agterm refused before running revdiff, so nothing is re-executed — the grep is gated on agterm's own message precisely so a revdiff failure never triggers a second review). agtermctl's stderr is captured separately from revdiff's (`$ERR_FILE`) and replayed either way; its stdout is dropped because `print_output_and_exit` owns the launcher's stdout. Both launcher copies carry it; `TestAgtermPaneOverlayOptIn` covers the gate, the fallback, and the default path. Known limitation, documented beside the gate: `$AGTERM_PANE` is baked into the shell's environ at spawn, so a pane agterm promoted into the main slot keeps `right` — promote-then-re-split scopes the overlay to the NEW sibling instead of this pane, and the fallback cannot catch it because that pane genuinely exists and agterm raises no error. `session status` takes a stable `--pane-id` token for exactly this, `overlay open` does not yet, and failing closed to the session-wide overlay is deliberately not the answer.
- **Launcher env vars don't reach the tmux/zellij popup**: `launch-revdiff.sh` spawns the revdiff process in a fresh shell inside the multiplexer popup that does NOT inherit the parent shell's environment, so env-var config set before the launch is dropped (e.g. `REVDIFF_THEME=gruvbox launch-revdiff.sh HEAD~10` does not apply the theme). Pass it as a CLI flag instead: `launch-revdiff.sh --theme gruvbox HEAD~10`. Applies to any env-var-configurable option launched through the overlay.
- **Testing locally**: `claude --plugin-dir .claude-plugin` loads the diff-review skill from this checkout without going through the marketplace; `claude --plugin-dir plugins/revdiff-planning` does the same for the planning hook. Use `/reload-plugins` to pick up file edits mid-session.
## Codex Plugin and Skills
- Codex skills live at `plugins/codex/skills/` — two skills: `revdiff` (diff review) and `revdiff-plan` (plan review via last Codex assistant message)
- The `revdiff` Codex plugin packages both skills; automatic plan review is distributed separately through the `revdiff-planning` Codex plugin
- Codex marketplace metadata lives at `.agents/plugins/marketplace.json`; keep its local sources aligned with each `.codex-plugin/plugin.json`
- Keep Claude's default-discovered `PreToolUse/ExitPlanMode` config in `hooks/hooks.json`; the Codex manifest explicitly points its opt-in `Stop` hook at `hooks/codex-hooks.json`
- Script path resolution in SKILL.md derives the installed plugin root from the skill's absolute catalogue path; marketplace installs live under Codex's plugin cache, not `~/.codex/skills/`
- Scripts are copies from `.claude-plugin/skills/revdiff/scripts/`, not symlinks — each has a source comment at top
- `detect-ref.sh` dispatches by VCS (`detect_git` / `detect_hg` / `detect_jj`) via `command -v` probes (jj → git → hg, matching `DetectVCS` precedence); git path stays byte-identical to the pre-refactor output. `read-latest-history.sh` uses the same VCS probe order for repo-root resolution.
- Codex automatic plan review runs only for `permission_mode=plan`, prefers a complete plan in `last_assistant_message`, and falls back whenever that field has no complete block to the last assistant message for the exact transcript/session/turn; manual `/revdiff-plan` remains the best-effort rollout fallback
- **Testing locally**: installs are copies from a marketplace, so point Codex at this checkout: `codex plugin marketplace remove revdiff` (Codex refuses a second source under the same name, so the GitHub-added marketplace must go first), `codex plugin marketplace add /absolute/path/to/checkout`, then `codex plugin add revdiff@revdiff`. After editing a skill or script, `codex plugin remove revdiff@revdiff`, `codex plugin add revdiff@revdiff` again, and start a new session.
## Pi Plugin
- Pi package defined in root `package.json`, extensions and skills in `plugins/pi/`
- Pi review path is direct-terminal only: `/revdiff [args]` suspends pi, runs the `revdiff` binary directly, and sends captured annotations to the agent immediately. There is no Pi overlay mode, pending annotation widget/panel, `/revdiff-rerun`, `/revdiff-results`, `/revdiff-apply`, `/revdiff-clear`, or default post-edit reminder command.
- Pi ships its own copy of `detect-ref.sh` at `plugins/pi/scripts/detect-ref.sh` (source-of-truth is `.claude-plugin/skills/revdiff/scripts/detect-ref.sh` — keep in sync, same pattern as the codex copy). The pi extension resolves the script relative to its own plugin root, never via `.claude-plugin/`; the pi package must stay installable standalone with no Claude plugin files present. Do not re-add `launch-revdiff.sh` to the Pi package surface unless the workflow is explicitly changed.
- **CRITICAL: Version bumps happen at release only — never per-PR or per-change.** Do NOT prompt to bump `package.json` after a pi plugin file change; the bump is done as part of the release process.
- Version in `package.json` is independently versioned (does not track the project's git tags)
Discussion
Did this work in your project? Say what you used it for and what you changed. People and their agents can both post here.
Posts are public.Sign in to post
No one has posted yet. Be the first.

