Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
24 commits
Select commit Hold shift + click to select a range
e0b010a
docs(review): lock M7 review-any-source design (ADR-036 + plan)
lettertwo Jul 9, 2026
a9ea894
refactor(lib): rename ChangesetSource to ChangesetSpan
lettertwo Jul 9, 2026
c21121a
feat(review): source arg with stack and uncommitted keywords
lettertwo Jul 9, 2026
654c4b5
fix(review): show nothing-to-review for empty stack resolution
lettertwo Jul 9, 2026
d8f71a7
fix(review): refresh re-resolves the launched review source
lettertwo Jul 9, 2026
f0f3704
feat(review): resolve ref and range review sources
lettertwo Jul 9, 2026
dc19db0
refactor(review): dedupe source error mapping and committed diffs
lettertwo Jul 9, 2026
d6eba5c
feat(review): resolve PR references as review sources
lettertwo Jul 9, 2026
17269a0
fix(pr): fetch PR head and base fresh for review sources
lettertwo Jul 9, 2026
46b1df8
refactor(review): reuse fork dispatch and PR error closure
lettertwo Jul 9, 2026
a1831d7
fix(review): refresh no-ops for a PR review source
lettertwo Jul 9, 2026
60ad84c
feat(completions): review source completion and delegation
lettertwo Jul 9, 2026
e2bcbbc
fix(completions): gate delegation to known responders, null stdin
lettertwo Jul 9, 2026
398b77a
perf(review): coalesce buffered nav input in the event loop
lettertwo Jul 9, 2026
c6e7f81
fix(review): land diff on last crossed file for header landings
lettertwo Jul 9, 2026
5f05abc
refactor(review): collapse duplicated nav-coalescing arms
lettertwo Jul 9, 2026
017ff45
perf(review): parallelize changeset diff acquisition
lettertwo Jul 9, 2026
1291afd
perf(review): defer file loads to an input-idle window
lettertwo Jul 9, 2026
dfa176b
fix(review): reopen cached files eagerly in defer mode
lettertwo Jul 9, 2026
423f42e
feat(review): show launch splash before changeset acquisition
lettertwo Jul 9, 2026
6f34336
fix(review): keep terminal free during PR source resolution
lettertwo Jul 9, 2026
9761ecb
fix(review): probe theme and resolve sources before terminal takeover
lettertwo Jul 9, 2026
c28148a
test(review): PTY responsiveness harness for launch and nav burst
lettertwo Jul 9, 2026
eec964f
refactor(review): share the PTY spawn helper across test suites
lettertwo Jul 9, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions CONTEXT.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,16 @@ Terms used throughout the `git-workon` codebase. Implementation details do not b

**Status filter** — a flag (`--dirty`, `--clean`, `--ahead`, `--behind`, `--gone`) that narrows a `list` or `find` result to worktrees in a specific state. Filters select **worktrees**: each check queries the working tree or branch-tracking state of a checked-out worktree. A metadata-only stack diff (`◯`) has no working tree and can never satisfy a status filter; it is excluded from any filtered result. See also: `StatusFilter`, `WorktreeDescriptor::is_dirty()`.

## Review

**Changeset** — one reviewable unit in the review TUI: a node in a stack, a single inferred commit, or the uncommitted layer. Ordered base → head when part of a stack. See also: `workon::Changeset`.

**Changeset span** — what a changeset covers: a resolved commit range (`base..head`) or the uncommitted working tree + index. _Avoid_: "changeset source" (renamed; "source" is the review-source concept below).

**Review source** — the user's answer to "review *what?*": auto-detect (no argument), the `stack` keyword, the `uncommitted` keyword, a ref, a range, or a PR reference. Exact bare keywords win over same-named refs; a qualified spelling (`refs/heads/stack`) escapes. See also: [ADR-036](docs/adr/036-review-source-grammar.md).

**Uncommitted layer** — the synthetic changeset spanning the dirty working tree + index. Appears in a review only when the review is focused where `HEAD` actually is, since uncommitted changes diff against `HEAD`.

## Prune Candidate Reasons

**BranchDeleted** — the local branch ref for the worktree no longer exists in the repository. Always a prune candidate regardless of flags.
Expand Down
97 changes: 97 additions & 0 deletions docs/adr/036-review-source-grammar.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
# 036 — Review Source: One Sniffed Positional, Shape-Aware Resolution

Status: accepted (2026-07-09, M7 design session)

