Make the dashboard a review surface, and let it admit it is old #77

Merged

a73x merged 1 commit from worktree-agent-a5cb4ce56d36b2fb7 into main

2026-08-12 17:58:16 +00:00

a73x commented

2026-08-12 17:53:44 +00:00

(Migrated from git.a73x.sh)

Implements 094b055b (the TUI can only open an issue) and 59ab40e8 (the
TUI never refreshes), to the decisions recorded in their comments.

The review loop (`094b055b`)

Bound on the patch detail pane, where the thing being responded to is
already on screen. None of these keys were bound there before, so nothing
in Details mode moved:

c comment on the diff line under the cursor; a thread comment anywhere else
R review with a verdict (a/r/c prompt, then the editor)
x mark the inline comment under the cursor answered, or withdraw it
a show the change that answered it (--answers)
o check the patch out
j/k, PageUp/Down now move a cursor; the view follows only when it must

Deliberately absent, as the issue settled: merge, revise, label, assign,
close.

The pane needed a cursor for "the selected line" and "the selected
comment" to mean anything, so j/k select rather than scroll. Rows are
built when the view is built, not during a draw, so the key handler acts
on the rows the reader was looking at.

Text arrives through $EDITOR, seeded with the anchored hunk as # lines.
Those lines are stripped on return and an empty body aborts with a
message. Note the divergence this creates: editor::compose deliberately
does not strip #, because a CLI body is markdown where a leading # is
a heading. The TUI must strip, because it seeds the buffer with context.
So a markdown heading cannot be written from the dashboard; it still can
from the CLI. That is git commit's own trade and it is documented at
the call site, but it is a real difference between the two surfaces.

n lost its hand-rolled body buffer entirely — the title stays a
one-line prompt, the body goes to the editor. The issue asked for the
editor treatment and for no second, worse text input; keeping both would
have been keeping the thing being complained about.

Inline anchoring goes through patch::comment(.., file, line, revision, ..),
so it inherits fe555587's validation against the revision the comment
anchors to. The anchors are not recomputed: format_diff now builds
DiffRows that carry them and renders those, and --line-numbers prints
the same rows with a gutter. patch::diff_rows_for hands them to the
TUI. Interdiff's diff construction was split out of interdiff for the
same reason. CLI diff output is byte-identical — interdiff_test,
inline_anchor_test and cli_test were run against the refactor alone
before anything else changed.

x surfaces the id patch show prints in brackets, at the same eight
characters, because a reviewer reads it off one surface and types it into
the other.

Staleness (`59ab40e8`)

A header carries as of HH:MM unconditionally, and a banner counting new
events when the collab tips have moved. Detection is a tip comparison on
a two-second timer — references_glob("refs/collab/**"), no fold — and
only the refs that actually moved get walked, to turn "something changed"
into an event count. refs/collab/local/ is excluded, so the seen/
marker patch show moves in another terminal is not mistaken for someone
speaking. Nothing reloads under the reader; r still does that.

App::reload re-baselines the snapshot, and every write path calls it,
which is what keeps the dashboard's own comment from being announced back
as news.

Also fixed

o ran git checkout on PatchState.branch — provenance-only since
659f0350, and a dead name on agent-produced patches (850f3b8e). It now
runs patch::checkout, the same code the CLI runs, against the latest
revision's commit. Tested with the authoring branch deleted, which is the
state every agent patch is in.

Testing

New tests/tui_review_test.rs, 12 tests through a real pty via a new
run_dashboard_driven harness that drives stdin from a callback, so a
test can interleave keystrokes with an external process writing to the
repo. $EDITOR is a script; the killed-editor case is a script that
SIGTERMs itself. Assertions are against the collab DAG afterwards, not
the screen, except where the screen is the feature.

Covered: the banner appears on an external write and does not appear
after the TUI's own; a comment-only body aborts and records nothing and
says so; a killed editor records nothing, reports "Editor exited" and
leaves the dashboard usable; no $EDITOR names the alternatives instead of
hanging; c/R/x/a/o/n each land what they claim. Plus unit tests for row
targets, cursor/scroll behaviour, seed stripping, and the tip snapshot's
local-ref exclusion.

Full suite green, cargo clippy --all-targets silent.

Not verified

Not run against a human terminal — every check is the pty harness or
TestBackend. The wrapping was turned off on the detail pane (a wrapped
line occupies more rows than it has, which would put the cursor highlight
on a different line than the one c comments on), so long diff lines now
clip instead of wrapping. That is the right trade for correctness but it
is a visible change nobody has looked at.

One test of mine ran git-collab issue open in the wrong working
directory before I caught it, and opened issue f2e57779 "landed while you
were reading" in this repository. I could not delete the ref (permission
denied) so it is still there and should be dropped.

Migrated from git-collab patche 07a78a6019b16bef7502087c154fcd5a8484cde8.

Original author: a73x dev@a73x.sh.

Fixes #74

Revision 1: 15bc9a3c2f8c1622b6eb5157c6176452ba7569bb; base 602505d27ec0faa5d3ca540facd54d6e5eeab4ef