## Context

Through M6.5 the review binary's `Cli` is empty: it reviews only what auto-detect finds
(the Graphite stack when one is active, else a single uncommitted changeset). M7 makes it
review *anything* — the RFC's "review any source" — which forces three intertwined
decisions: how a source is spelled on the command line, what changeset(s) each spelling
resolves to, and what happens when resolution fails. This is the binary's entire
user-facing argument surface, so it is expensive to re-shape once muscle memory forms.

Alternatives considered for the spelling: subcommands (`review pr 123`, `review range a b`)
— unambiguous but verbose and unlike git's rev-positional idiom; flags (`--pr`, `--range`)
— noisiest for a daily-driver tool, and sources are mutually exclusive so flags fight.

## Decision

**Grammar — one optional sniffed positional.** `git workon review [<source>]`. No argument
keeps auto-detect unchanged. An argument is classified by precedence:

1. **PR reference** — any form `workon`'s own default command accepts (`123` excluded;
`pr-123`, `#123`, `pr#123`, GitHub URLs), via git-workon-lib `parse_pr_reference`.
2. **Keyword** — exact bare `stack` or `uncommitted`.
3. **Range** — contains `..` or `...`.
4. **Ref** — everything else, resolved via rev-parse.

**Keywords win; qualify to escape.** Classification happens before rev-parse, so
`review stack` is deterministic regardless of repo state. A branch literally named
`stack` is reviewable via any qualified spelling (`refs/heads/stack`, `heads/stack`) —
only the exact bare word matches the keyword.

**`stack` keyword** — "give me the real stack": Graphite metadata when active, otherwise
git-inference (the lib's already-built `StackModel::Git` arm: one changeset per commit in
`upstream..HEAD`). No metadata and no upstream is a real error, never a silent fall-through
to uncommitted — an explicit ask deserves an explicit failure. This ships the M5-deferred
`StackModel::Git` wiring, scoped to the one keyword that means it.

**`uncommitted` keyword** — always the single uncommitted changeset (M2–M4 behavior),
even in a Graphite repo.

**`<ref>` — shape-aware dispatch.** Match what a person most plausibly means per shape:

- *Graphite-tracked branch* → the whole stack focused at that branch
(`assemble_changesets` already does exactly this; outline and `]c` nav come along).
- *Untracked branch* → one committed changeset, base = merge-base(upstream if set, else
repo trunk, else error) — "what this branch adds".
- *Bare commit-ish* (sha, tag, `HEAD~2`) → one changeset spanning just that commit
(`parent..ref`).

**Ranges — git-diff semantics, both dot forms.** `a..b` → base `a`, head `b` (endpoint
trees, exactly a committed span). `a...b` → base merge-base(a,b), head `b` (the PR-style
"what did b add since diverging"). An empty side defaults to `HEAD`. One committed
changeset either way; git-diff muscle memory transfers unchanged.

**PR — gh metadata + fetch, one changeset.** Reuse git-workon-lib `pr.rs` end-to-end:
`fetch_pr_metadata` (gh CLI) for base/head/title/fork detection, `fetch_branch` for the
objects — no worktree is created; review is read-only. Changeset =
`merge-base(base, head)..head` (GitHub's own three-dot PR diff), PR title carried into the
changeset. Requires gh + network, like `workon #123` today.

**Uncommitted layer only when focused on real HEAD.** The layer rides along exactly when
the thing under review is where the working tree actually is: `stack`, and `<ref>` where
ref is the current `HEAD` branch. Every other source — range, commit, PR, untracked
branch, a tracked branch you're not standing on — is committed-only. Rationale:
uncommitted changes diff against `HEAD`; the lib's unconditional insert-after-current
would attach them to a branch they don't belong to.

**Failures surface before the TUI.** Unresolvable ref, bad range endpoint, missing gh, PR
fetch failure, no-upstream: pre-TUI miette errors naming the offending source text, with a
hint where one exists. Never enter the TUI on a broken source; never fall back to
auto-detect (silently reviewing the wrong thing after a typo is the one surprise a review
tool must not have). A valid-but-empty source keeps "nothing to review" + exit 0, extended
to name the source.

**Completion — keywords + local branches + tags.** Offline git2 ref enumeration only;
after a `..`/`...` prefix, complete the right-hand ref the same way. No PR-number
completion (network in the TAB hot path). This is M6's deferred sub-delegation trigger:
git-workon's dynamic completer now shells out to `COMPLETE=<shell> git-workon-review` for
post-subcommand words.

**Rename `ChangesetSource` → `ChangesetSpan`.** Its doc comment already says "what a
Changeset spans"; the rename frees "source" for the user-facing concept every roadmap
document already uses. Safe while the M1–M6.5 tower is unmerged.

## Consequences

- The review binary gains its first real argument; the `Source` enum
(Auto | Stack | Uncommitted | Ref | Range | Pr) becomes the seam between CLI parse and
changeset resolution.
- Stack assembly for a non-HEAD tracked branch must suppress the uncommitted layer — a
lib-side knob or an acquire-side filter (execution detail, see the M7 plan).
- `review <trunk>` resolves through the untracked-branch arm via its upstream (unpushed
commits) — an acceptable edge, not a special case.
- Git-inference changesets become reachable from the binary for the first time; its
per-commit semantics get real exposure.
152 changes: 152 additions & 0 deletions docs/plans/review-any-source.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,152 @@
# Plan — Review Any Source (M7)

Design locked 2026-07-09. Decisions live in **[ADR-036](../adr/036-review-source-grammar.md)**
(source grammar, per-shape resolution, error posture, completion scope, the
`ChangesetSpan` rename). This doc is the *execution* plan: what lands, in what order, how
each unit is verified. Read the ADR before implementing — this plan does not restate its
rationale. Glossary terms ("Review source", "Changeset span", "Uncommitted layer") are in
[CONTEXT.md](../../CONTEXT.md).

Goal: `git workon review [<source>]` reviews *anything* — stack, uncommitted, ref, range,
PR — not just the auto-detected state. Read-only for committed sources (M5 semantics);
no-arg auto-detect behavior is byte-identical to today.

## Scope (five tracks)

1. **`ChangesetSpan` rename** — `workon::ChangesetSource` → `workon::ChangesetSpan`
(field `source` → `span`), mechanical across lib + review crates.
2. **Source classifier + keywords** — `Source` enum in the review crate; the binary's
`Cli` gains one optional positional (`[SOURCE]`); exact-bare-keyword precedence;
`stack` (Graphite → Git-inference → error) and `uncommitted` resolution; the
uncommitted-layer suppression seam in the lib.
3. **Rev sources** — `<ref>` shape-aware dispatch (tracked branch → focused stack;
untracked branch → merge-base changeset; commit-ish → single commit) and
`a..b` / `a...b` ranges (git-diff semantics, empty side = `HEAD`).
4. **PR source** — `parse_pr_reference` forms at top precedence; `fetch_pr_metadata` +
`fetch_branch` (fork-aware) → one committed changeset `merge-base(base,head)..head`,
PR title carried through. No worktree is created.
5. **Completion** — review-binary completer offers keywords + local branches + tags
(and the RHS after `..`/`...`); git-workon's completer sub-delegates post-subcommand
words to `COMPLETE=<shell> git-workon-review` (the M6-deferred shell-out).

## Changeset partition (Graphite stack)

Linear stack — each unit extends the classifier the previous one introduced. Base:
the current M3–M6.5 tower tip (`uc-roadmap-reprioritize`/`uc-pty-smoke`), or `main` once
the tower lands. Each unit is land-alone (green + valuable by itself) and
standalone-review (~≤400 non-mechanical lines).

```
<tower tip>
└─ m7-span-rename CS1 ── ChangesetSource → ChangesetSpan (mechanical)
└─ m7-source-keywords CS2 ── Source enum, positional arg, stack/uncommitted keywords
└─ m7-source-revs CS3 ── <ref> dispatch + ranges (grammar complete)
└─ m7-source-pr CS4 ── PR references via pr.rs
└─ m7-complete CS5 ── source completion + git-workon sub-delegation
```

Interim behavior is honest at every cut: before CS3, a ref/range argument fails the
keyword match and errors pre-TUI as an unresolvable source; before CS4, `pr-123` falls
through to the ref arm and errors the same way (named, hinted).

## Per-changeset detail

### CS1 — `m7-span-rename` (refactor, lib + review)

- `refactor(lib): rename ChangesetSource to ChangesetSpan`. Type, `Changeset.source`
field → `Changeset.span`, doc comments, all use sites in `acquire.rs`/`app.rs`/tests.
- Purely mechanical; no behavior change. Verify: full workspace green, `grep -rn
ChangesetSource` returns nothing.

### CS2 — `m7-source-keywords` (review crate + one lib seam)

- New `source.rs` in the review lib: `Source` enum
(`Auto | Stack | Uncommitted | Ref(String) | Range{..} | Pr(PullRequest)`) with
`Source::classify(&str)` implementing the ADR precedence. In CS2 the classifier ships
with keyword + fallback-to-`Ref` arms only; `Ref` resolution errors as unresolvable
(real resolution is CS3). Classification is pure → unit-test exhaustively (keyword
exactness: `Stack` ≠ `stack` keyword? No — exact bare match is case-sensitive `stack`;
`refs/heads/stack` classifies as `Ref`).
- `Cli` gains `Option<String>` positional `[SOURCE]`; `main.rs` routes
`None` → `Source::Auto` → existing `resolve_changesets` (unchanged path).
- `stack`: Graphite → `assemble_changesets(.., Graphite)`; else Git-inference
(`StackModel::Git` — first binary wiring); `NoUpstream` surfaces pre-TUI with a hint
(set an upstream, or `review uncommitted`).
- `uncommitted`: the single synthetic uncommitted changeset (extract today's
`resolve_changesets` fallback arm for reuse).
- **Lib seam**: `assemble_changesets` must be able to omit the uncommitted layer
(ADR-036: layer only when focused on real HEAD). Prefer an explicit parameter over a
post-filter — a post-filter must also repair the `current` flag, which is subtle.
CS2 introduces the seam (keywords always run with the layer *on*, since `stack`
reviews HEAD's stack); CS3 is the first caller that turns it off.
- New error variants in review `error.rs` per ADR-008 — **load `/docs errors` first**.
- Verify: fixture tests for both keyword resolutions in Graphite and plain-git repos
(sqlite + legacy metadata modes), error cases asserted with `NO_COLOR=1`.

### CS3 — `m7-source-revs` (review crate + acquire)

- `Ref` resolution, dispatched on shape (ADR-036): Graphite-tracked branch →
`assemble_changesets` focused there, uncommitted layer ON iff the ref is the actual
`HEAD` branch (first user of the CS2 lib seam); untracked branch → one committed
changeset, base = merge-base(upstream, else trunk, else error); commit-ish →
`parent..ref` (root commit: empty-tree base).
- `Range` resolution: split on `...` first, then `..`; empty side → `HEAD`; rev-parse
each endpoint; `...` → merge-base base. One committed changeset named after the
source text as typed.
- Empty-but-valid results extend the existing "nothing to review" to name the source.
- Verify: fixture matrix — tracked/untracked/commit/tag shapes; both dot forms;
`review <current-branch>` == auto-detect output (layer present); reviewing a non-HEAD
tracked branch on a dirty tree asserts NO uncommitted layer and correct `current`.

### CS4 — `m7-source-pr` (review crate, reuses lib `pr.rs`)

- Classifier gains the PR arm at top precedence (`parse_pr_reference`; also accept
`pr-123` if the lib parser doesn't already — check first, extend the *lib parser*
if not, with its existing tests as the pattern).
- Resolution: `check_gh_available` → `fetch_pr_metadata` → `detect_pr_remote` /
`setup_fork_remote` → `fetch_branch` → merge-base(base, head) → one committed
changeset, `title` from PR metadata. Every failure pre-TUI, named, hinted.
- Verify: classification unit tests offline; resolution wiring behind the smallest
testable seam (metadata → changeset mapping fixture-tested with a local "remote";
the gh-network path itself is exercised manually — record the manual check in the
changeset description).

### CS5 — `m7-complete` (review crate + git-workon completer)

- Review-binary completer: keywords + local branch names + tag names via git2 ref
enumeration; when the current word contains `..`/`...`, complete the RHS ref the
same way. Offline only; no PR numbers.
- git-workon side: the dynamic completer's external-subcommand arm shells out
`COMPLETE=<shell> git-workon-review -- <partial>` for post-subcommand words
(M6 CS3 left this seam documented; remember `_CLAP_COMPLETE_INDEX`).
- Verify: completion integration tests per M6's pattern (`COMPLETE=` env protocol),
asserting keyword + ref candidates and the delegation path.

## Traps / notes for the implementer

- **Load `/docs testing` before any tests; `/docs errors` before error variants.**
- `FORCE_COLOR=3` is set in this environment — output-asserting tests pin `NO_COLOR=1`.
- Verify TUI behavior by instrumenting, never by grepping ratatui frames.
- The Git-inference arm (`assemble_git`) is lib-complete and lib-tested; CS2 only wires
it. Don't reimplement.
- `resolve_changesets`'s doc comment explains why auto-detect must NOT route plain-git
repos to `StackModel::Git` — that reasoning stays true; only the explicit `stack`
keyword takes the Git arm.
- Working-tree leftovers are user WIP — never stage `.claude/settings.json`,
`.claude/hooks/post-edit-rust.sh`, `docs/diagrams/agent-integration.md`,
`docs/recipes/agent-integration.md`. `git add` specific files, never `-A`/`-u`.
- Commits: Conventional, single line ≤72 chars, no body/footer.

## Verification gates (green before any changeset is called done)

```bash
NO_COLOR=1 cargo test --workspace
cargo clippy --workspace --all-targets --all-features -- -D warnings
cargo fmt --all -- --check
cargo run -p git-workon-review -- <source> # manual: each source shape renders
```

## Acceptance (RFC M7)

`git workon review <ref>` / `<a..b>` / `pr-123` renders the right changeset(s);
`git workon review <TAB>` completes sources.
2 changes: 1 addition & 1 deletion docs/rfc/workon-review.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ The remaining roadmap is resequenced around the tool being **the author's own ev

- **Prerequisite — Land M3–M6.5** (process, parallel to features; not a numbered milestone). QA the unmerged M3→M6.5 tower → merge to `main` → reliable install (a local build on PATH is enough to dogfood; the [ADR-033](../adr/033-review-crate-workspace-placement.md) release/homebrew "M3 flip" is a deferrable sub-decision). Gates real daily use regardless of features. QA checklist in memory `review-tui-priority-everyday-use` (`theme=auto` responsiveness, `theme=light` canvas, committed-changeset nav).

- **M7 — review any source.** A source selector — `stack | uncommitted | <ref> | <ref..range> | pr-####` — so the tool reviews *anything*, not just the auto-detected stack/uncommitted state. **Ordered first:** it is the tool's core *read* identity, read-only (low-risk), independent of the write verbs, and the M1/M5 lib already provides `assemble_changesets` + the `diff_changeset` router — mostly source-arg parse → resolve to changeset(s) → existing pipeline. PR support reuses git-workon-lib's `pr.rs`. Also **completes M6's deferred completion sub-delegation** (its trigger was exactly this arg gaining completion-worthy values). Acceptance: `git workon review <ref>` / `<a..b>` / `pr-123` renders the right changeset(s); `git workon review <TAB>` completes sources.
- **M7 — review any source.** A source selector — `stack | uncommitted | <ref> | <ref..range> | pr-####` — so the tool reviews *anything*, not just the auto-detected stack/uncommitted state. **Ordered first:** it is the tool's core *read* identity, read-only (low-risk), independent of the write verbs, and the M1/M5 lib already provides `assemble_changesets` + the `diff_changeset` router — mostly source-arg parse → resolve to changeset(s) → existing pipeline. PR support reuses git-workon-lib's `pr.rs`. Also **completes M6's deferred completion sub-delegation** (its trigger was exactly this arg gaining completion-worthy values). Acceptance: `git workon review <ref>` / `<a..b>` / `pr-123` renders the right changeset(s); `git workon review <TAB>` completes sources. Design locked 2026-07-09 — [ADR-036](../adr/036-review-source-grammar.md) (one sniffed positional, keyword-over-ref precedence, shape-aware `<ref>` dispatch, git-diff dot semantics, gh-backed PR resolution, uncommitted-layer-on-HEAD-only, fail-before-TUI, offline completion, `ChangesetSource`→`ChangesetSpan` rename); execution plan `docs/plans/review-any-source.md` (five changesets `m7-span-rename → m7-source-keywords → m7-source-revs → m7-source-pr → m7-complete`).

- **M8 — commit operations.** Commit the staged changes without leaving the TUI — message editor (inline vs `$EDITOR`), Conventional-Commit-aware (enforced by `git-hooks/commit-msg`); **amend** the current commit; **fixup/absorb** staged changes into an earlier changeset in the stack. Closes the review→stage→**commit** loop — the acute daily-driver gap. Acceptance: stage in the TUI, commit/amend/fixup, verified against real git.

Expand Down
Loading