Implements 094b055b (the TUI can only open an issue) and 59ab40e8 (the TUI never refreshes), to the decisions recorded in their comments. ## The review loop (094b055b) Bound on the patch detail pane, where the thing being responded to is already on screen. None of these keys were bound there before, so nothing in Details mode moved: c comment on the diff line under the cursor; a thread comment anywhere else R review with a verdict (a/r/c prompt, then the editor) x mark the inline comment under the cursor answered, or withdraw it a show the change that answered it (--answers) o check the patch out j/k, PageUp/Down now move a cursor; the view follows only when it must Deliberately absent, as the issue settled: merge, revise, label, assign, close. The pane needed a cursor for "the selected line" and "the selected comment" to mean anything, so j/k select rather than scroll. Rows are built when the view is built, not during a draw, so the key handler acts on the rows the reader was looking at. Text arrives through $EDITOR, seeded with the anchored hunk as `#` lines. Those lines are stripped on return and an empty body aborts with a message. Note the divergence this creates: `editor::compose` deliberately does not strip `#`, because a CLI body is markdown where a leading `#` is a heading. The TUI must strip, because it seeds the buffer with context. So a markdown heading cannot be written from the dashboard; it still can from the CLI. That is `git commit`'s own trade and it is documented at the call site, but it is a real difference between the two surfaces. `n` lost its hand-rolled body buffer entirely — the title stays a one-line prompt, the body goes to the editor. The issue asked for the editor treatment and for no second, worse text input; keeping both would have been keeping the thing being complained about. Inline anchoring goes through `patch::comment(.., file, line, revision, ..)`, so it inherits fe555587's validation against the revision the comment anchors to. The anchors are not recomputed: `format_diff` now builds `DiffRow`s that carry them and renders those, and `--line-numbers` prints the same rows with a gutter. `patch::diff_rows_for` hands them to the TUI. Interdiff's diff construction was split out of `interdiff` for the same reason. CLI diff output is byte-identical — interdiff_test, inline_anchor_test and cli_test were run against the refactor alone before anything else changed. `x` surfaces the id `patch show` prints in brackets, at the same eight characters, because a reviewer reads it off one surface and types it into the other. ## Staleness (59ab40e8) A header carries `as of HH:MM` unconditionally, and a banner counting new events when the collab tips have moved. Detection is a tip comparison on a two-second timer — `references_glob("refs/collab/**")`, no fold — and only the refs that actually moved get walked, to turn "something changed" into an event count. `refs/collab/local/` is excluded, so the `seen/` marker `patch show` moves in another terminal is not mistaken for someone speaking. Nothing reloads under the reader; `r` still does that. `App::reload` re-baselines the snapshot, and every write path calls it, which is what keeps the dashboard's own comment from being announced back as news. ## Also fixed `o` ran `git checkout` on `PatchState.branch` — provenance-only since 659f0350, and a dead name on agent-produced patches (850f3b8e). It now runs `patch::checkout`, the same code the CLI runs, against the latest revision's commit. Tested with the authoring branch deleted, which is the state every agent patch is in. ## Testing New `tests/tui_review_test.rs`, 12 tests through a real pty via a new `run_dashboard_driven` harness that drives stdin from a callback, so a test can interleave keystrokes with an external process writing to the repo. $EDITOR is a script; the killed-editor case is a script that SIGTERMs itself. Assertions are against the collab DAG afterwards, not the screen, except where the screen is the feature. Covered: the banner appears on an external write and does not appear after the TUI's own; a comment-only body aborts and records nothing and says so; a killed editor records nothing, reports "Editor exited" and leaves the dashboard usable; no $EDITOR names the alternatives instead of hanging; c/R/x/a/o/n each land what they claim. Plus unit tests for row targets, cursor/scroll behaviour, seed stripping, and the tip snapshot's local-ref exclusion. Full suite green, `cargo clippy --all-targets` silent. ## Not verified Not run against a human terminal — every check is the pty harness or TestBackend. The wrapping was turned off on the detail pane (a wrapped line occupies more rows than it has, which would put the cursor highlight on a different line than the one `c` comments on), so long diff lines now clip instead of wrapping. That is the right trade for correctness but it is a visible change nobody has looked at. One test of mine ran `git-collab issue open` in the wrong working directory before I caught it, and opened issue f2e57779 "landed while you were reading" in this repository. I could not delete the ref (permission denied) so it is still there and should be dropped. --- Migrated from git-collab patche `07a78a6019b16bef7502087c154fcd5a8484cde8`. Original author: a73x <dev@a73x.sh>. Fixes #74 Revision 1: `15bc9a3c2f8c1622b6eb5157c6176452ba7569bb`; base `602505d27ec0faa5d3ca540facd54d6e5eeab4ef`

a73x commented

2026-08-12 17:54:05 +00:00

(Migrated from git.a73x.sh)

Correction to the last paragraph of the description: the stray issue f2e57779, opened by a test of mine that ran in the wrong working directory, has now been removed with issue delete. Nothing is left to clean up. The test was fixed to target the temp repo before anything else was written.

Correction to the last paragraph of the description: the stray issue f2e57779, opened by a test of mine that ran in the wrong working directory, has now been removed with `issue delete`. Nothing is left to clean up. The test was fixed to target the temp repo before anything else was written.