Skip to content

feat(tui): add a recaps tab drawn from three commands the config names - #16

Merged
datapointchris merged 2 commits into
mainfrom
prs-fleet-tab
Nov 8, 2026
Merged

datapointchris merged 2 commits into
mainfrom
prs-fleet-tab

Conversation

@datapointchris

@datapointchris datapointchris commented Nov 8, 2026 •

Copy link
Copy Markdown
Owner

A person who keeps a markdown recap of each day's or week's merged work had to leave the board to read one, and to write one for a window not yet covered. This adds a third tab, recaps, that lists the windows written, shows the cursor's recap beside the list, and writes one on w after asking.

prs knows nothing of the program behind the recaps. The config's recaps block names three commands as argv: list prints one window a line as <kind> <label>, show prints a window's recap as markdown, and write writes one, which is a model call. prs reads nothing they print but the list's two words.

What to look at

  1. internal/board/recaps_tab.go: two sources. list runs the list command, marks when that run began as refreshFrom, and rereads the recap the screen shows. recaps reads every recap read before refreshFrom, the cursor's first, four at a time. Check readRecap: a read that began before a list run landed leaves its recap due, and forces recaps again.
  2. askWrite and answeredWindow: y writes the cursor's window, enter alone or n writes nothing, and two typed words write that window, which may be one the list does not hold.
  3. internal/pullrequests/pullrequest.go: RunDetached runs the write in a session of its own, with stdin closed and its output in a file, and no timeout. It fails with the file's last line.
  4. cmd/prs/tui.go: tuiTab.absent and drawnTabs. A config naming no list, or a machine missing a program in needs, draws no tab, and --view recaps says why. prs config show reads the same absent.
  5. internal/config/config.go: placeholderSet[F] fills and checks the window actions' placeholders and the recaps' with one type.

How it was verified

$ go test ./cmd/prs -run Recaps -count=1 -v
--- PASS: TestTheRecapsTabIsDrawnOnlyWhereTheConfigNamesItsCommandsAndItsSnapshotHoldsItsSchema (3.12s)
--- PASS: TestTheRecapsTabShowsEachWindowsRecapAndWritesOneAfterAskingThenListsAgain (6.21s)
  • The interactive test drives the built binary with tuidrive on a stand-in recap program. It lists two windows and shows each recap as j moves. Enter alone on w's question says wrote no recap. Typing day 2026-11-07 writes that window, then RECAPS · 3 and the new recap appear.
  • The same test answers y for a second write, presses q while the footer names it, and waits for the stand-in to record the write finished. The write's log under $XDG_CACHE_HOME/prs/recap-writes holds its output.
  • With Setsid: false in RunDetached, that test fails: the write dies with prs's terminal, and the stand-in records one finished write rather than two.
  • With the r.list.Force() in Receive commented out, the test fails: timed out after 10.001s waiting for text "RECAPS · 3".
  • The second test starts the board on a config with no recaps and finds no tab, and has --view recaps --json exit 1. It then validates --view recaps --json against cmd/prs/recaps.schema.json with only the first recap read.
  • prs tui --view recaps --once and --json were run against a real recap program with five windows. The frame listed all five and drew the first window's recap in the side panel.

What changes

  • prs tui draws a recaps tab after summary wherever the config names recaps.list and PATH holds every program the commands need. Elsewhere nothing changes.
  • w on that tab asks write the recap of <kind> <label> with a model call? [y/N], or type another window's kind and label. The write runs detached, named on the footer, and the list runs again when it ends.
  • Quitting prs while a write runs leaves it to finish. Its output goes to $XDG_CACHE_HOME/prs/recap-writes/<kind>-<label>.log, and a failed write names that file.
  • The list runs when the tab first shows, on a timer while it shows, on R, and after a write. Each run reads every recap again.
  • prs config show prints whether each optional tab is drawn, then the three commands. prs config path --help documents the block.
  • An unknown placeholder in show or write is refused at load. list is not scanned, so an awk program's braces through sh -c stay legal.

Decisions, and what they rejected

  • One key, w, writes either the cursor's window or one typed. A recap the list does not hold yet can only be named by typing it, because prs knows no window the list did not print. Rejected: a second key for a typed window, which splits one act across two keys that ask nearly the same question.
  • Enter alone writes nothing. A reflex enter would spend a model call. The board's merge keeps enter meaning yes, because a reflex merge lands work that was landing anyway.
  • The write runs detached with no bound. It waits on a model call and on whatever admits it, which can take longer than any bound prs could pick. Rejected: a 30-minute bound that killed the write's process group, which could stop a write half done and still leave the footer unsure whether it wrote.
  • needs decides whether the tab draws, as it decides a window action's key. A list run through sh -c hides the programs behind it, so the argv's first word is not enough. Rejected: drawing the tab with a failing list, which shows an empty list as if no recap were written.
  • Every listed recap is read in the background, so moving the cursor shows a recap at once and / matches recap text. A printed tab reads only the first, since nothing waits for the rest.
  • Smallest change that would work — reading only the cursor's recap on demand. It leaves / blind to recap text and shows Reading. on each row the cursor first reaches.

What this does not do

  • It does not move the cursor onto a window a write added. The footer names it, and the row joins the list in its place.
  • It does not serialize writes across processes. A second write of one window from this tab is refused while the first runs. Writing atomically, and refusing a window another process is writing, are the write program's, as the README says.

The review

Round 1: #16 (comment)

  • failure-paths.1 fixed. A write ending while a show read was under way no longer loses the newer recap. Each list run marks when it began, and a read that began before the mark stays due and is read again.
  • code-architecture.1 fixed. One generic placeholderSet[F] fills and checks both the window actions' placeholders and the recaps', in place of two maps and two checks.
  • code-architecture.2 fixed. prs config show loops tuiTabs and prints each absent verdict, so a later optional tab needs no edit there.
  • system-design.1 fixed. The write's life is no longer tied to prs, and nothing kills it half done. The README says atomic writes and refusing a concurrent writer are the write program's. Locking across prs processes is left out, because prs knows nothing of where the program writes.
  • stale-prose.1 fixed. The message it quoted is gone with the 30-minute bound.
  • stale-prose.2 fixed. prs tui --help names the exit 1 on a list line that is not a window. The schema says every field of a window is always present but fault.
  • stale-prose.3 fixed. The Recaps doc, prs tui --help and the schema say the list command's order, with no newest first. OpenRecaps names a board that reads detail rather than the board's reason. The README and prs config path --help say the tab draws the list's order too, so no sentence asks the program for an order prs does not check.
  • stale-prose.4 fixed for the interval: the README says a timer and points at --json's every. Rejected for the placeholders: a config writer needs {kind} and {label} on the page, as the README names {number}, {repo} and {slug} for window actions.
  • useless-test.1 fixed. The --view recaps check holds only the exit code.
  • contracts.1 fixed. R, the timer and a write's end each run the list, and every run reads every recap again, as R means in every other tab.
  • fleet-patterns.1 rejected. Exit 4 is one CLI family's contract, and prs's tracked files name none of the programs a config may run. gh exits 4 for an auth failure, so reading 4 as no answer would mislabel that. A failed write now says its log's last line, which carries the program's own words for what happened.
  • G1.1 fixed. Enter alone writes nothing, so w then enter from the Summary tab's habit spends no model call.
  • G1.2 fixed. Quitting leaves the write to finish, with its output in a file rather than a closed pipe. The interactive test quits mid-write and sees it finish.
  • synthesis.1 fixed — same as G1.2 and system-design.1.
  • synthesis.2 fixed — same as stale-prose.3 and stale-prose.4.

Session 7e57e57e-0638-4af5-be9f-487326ef59f6

A recap is a markdown account of what landed over a day or a week, which
a program of the user's own writes and keeps. prs knows nothing of that
program, so `recaps` in the config names its three commands as argv:
list prints the windows written, one a line as a kind and a label, show
prints one window's recap, and write writes one with a model call. prs
reads nothing they print but the list's two words.

The tab lists the windows newest first, and the side panel shows the
cursor's recap, which enter opens on the Reader. Every listed recap is
read in the background once the list lands, so / matches their text.
w asks before writing, naming the model call: enter writes the cursor's
window, and a kind and a label typed write a window the list does not
hold yet. The write runs in the background under RunWithin's 30-minute
bound, and the list runs again once it ends, so the new window joins it.

A config naming no list, or a machine missing a program `needs` names,
draws no recaps tab, so one config serves machines with and without the
program. --view recaps --json prints each window and the first's recap
in the shape cmd/prs/recaps.schema.json gives.

Agent-Name: claude-code-unattended
Agent-Session: 7e57e57e-0638-4af5-be9f-487326ef59f6
Agent-Run: 01a1bc01-a281-7515-9483-0ecb8a28f7c5
Worklog: 1333.1

@datapointchris datapointchris left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — failure-paths

Used: Built the PR head, pointed XDG and HOME inside my directory, and ran prs tui --view recaps --json with a list that works and a show that prints and exits 3. Traced the write, list and show paths in internal/board/recaps_tab.go by reading.
Standards: I did not run fleet standards search, so I cite no rule. I proposed none.
Findings: 1 correctness · 0 design · 0 prose

Correctness

failure-paths.1 internal/board/recaps_tab.go:187 · internal/board/recaps_tab.go:327

Reach: A show read of window W is under way when a write of W ends. A read starts on a cursor move onto W or on the 10-minute tick, and show takes a moment. I traced this by reading the code and did not force the race in a run.

A write that ends during a show read leaves the old recap on screen, and nothing says so.

Receive sets stale = true on the written window's record. It then forces the list, and readList calls readRecap(..., always=true).

readRecap returns at once when record.reading is true. The older read then ends and runs record.stale = false in the success branch.

That clears the mark the write set. The record holds the pre-write markdown with held true and no fault. due() is false until the next 10-minute tick.

The row shows the old recap as current. The --json snapshot shows the same text with no fault.

Fix: have the read keep the stale mark when a write ended after the read began. Give each record a write generation that Receive bumps. readRecap copies the generation before it runs show. It clears stale only if the generation is unchanged when show returns. Otherwise it keeps stale true and reads again.

Findings as JSON
{
  "used": "Built the PR head, pointed XDG and HOME inside my directory, and ran `prs tui --view recaps --json` with a list that works and a show that prints and exits 3. Traced the write, list and show paths in internal/board/recaps_tab.go by reading.",
  "standards": "I did not run `fleet standards search`, so I cite no rule. I proposed none.",
  "findings": [
    {
      "number": "failure-paths.1",
      "section": "correctness",
      "sites": [
        "internal/board/recaps_tab.go:187",
        "internal/board/recaps_tab.go:327"
      ],
      "reach": "A show read of window W is under way when a write of W ends. A read starts on a cursor move onto W or on the 10-minute tick, and show takes a moment. I traced this by reading the code and did not force the race in a run.",
      "body": "**A write that ends during a show read leaves the old recap on screen, and nothing says so.**\n\n`Receive` sets `stale = true` on the written window's record. It then forces the list, and `readList` calls `readRecap(..., always=true)`.\n\n`readRecap` returns at once when `record.reading` is true. The older read then ends and runs `record.stale = false` in the success branch.\n\nThat clears the mark the write set. The record holds the pre-write markdown with `held` true and no `fault`. `due()` is false until the next 10-minute tick.\n\nThe row shows the old recap as current. The `--json` snapshot shows the same text with no fault.\n\nFix: have the read keep the stale mark when a write ended after the read began. Give each record a write generation that `Receive` bumps. `readRecap` copies the generation before it runs show. It clears `stale` only if the generation is unchanged when show returns. Otherwise it keeps `stale` true and reads again."
    }
  ]
}

This review does not gate merge.

@datapointchris datapointchris left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — code-architecture

Used: I cloned the repo and checked out PR 16 at dfedd4b. I read the whole diff and the head versions of internal/board/recaps_tab.go, internal/config/config.go, cmd/prs/tui.go and cmd/prs/config.go. I traced every caller of tuiTabs, absent, Missing, MissingPrograms, pullrequests.Run and the placeholder helpers with rg. I ran nothing: no build or test.
Standards: I cited configuration.md § "A templated config has one substitution boundary, and the rendered value is a distinct type" and cli-design.md § "One fact gets one answer on every command that reaches it". I found both through fleet standards search.
Findings: 0 correctness · 2 design · 0 prose

Design and architecture

code-architecture.1 internal/config/config.go:103 · internal/config/config.go:109 · internal/config/config.go:112 · internal/config/config.go:136 · internal/config/config.go:199 · internal/config/config.go:209 · internal/config/config.go:212 · internal/config/config.go:426

Reach: This PR is the second templated command family. It copied the first family's machinery rather than parameterizing it. The next field that takes placeholders, such as a recaps open command or a window action that names a recap's window, writes a third copy. A change to how placeholders are matched or filled, such as escaping braces or refusing an empty value, has to be made in step at two sites today.

The placeholder machinery now exists twice in config. Window actions have placeholders, Placeholders(), Fill and a validation loop in WindowAction.check. The recaps block adds recapPlaceholders, RecapPlaceholders(), FillRecap and a second validation loop in Recaps.check. Each pair matches line for line, with only the input type changed (Fields against kind, label). The two sites must agree on these facts:

  • the shared placeholder regex
  • the order in which a strings.NewReplacer is built from a map
  • the error text "%s names %s, and only %s are filled in"
  • the help text that joins the sorted names

configuration.md § "A templated config has one substitution boundary" asks for one boundary.

Fix: add one generic type to config, such as type template[F any] map[string]func(F) string, with methods Names(), Fill(s string, f F) and Check(field, s string) error. Declare placeholders and recapPlaceholders as two values of that type. WindowAction.check, Recaps.check, Fill and FillRecap then become one-line calls.

code-architecture.2 cmd/prs/config.go:149 · cmd/prs/config.go:151 · cmd/prs/tui.go:57 · cmd/prs/tui.go:61 · cmd/prs/tui.go:87 · cmd/prs/tui.go:279

Reach: The PR adds absent to tuiTab as a general mechanism. Its comment reads "A nil absent is a tab every config draws", and drawnTabs and the --view refusal both read tab.absent. The next optional tab gets those two answers for free. It still needs a hand-written block in prs config show, or config show stays silent about whether that tab is drawn while prs tui hides it.

Three sites decide whether a tab is drawn, and one of them skips the tab list. Two of them, drawnTabs (tui.go:87) and the --view refusal (tui.go:279), go through tuiTab.absent. The third is prs config show (cmd/prs/config.go:149-151). It calls recapsAbsent(cfg) by name, behind its own recaps.Named() check, and prints a line labelled recaps by hand. So the answer to "is this tab drawn here?" has a second owner. That owner knows of one tab only. cli-design.md § "One fact gets one answer on every command that reaches it" covers this.

Fix: have config show loop over tuiTabs. For each tab whose absent is non-nil, it prints drawn or not drawn, because <why>. The printing of the recaps commands can stay where it is, keyed on cfg.Recaps.Named(). The drawn or not-drawn verdict then comes from the one list that --view and NewTabs already read.

Findings as JSON
{
  "used": "I cloned the repo and checked out PR 16 at dfedd4b. I read the whole diff and the head versions of `internal/board/recaps_tab.go`, `internal/config/config.go`, `cmd/prs/tui.go` and `cmd/prs/config.go`. I traced every caller of `tuiTabs`, `absent`, `Missing`, `MissingPrograms`, `pullrequests.Run` and the placeholder helpers with rg. I ran nothing: no build or test.",
  "standards": "I cited `configuration.md` § \"A templated config has one substitution boundary, and the rendered value is a distinct type\" and `cli-design.md` § \"One fact gets one answer on every command that reaches it\". I found both through `fleet standards search`.",
  "findings": [
    {
      "number": "code-architecture.1",
      "section": "design",
      "sites": [
        "internal/config/config.go:103",
        "internal/config/config.go:109",
        "internal/config/config.go:112",
        "internal/config/config.go:136",
        "internal/config/config.go:199",
        "internal/config/config.go:209",
        "internal/config/config.go:212",
        "internal/config/config.go:426"
      ],
      "reach": "This PR is the second templated command family. It copied the first family's machinery rather than parameterizing it. The next field that takes placeholders, such as a recaps `open` command or a window action that names a recap's window, writes a third copy. A change to how placeholders are matched or filled, such as escaping braces or refusing an empty value, has to be made in step at two sites today.",
      "body": "**The placeholder machinery now exists twice in `config`.** Window actions have `placeholders`, `Placeholders()`, `Fill` and a validation loop in `WindowAction.check`. The recaps block adds `recapPlaceholders`, `RecapPlaceholders()`, `FillRecap` and a second validation loop in `Recaps.check`. Each pair matches line for line, with only the input type changed (`Fields` against `kind, label`). The two sites must agree on these facts:\n\n- the shared `placeholder` regex\n- the order in which a `strings.NewReplacer` is built from a map\n- the error text `\"%s names %s, and only %s are filled in\"`\n- the help text that joins the sorted names\n\n`configuration.md` § \"A templated config has one substitution boundary\" asks for one boundary.\n\n**Fix:** add one generic type to `config`, such as `type template[F any] map[string]func(F) string`, with methods `Names()`, `Fill(s string, f F)` and `Check(field, s string) error`. Declare `placeholders` and `recapPlaceholders` as two values of that type. `WindowAction.check`, `Recaps.check`, `Fill` and `FillRecap` then become one-line calls."
    },
    {
      "number": "code-architecture.2",
      "section": "design",
      "sites": [
        "cmd/prs/config.go:149",
        "cmd/prs/config.go:151",
        "cmd/prs/tui.go:57",
        "cmd/prs/tui.go:61",
        "cmd/prs/tui.go:87",
        "cmd/prs/tui.go:279"
      ],
      "reach": "The PR adds `absent` to `tuiTab` as a general mechanism. Its comment reads \"A nil absent is a tab every config draws\", and `drawnTabs` and the `--view` refusal both read `tab.absent`. The next optional tab gets those two answers for free. It still needs a hand-written block in `prs config show`, or `config show` stays silent about whether that tab is drawn while `prs tui` hides it.",
      "body": "**Three sites decide whether a tab is drawn, and one of them skips the tab list.** Two of them, `drawnTabs` (`tui.go:87`) and the `--view` refusal (`tui.go:279`), go through `tuiTab.absent`. The third is `prs config show` (`cmd/prs/config.go:149-151`). It calls `recapsAbsent(cfg)` by name, behind its own `recaps.Named()` check, and prints a line labelled `recaps` by hand. So the answer to \"is this tab drawn here?\" has a second owner. That owner knows of one tab only. `cli-design.md` § \"One fact gets one answer on every command that reaches it\" covers this.\n\n**Fix:** have `config show` loop over `tuiTabs`. For each tab whose `absent` is non-nil, it prints `drawn` or `not drawn, because <why>`. The printing of the recaps commands can stay where it is, keyed on `cfg.Recaps.Named()`. The drawn or not-drawn verdict then comes from the one list that `--view` and `NewTabs` already read."
    }
  ]
}

This review does not gate merge.

@datapoints-fleet-automation datapoints-fleet-automation Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — system-design

Used: Cloned the PR at its head and read the full diff. I read the new recaps tab, the config loader and the run helper. I did not build or run it.
Standards: I cited no standards rule and ran no fleet standards search.
Findings: 1 correctness · 0 design · 0 prose

Correctness

system-design.1 internal/board/recaps_tab.go:312 · internal/board/recaps_tab.go:360 · internal/board/recaps_tab.go:28

Reach: A user runs prs tui in two terminals, or a scheduled recap write runs on the same machine. Both press w on week 2026-W45. Each process's own writing list is empty, so both start the write command. Two model calls then write one recap file. A write killed at the 30-minute bound can leave a half-written file, and the next show reads it. The far side was not available, so I read nothing of the recap program's behaviour. This is unproven past the tab's own code.

The tab guards double writes only with the in-memory writingOf check, which holds for one prs process. Nothing outside the process serialises writes of one window. The write is an external command that costs a model call and writes a shared file store. A second writer therefore duplicates the cost and can race on the same recap file. The tab cannot know whether the command is atomic or idempotent. RunWithin kills the process group at 30 minutes. A write killed mid-way may have partly replaced an existing recap. Ctx is context.Background(), so quitting prs does not stop the write either. The child keeps running after the TUI exits, and nothing shows it.

Fix: document in the README that the write command must write atomically and take its own per-window lock. Optionally have prs refuse a write whose window is already in the list, unless the user confirms an overwrite. Tie the write to a cancellable context, or say that it outlives prs.

Findings as JSON
{
  "used": "Cloned the PR at its head and read the full diff. I read the new recaps tab, the config loader and the run helper. I did not build or run it.",
  "standards": "I cited no standards rule and ran no `fleet standards search`.",
  "findings": [
    {
      "number": "system-design.1",
      "section": "correctness",
      "sites": [
        "internal/board/recaps_tab.go:312",
        "internal/board/recaps_tab.go:360",
        "internal/board/recaps_tab.go:28"
      ],
      "reach": "A user runs `prs tui` in two terminals, or a scheduled `recap write` runs on the same machine. Both press `w` on week 2026-W45. Each process's own `writing` list is empty, so both start the write command. Two model calls then write one recap file. A write killed at the 30-minute bound can leave a half-written file, and the next show reads it. The far side was not available, so I read nothing of the recap program's behaviour. This is unproven past the tab's own code.",
      "body": "The tab guards double writes only with the in-memory `writingOf` check, which holds for one prs process. Nothing outside the process serialises writes of one window. The write is an external command that costs a model call and writes a shared file store. A second writer therefore duplicates the cost and can race on the same recap file. The tab cannot know whether the command is atomic or idempotent. `RunWithin` kills the process group at 30 minutes. A write killed mid-way may have partly replaced an existing recap. Ctx is `context.Background()`, so quitting prs does not stop the write either. The child keeps running after the TUI exits, and nothing shows it.\n\nFix: document in the README that the write command must write atomically and take its own per-window lock. Optionally have prs refuse a write whose window is already in the list, unless the user confirms an overwrite. Tie the write to a cancellable context, or say that it outlives prs."
    }
  ]
}

This review does not gate merge.

@datapointchris datapointchris left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — stale-prose

Used: I checked out PR 16 at dfedd4b and read every comment, help text, README, CLAUDE.md and schema sentence the diff adds, each beside its code. I built prs into my own directory. I ran prs tui --view recaps --json with a stand-in recap program whose list prints day 2026-11-06 and this is not a window, and a config and registry kept in my directory.
Standards: pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite. Search "sentence states a value the code holds" also surfaced configuration.md/A template that documents a default names the value the code defaults to, and documentation.md/A comment describes the code as it is, never the change or history that produced it.
Findings: 0 correctness · 0 design · 4 prose

Prose

stale-prose.1 internal/board/recaps_tab.go:388

Reach: Any write that runs past recapWriteTimeout (30 minutes) hits the errors.Is(err, pullrequests.ErrNoAnswer) branch in Receive, and the footer shows this message.

False: the timeout message says a write prs has already killed may still be running.

  • internal/board/recaps_tab.go:388: "the write of the recap of %s gave %v, so it may still be running; R reads the list again"

runWithin starts the write with Setsid: true and sets cmd.Cancel to syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL). At the deadline it kills the write's whole process group, and only then returns the ErrNoAnswer this branch matches. The write is not running. What the user does not know is whether it wrote the recap before it was killed. The ErrNoAnswer comment says exactly that: "A client that changes a pull request may have changed it by then". The sibling message in internal/board/work.go:186 says "so it may have happened".

Rewrite: "the write of the recap of %s gave %v and was stopped, so it may have written the recap; R reads the list again".

stale-prose.2 cmd/prs/tui.go:233 · cmd/prs/recaps.schema.json:5

Reach: A list command that prints any line that is not two words reaches the help text's gap. Every --view recaps --json run reaches the schema sentence, because the sources entries leave out reading, fault and note.

False: two sentences describe --view recaps output and exit codes that the head does not produce.

  • cmd/prs/tui.go:232-233 (prs tui --help): "either exits 1 where the list or that recap could not be read." My stand-in list read cleanly and the recap read cleanly. The run still printed error: the list's 1 line printed not a kind and a label, so left out and exited exit=1. That is because Faults adds the list's Note. The README's version at README.md:409 already names this case.
    Rewrite: "either exits 1 where the list or that recap could not be read, or the list printed a line that is not a window."
  • cmd/prs/recaps.schema.json:5: "Every field is always present but fault." In the same run, the recaps source printed no fault, note or reading. The list source printed no fault or reading. The schema itself requires only name and every on a source.
    Rewrite: "Every field of a window is always present but fault."

stale-prose.3 internal/board/recaps_tab.go:104 · cmd/prs/tui.go:216 · cmd/prs/recaps.schema.json:5 · internal/board/recaps_tab.go:126-128

Reach: A user's list command that prints oldest first, or alphabetically, makes the first three sentences false. Nothing in prs checks the order. Any change to how Board.readDetails gates the pull-requests board's detail makes the OpenRecaps reason false.

Will go stale: these sentences state, as prs's behavior, things that another program or another file owns.

  • internal/board/recaps_tab.go:104: "the windows the config's list command prints, newest first"
  • cmd/prs/tui.go:216: "It lists the windows the list command prints, newest first"
  • cmd/prs/recaps.schema.json:5: "each window the config's recaps list command printed, in its order, newest first"

The order belongs to the user's recap program. parseRecapList keeps whatever order the list prints, and nothing sorts it. The README (README.md:362) and the config path help (cmd/prs/config.go:77) can say "newest first", because they tell the user what the list should print. These three sentences instead state as fact what prs shows.
Rewrite each one as "in the order the list command prints them". In the schema, drop ", newest first".

  • internal/board/recaps_tab.go:126-128: "a printed one only the recap it draws, since a board printed once reads no detail either." The reason given is the pull-requests board's behavior in board.go/rows.go, which this file does not keep in step with. The code here only checks r.board.readDetails.
    Rewrite: "An open board reads every listed window's recap once the list lands, and a board without readDetails reads only the recap it draws."

stale-prose.4 README.md:380 · README.md:376

Reach: Changing recapsListEvery in internal/board/recaps_tab.go:25, or adding a key to recapPlaceholders in internal/config/config.go, leaves these README sentences wrong. No test reads the README.

A value the code holds, written out in the README.

  • README.md:380: "The list runs when the tab first shows, every ten minutes while it shows, on R, and once a write ends." The interval is recapsListEvery = 10 * time.Minute. The --json snapshot already prints it as "every": "10m0s" on each source.
    Rewrite: "The list runs when the tab first shows, on a timer while it shows, on R, and once a write ends. --json gives the timer as each source's every."
  • README.md:376: "{kind} and {label} in show and write take the window's." The set is recapPlaceholders. The prs config path --help text in the same diff fills that list from config.RecapPlaceholders(), so the help text cannot drift but the README can.
    Rewrite: "Placeholders in show and write take the window's kind and label. prs config path --help lists them." Or keep the example names and point to the help text for the full list.
Findings as JSON
{
  "used": "I checked out PR 16 at dfedd4b and read every comment, help text, README, CLAUDE.md and schema sentence the diff adds, each beside its code. I built `prs` into my own directory. I ran `prs tui --view recaps --json` with a stand-in `recap` program whose list prints `day 2026-11-06` and `this is not a window`, and a config and registry kept in my directory.",
  "standards": "pull-requests.md/Sentences the diff adds are checked for what goes stale, and the finding carries the rewrite. Search \"sentence states a value the code holds\" also surfaced configuration.md/A template that documents a default names the value the code defaults to, and documentation.md/A comment describes the code as it is, never the change or history that produced it.",
  "findings": [
    {
      "number": "stale-prose.1",
      "section": "prose",
      "sites": [
        "internal/board/recaps_tab.go:388"
      ],
      "reach": "Any write that runs past `recapWriteTimeout` (30 minutes) hits the `errors.Is(err, pullrequests.ErrNoAnswer)` branch in `Receive`, and the footer shows this message.",
      "body": "**False: the timeout message says a write prs has already killed may still be running.**\n\n- `internal/board/recaps_tab.go:388`: \"the write of the recap of %s gave %v, so it may still be running; R reads the list again\"\n\n`runWithin` starts the write with `Setsid: true` and sets `cmd.Cancel` to `syscall.Kill(-cmd.Process.Pid, syscall.SIGKILL)`. At the deadline it kills the write's whole process group, and only then returns the `ErrNoAnswer` this branch matches. The write is not running. What the user does not know is whether it wrote the recap before it was killed. The `ErrNoAnswer` comment says exactly that: \"A client that changes a pull request may have changed it by then\". The sibling message in `internal/board/work.go:186` says \"so it may have happened\".\n\nRewrite: `\"the write of the recap of %s gave %v and was stopped, so it may have written the recap; R reads the list again\"`."
    },
    {
      "number": "stale-prose.2",
      "section": "prose",
      "sites": [
        "cmd/prs/tui.go:233",
        "cmd/prs/recaps.schema.json:5"
      ],
      "reach": "A list command that prints any line that is not two words reaches the help text's gap. Every `--view recaps --json` run reaches the schema sentence, because the `sources` entries leave out `reading`, `fault` and `note`.",
      "body": "**False: two sentences describe `--view recaps` output and exit codes that the head does not produce.**\n\n- `cmd/prs/tui.go:232-233` (`prs tui --help`): \"either exits 1 where the list or that recap could not be read.\" My stand-in list read cleanly and the recap read cleanly. The run still printed `error: the list's 1 line printed not a kind and a label, so left out` and exited `exit=1`. That is because `Faults` adds the list's `Note`. The README's version at `README.md:409` already names this case.\n  Rewrite: \"either exits 1 where the list or that recap could not be read, or the list printed a line that is not a window.\"\n- `cmd/prs/recaps.schema.json:5`: \"Every field is always present but fault.\" In the same run, the `recaps` source printed no `fault`, `note` or `reading`. The `list` source printed no `fault` or `reading`. The schema itself requires only `name` and `every` on a source.\n  Rewrite: \"Every field of a window is always present but fault.\""
    },
    {
      "number": "stale-prose.3",
      "section": "prose",
      "sites": [
        "internal/board/recaps_tab.go:104",
        "cmd/prs/tui.go:216",
        "cmd/prs/recaps.schema.json:5",
        "internal/board/recaps_tab.go:126-128"
      ],
      "reach": "A user's list command that prints oldest first, or alphabetically, makes the first three sentences false. Nothing in prs checks the order. Any change to how `Board.readDetails` gates the pull-requests board's detail makes the `OpenRecaps` reason false.",
      "body": "**Will go stale: these sentences state, as prs's behavior, things that another program or another file owns.**\n\n- `internal/board/recaps_tab.go:104`: \"the windows the config's list command prints, newest first\"\n- `cmd/prs/tui.go:216`: \"It lists the windows the list command prints, newest first\"\n- `cmd/prs/recaps.schema.json:5`: \"each window the config's recaps list command printed, in its order, newest first\"\n\nThe order belongs to the user's `recap` program. `parseRecapList` keeps whatever order the list prints, and nothing sorts it. The README (`README.md:362`) and the `config path` help (`cmd/prs/config.go:77`) can say \"newest first\", because they tell the user what the list should print. These three sentences instead state as fact what prs shows.\nRewrite each one as \"in the order the list command prints them\". In the schema, drop \", newest first\".\n\n- `internal/board/recaps_tab.go:126-128`: \"a printed one only the recap it draws, since a board printed once reads no detail either.\" The reason given is the pull-requests board's behavior in `board.go`/`rows.go`, which this file does not keep in step with. The code here only checks `r.board.readDetails`.\n  Rewrite: \"An open board reads every listed window's recap once the list lands, and a board without readDetails reads only the recap it draws.\""
    },
    {
      "number": "stale-prose.4",
      "section": "prose",
      "sites": [
        "README.md:380",
        "README.md:376"
      ],
      "reach": "Changing `recapsListEvery` in `internal/board/recaps_tab.go:25`, or adding a key to `recapPlaceholders` in `internal/config/config.go`, leaves these README sentences wrong. No test reads the README.",
      "body": "**A value the code holds, written out in the README.**\n\n- `README.md:380`: \"The list runs when the tab first shows, every ten minutes while it shows, on `R`, and once a write ends.\" The interval is `recapsListEvery = 10 * time.Minute`. The `--json` snapshot already prints it as `\"every\": \"10m0s\"` on each source.\n  Rewrite: \"The list runs when the tab first shows, on a timer while it shows, on `R`, and once a write ends. `--json` gives the timer as each source's `every`.\"\n- `README.md:376`: \"`{kind}` and `{label}` in `show` and `write` take the window's.\" The set is `recapPlaceholders`. The `prs config path --help` text in the same diff fills that list from `config.RecapPlaceholders()`, so the help text cannot drift but the README can.\n  Rewrite: \"Placeholders in `show` and `write` take the window's kind and label. `prs config path --help` lists them.\" Or keep the example names and point to the help text for the full list."
    }
  ]
}

This review does not gate merge.

@datapoints-fleet-automation datapoints-fleet-automation Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — useless-test

Used: I cloned the repo and checked out PR 16. I ran go test ./cmd/prs -run Recaps and both new tests passed. I turned off the chosen.absent refusal in cmd/prs/tui.go and reran the drawn-only test. It failed on exit 2 from a panic in RunWithin, so the exit code carries the condition by itself. I restored the file afterwards. I also searched the existing tests in tui_test.go, summary_test.go and internal/board for the code the diff changes. None of them pins help text, ErrNoAnswer's wording or the order of tuiTabs.
Standards: testing.md § "Test behavior the user cares about, not what this commit happened to change"; pull-requests.md § "Tests the diff adds are checked for whether they test the program or the change"; testing.md § "Never assert on rendered output — assert the value it was built from".
Findings: 0 correctness · 1 design · 0 prose

Design and architecture

useless-test.1 cmd/prs/recaps_test.go:125 · cmd/prs/tui.go:64

Reach: prs tui --view recaps --json on a config with no recaps block reaches recapsAbsent. The test's exit-code check already catches it when the refusal is removed: the run then panics with exit 2.

Delete one assertion from a test that stays: the check for the absent tab's reason.

This is not a request to delete a test. Both new tests stay:

  • TestTheRecapsTabShowsEachWindowsRecapAndWritesOneAfterAskingThenListsAgain runs the binary on a stand-in recap program. It checks the recorded argv of write --day 2026-11-07 and that list ran again after the write. Each prompt and footer it waits for contains the window the user chose, which is a computed value.
  • TestTheRecapsTabIsDrawnOnlyWhereTheConfigNamesItsCommandsAndItsSnapshotHoldsItsSchema pins three behaviors. The tab is missing from the tab bar when drawnTabs drops it. --json, a contract, validates against the schema. Only the first recap is read.

One clause is the help-text shape. At line 125 the test checks strings.Contains(said, "the config names no recaps commands"). That string is the literal recapsAbsent returns at cmd/prs/tui.go:64. It interpolates nothing, so it fails on any rewording and exercises no logic.

I turned off the refusal (chosen.absent(cfg) != "" && false) and reran the test. The code != 1 half failed by itself, with exit 2 from a panic in RunWithin on an empty argv. Dropping the substring leaves the test pinning the refusal through its exit code.

Fix: delete the substring clause and keep code != 1. Suppose a later test needs to tell "names no commands" from "a needed program is missing". Then recapsAbsent should return a key, the sentence should be derived from it, and the test should compare keys.

Findings as JSON
{
  "used": "I cloned the repo and checked out PR 16. I ran `go test ./cmd/prs -run Recaps` and both new tests passed. I turned off the `chosen.absent` refusal in `cmd/prs/tui.go` and reran the drawn-only test. It failed on exit 2 from a panic in `RunWithin`, so the exit code carries the condition by itself. I restored the file afterwards. I also searched the existing tests in `tui_test.go`, `summary_test.go` and `internal/board` for the code the diff changes. None of them pins help text, `ErrNoAnswer`'s wording or the order of `tuiTabs`.",
  "standards": "testing.md § \"Test behavior the user cares about, not what this commit happened to change\"; pull-requests.md § \"Tests the diff adds are checked for whether they test the program or the change\"; testing.md § \"Never assert on rendered output — assert the value it was built from\".",
  "findings": [
    {
      "number": "useless-test.1",
      "section": "design",
      "sites": [
        "cmd/prs/recaps_test.go:125",
        "cmd/prs/tui.go:64"
      ],
      "reach": "`prs tui --view recaps --json` on a config with no `recaps` block reaches `recapsAbsent`. The test's exit-code check already catches it when the refusal is removed: the run then panics with exit 2.",
      "body": "**Delete one assertion from a test that stays: the check for the absent tab's reason.**\n\nThis is not a request to delete a test. Both new tests stay:\n- `TestTheRecapsTabShowsEachWindowsRecapAndWritesOneAfterAskingThenListsAgain` runs the binary on a stand-in `recap` program. It checks the recorded argv of `write --day 2026-11-07` and that `list` ran again after the write. Each prompt and footer it waits for contains the window the user chose, which is a computed value.\n- `TestTheRecapsTabIsDrawnOnlyWhereTheConfigNamesItsCommandsAndItsSnapshotHoldsItsSchema` pins three behaviors. The tab is missing from the tab bar when `drawnTabs` drops it. `--json`, a contract, validates against the schema. Only the first recap is read.\n\nOne clause is the help-text shape. At line 125 the test checks `strings.Contains(said, \"the config names no recaps commands\")`. That string is the literal `recapsAbsent` returns at `cmd/prs/tui.go:64`. It interpolates nothing, so it fails on any rewording and exercises no logic.\n\nI turned off the refusal (`chosen.absent(cfg) != \"\" && false`) and reran the test. The `code != 1` half failed by itself, with exit 2 from a panic in `RunWithin` on an empty argv. Dropping the substring leaves the test pinning the refusal through its exit code.\n\nFix: delete the substring clause and keep `code != 1`. Suppose a later test needs to tell \"names no commands\" from \"a needed program is missing\". Then `recapsAbsent` should return a key, the sentence should be derived from it, and the test should compare keys."
    }
  ]
}

This review does not gate merge.

@datapoints-fleet-automation datapoints-fleet-automation Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — contracts

Used: Built the PR head into my own directory and ran it against a config, registry and cache there, with a stand-in recap on PATH. Ran prs config show, prs tui --view recaps --json and --once, --view recaps with --since and --pr, the tab with recap missing from PATH, a list printing a line that is not two words, and four bad recaps blocks: no show, an unknown placeholder, an unknown key, and a string where a list belongs. Searched the owner's repos (dotfiles cloned) for callers of recaps, recap list/show/write, prs tui --view and the prs config.
Standards: tui.md/A TUI key means the same thing in every fleet TUI; cli-design.md/Exit codes are 0 success, 2 usage error, other non-zero for failure; cli-design.md/A usage error is decided before the first request; cli-design.md/The shape a --json read or an endpoint returns is a JSON Schema file that the data catalog publishes.
Findings: 0 correctness · 1 design · 0 prose

Design and architecture

contracts.1 internal/board/recaps_tab.go:90 · internal/board/recaps_tab.go:138 · internal/board/recaps_tab.go:171 · internal/board/recaps_tab.go:131 · README.md:380

Reach: Someone whose recap program rewrites a window outside prs, for example a scheduled recap write, presses R or waits for the timer. Every row but the cursor's keeps its old recap, and / keeps matching the old text. A script reading prs tui --view recaps --json sees the recaps source as "every": "10m0s", which the held recaps never honour. Read from the code. An interactive tuidrive run would confirm it.

On the recaps tab, R and the ten-minute timer re-read only recaps that were never read. due() is !reading && (!held || stale). Only a write that ends from this tab sets stale. readList re-reads the cursor's recap alone, and readRecaps reads only the due ones. So once a recap is read, R never reads it again unless the cursor is on it. Moving the cursor onto it later does not read it again either. tui.md § "A TUI key means the same thing in every fleet TUI" gives R one meaning in every TUI: it reads every source again. The --json snapshot I ran also lists the recaps source with "every": "10m0s", a period for recap text that is never refreshed on that schedule.

Fix: have R mark every held recap stale before it forces the list, so it reads them all again. Keep the timer's narrower re-read of the cursor's recap. Or, if only the cursor's recap is ever meant to refresh, report the recaps source with no period and say so in the R help line.

Findings as JSON
{
  "used": "Built the PR head into my own directory and ran it against a config, registry and cache there, with a stand-in `recap` on PATH. Ran `prs config show`, `prs tui --view recaps --json` and `--once`, `--view recaps` with `--since` and `--pr`, the tab with `recap` missing from PATH, a list printing a line that is not two words, and four bad `recaps` blocks: no `show`, an unknown placeholder, an unknown key, and a string where a list belongs. Searched the owner's repos (dotfiles cloned) for callers of `recaps`, `recap list/show/write`, `prs tui --view` and the prs config.",
  "standards": "tui.md/A TUI key means the same thing in every fleet TUI; cli-design.md/Exit codes are 0 success, 2 usage error, other non-zero for failure; cli-design.md/A usage error is decided before the first request; cli-design.md/The shape a --json read or an endpoint returns is a JSON Schema file that the data catalog publishes.",
  "findings": [
    {
      "number": "contracts.1",
      "section": "design",
      "sites": [
        "internal/board/recaps_tab.go:90",
        "internal/board/recaps_tab.go:138",
        "internal/board/recaps_tab.go:171",
        "internal/board/recaps_tab.go:131",
        "README.md:380"
      ],
      "reach": "Someone whose recap program rewrites a window outside prs, for example a scheduled `recap write`, presses `R` or waits for the timer. Every row but the cursor's keeps its old recap, and `/` keeps matching the old text. A script reading `prs tui --view recaps --json` sees the `recaps` source as `\"every\": \"10m0s\"`, which the held recaps never honour. Read from the code. An interactive tuidrive run would confirm it.",
      "body": "**On the recaps tab, `R` and the ten-minute timer re-read only recaps that were never read.** `due()` is `!reading && (!held || stale)`. Only a write that ends from this tab sets `stale`. `readList` re-reads the cursor's recap alone, and `readRecaps` reads only the due ones. So once a recap is read, `R` never reads it again unless the cursor is on it. Moving the cursor onto it later does not read it again either. `tui.md` § \"A TUI key means the same thing in every fleet TUI\" gives `R` one meaning in every TUI: it reads every source again. The `--json` snapshot I ran also lists the `recaps` source with `\"every\": \"10m0s\"`, a period for recap text that is never refreshed on that schedule.\n\n**Fix:** have `R` mark every held recap stale before it forces the list, so it reads them all again. Keep the timer's narrower re-read of the cursor's recap. Or, if only the cursor's recap is ever meant to refresh, report the `recaps` source with no period and say so in the `R` help line."
    }
  ]
}

This review does not gate merge.

@datapointchris datapointchris left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — fleet-patterns

Used: I checked out PR 16 at dfedd4b in my clone and read every changed file. I read the program the tab will run in practice: fleet's recap package plus ledger_recap.go and agents_recap.go. I read the matching dotfiles config on branch prs-recaps-config. I searched indy for config-named argv commands, PATH-gated tabs and markdown list tabs. I compared prs's shellQuoted with the shellQuote helpers in slate and fleet.
Standards: I cited cli-design.md/A client retries a bounded few times, names the server and what chose it, and exits 4 when no answer came. I also searched for api-design.md/A value the server owns reaches the client from the server, and the PR follows it: the list supplies the windows and fleet validates a typed one. I searched for code-architecture.md/Every external effect goes through one named wrapper every call site uses, and RunWithin follows it. Searches on config-named argv commands and on tabs gated on PATH found no rule.
Findings: 0 correctness · 1 design · 0 prose

Design and architecture

fleet-patterns.1 internal/board/recaps_tab.go:387 · internal/pullrequests/pullrequest.go:171 · fleet:cmd/agents_recap.go:121 · dotfiles:configs/common/.config/prs/config.yml.tmpl:47

Reach: The only real config is the dotfiles branch prs-recaps-config. It sets write: [fleet, agents, dispatch, recap, ...], a command that follows the run and exits 4 when the daemon gives no answer or is lost mid-run. Receive sends that exit down its err != nil branch. The next caller to pay for this is any key or tab that runs a fleet client and has to tell 'no answer' apart from 'refused'.

The write treats "no answer" as "failed" in a different way from the fleet's exit-status contract. Receive says a write "may still be running" only when prs's own 30-minute kill fired (errors.Is(err, pullrequests.ErrNoAnswer)). The fleet has its own fleet-wide signal for the same state: cli-design.md § "A client retries a bounded few times, names the server and what chose it, and exits 4 when no answer came". fleet agents dispatch recap follows that rule. Its help (fleet:cmd/agents_recap.go:121) says it exits 4 when the daemon gave no answer or was lost while following. Ctrl-C or a lost connection leaves the run going.

Suppose the daemon drops the follow halfway through a model call. prs then prints could not write the recap of day 2026-11-07: .... The recap is in fact still being written on the daemon. The user presses w again. fleet refuses it because a run already holds that window. The tab then reports a second failure for a write that is succeeding.

prs already reads exit statuses with exitedWith (internal/pullrequests/fetch.go:132). Here only its own timeout counts.

Fix: have RunWithin wrap ErrNoAnswer when the client exits 4, as the fleet contract defines, as well as when prs's own bound fires. The existing ErrNoAnswer branch in Receive then gives the right message for both. Keep the config-agnostic design: exit 4 is the fleet-wide CLI contract, not anything specific to fleet's recap internals.

Findings as JSON
{
  "used": "I checked out PR 16 at dfedd4b in my clone and read every changed file. I read the program the tab will run in practice: fleet's `recap` package plus `ledger_recap.go` and `agents_recap.go`. I read the matching dotfiles config on branch `prs-recaps-config`. I searched indy for config-named argv commands, PATH-gated tabs and markdown list tabs. I compared prs's shellQuoted with the shellQuote helpers in slate and fleet.",
  "standards": "I cited cli-design.md/A client retries a bounded few times, names the server and what chose it, and exits 4 when no answer came. I also searched for api-design.md/A value the server owns reaches the client from the server, and the PR follows it: the list supplies the windows and fleet validates a typed one. I searched for code-architecture.md/Every external effect goes through one named wrapper every call site uses, and RunWithin follows it. Searches on config-named argv commands and on tabs gated on PATH found no rule.",
  "findings": [
    {
      "number": "fleet-patterns.1",
      "section": "design",
      "sites": [
        "internal/board/recaps_tab.go:387",
        "internal/pullrequests/pullrequest.go:171",
        "fleet:cmd/agents_recap.go:121",
        "dotfiles:configs/common/.config/prs/config.yml.tmpl:47"
      ],
      "reach": "The only real config is the dotfiles branch `prs-recaps-config`. It sets `write: [fleet, agents, dispatch, recap, ...]`, a command that follows the run and exits 4 when the daemon gives no answer or is lost mid-run. `Receive` sends that exit down its `err != nil` branch. The next caller to pay for this is any key or tab that runs a fleet client and has to tell 'no answer' apart from 'refused'.",
      "body": "**The write treats \"no answer\" as \"failed\" in a different way from the fleet's exit-status contract.** `Receive` says a write \"may still be running\" only when prs's own 30-minute kill fired (`errors.Is(err, pullrequests.ErrNoAnswer)`). The fleet has its own fleet-wide signal for the same state: `cli-design.md` § \"A client retries a bounded few times, names the server and what chose it, and exits 4 when no answer came\". `fleet agents dispatch recap` follows that rule. Its help (`fleet:cmd/agents_recap.go:121`) says it exits 4 when the daemon gave no answer or was lost while following. Ctrl-C or a lost connection leaves the run going.\n\nSuppose the daemon drops the follow halfway through a model call. prs then prints `could not write the recap of day 2026-11-07: ...`. The recap is in fact still being written on the daemon. The user presses `w` again. fleet refuses it because a run already holds that window. The tab then reports a second failure for a write that is succeeding.\n\nprs already reads exit statuses with `exitedWith` (`internal/pullrequests/fetch.go:132`). Here only its own timeout counts.\n\n**Fix:** have `RunWithin` wrap ErrNoAnswer when the client exits 4, as the fleet contract defines, as well as when prs's own bound fires. The existing `ErrNoAnswer` branch in `Receive` then gives the right message for both. Keep the config-agnostic design: exit 4 is the fleet-wide CLI contract, not anything specific to fleet's recap internals."
    }
  ]
}

This review does not gate merge.

@datapointchris datapointchris left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — G1

Used: Built prs from the branch. Ran config show, plus tui --view recaps with --json, --once and --since, against a stand-in recap under a private config. In a private tmux I drove the interactive tab: reading recaps, w with Enter, a typed window, a second w during a write, Tab away during a write, q during a write, and a 60-window list with 0.5 s reads.
Standards: tui.md § "A TUI key means the same thing in every fleet TUI" (its Why, m-then-Enter, applies inside one tool). Searches that turned up no rule to cite: "quit while a background command is still running", "sh -c pipefail", "confirmation prompt default yes or no for a costly action", "a question answered by enter alone".
Findings: 2 correctness · 0 design · 0 prose

Correctness

G1.1 internal/board/recaps_tab.go:287 · internal/board/recaps_tab.go:301 · internal/board/recaps_tab.go:334 · internal/board/summary_tab.go:169

Reach: Anyone who steps the Summary tab's window with w and then presses Tab. The Recaps tab sits right after Summary in tuiTabs, so one Tab lands there. Both tabs call their subject a "window".

On the Summary tab, w means "count the next longer window" (footer w window). On the Recaps tab, which is one Tab away, w asks write the recap of <cursor's window> with a model call? [Y/n]. Enter alone answers yes (answeredWindow treats "" as yes). So w then Enter, the habit from the tab next door, starts a paid model call for whatever row the cursor sits on.

I ran it: Tab to recaps, w, Enter. recap write --week 2026-W45 ran at once, and the footer said writing the recap of week 2026-W45. fleet's own dispatch recap help says that writing a window again keeps both records, so each accidental Enter is a real, billed second recap.

w is not in the shared key tables, so tui.md § "A TUI key means the same thing in every fleet TUI" does not bind it. Its Why is this exact case, though: "m then Enter moving a row in one tool would merge a pull request in another". Here the clash sits inside one program.

Fix: make the question [y/N], so Enter alone writes nothing and y or a typed window is needed. The rest of the design stays. Or bind the write to a letter prs uses nowhere else, and keep [y/N] either way.

G1.2 internal/board/recaps_tab.go:354 · internal/board/recaps_tab.go:360

Reach: unproven. The trigger is common: q pressed while a write runs, and a write can take up to 30 minutes. Whether the recap is lost depends on the configured write. fleet's agents dispatch recap client detaches and its daemon run survives. A write that runs the model in-process and prints to stdout dies. Running q against the real write command would settle it.

Pressing q while the footer shows writing the recap of … quits at once with exit 0. There is no warning that work is under way. The write's process lives in its own session (Setsid) and nobody waits on it, so it is orphaned with its stdout pipe closed.

I ran it with a stand-in write that sleeps, prints a line, then writes the file. After q, ps showed recap write --day 2026-11-09 still alive. It then died on its first print to the closed pipe, and day-2026-11-09.md was never written. For a write that spends the model call before printing, the money is gone and there is no recap. Nothing in "What this does not do" covers it.

Fix: while UnderWay() is non-empty, have q ask (a recap write is under way; quit anyway? [y/N]). That guard likely belongs in gotermboard's quit handling for any BusyBoard. Or run the write with stdout and stderr sent to a file under the cache dir, so it survives the board quitting, and say so in the README.

Findings as JSON
{
  "used": "Built `prs` from the branch. Ran `config show`, plus `tui --view recaps` with `--json`, `--once` and `--since`, against a stand-in `recap` under a private config. In a private tmux I drove the interactive tab: reading recaps, `w` with Enter, a typed window, a second `w` during a write, Tab away during a write, `q` during a write, and a 60-window list with 0.5 s reads.",
  "standards": "tui.md § \"A TUI key means the same thing in every fleet TUI\" (its Why, `m`-then-Enter, applies inside one tool). Searches that turned up no rule to cite: \"quit while a background command is still running\", \"sh -c pipefail\", \"confirmation prompt default yes or no for a costly action\", \"a question answered by enter alone\".",
  "findings": [
    {
      "number": "G1.1",
      "section": "correctness",
      "sites": [
        "internal/board/recaps_tab.go:287",
        "internal/board/recaps_tab.go:301",
        "internal/board/recaps_tab.go:334",
        "internal/board/summary_tab.go:169"
      ],
      "reach": "Anyone who steps the Summary tab's window with `w` and then presses Tab. The Recaps tab sits right after Summary in `tuiTabs`, so one Tab lands there. Both tabs call their subject a \"window\".",
      "body": "On the Summary tab, `w` means \"count the next longer window\" (footer `w window`). On the Recaps tab, which is one Tab away, `w` asks `write the recap of <cursor's window> with a model call? [Y/n]`. Enter alone answers yes (`answeredWindow` treats `\"\"` as yes). So `w` then Enter, the habit from the tab next door, starts a paid model call for whatever row the cursor sits on.\n\nI ran it: Tab to recaps, `w`, Enter. `recap write --week 2026-W45` ran at once, and the footer said `writing the recap of week 2026-W45`. fleet's own `dispatch recap` help says that writing a window again keeps both records, so each accidental Enter is a real, billed second recap.\n\n`w` is not in the shared key tables, so tui.md § \"A TUI key means the same thing in every fleet TUI\" does not bind it. Its Why is this exact case, though: \"`m` then Enter moving a row in one tool would merge a pull request in another\". Here the clash sits inside one program.\n\nFix: make the question `[y/N]`, so Enter alone writes nothing and `y` or a typed window is needed. The rest of the design stays. Or bind the write to a letter prs uses nowhere else, and keep `[y/N]` either way."
    },
    {
      "number": "G1.2",
      "section": "correctness",
      "sites": [
        "internal/board/recaps_tab.go:354",
        "internal/board/recaps_tab.go:360"
      ],
      "reach": "unproven. The trigger is common: `q` pressed while a write runs, and a write can take up to 30 minutes. Whether the recap is lost depends on the configured `write`. fleet's `agents dispatch recap` client detaches and its daemon run survives. A write that runs the model in-process and prints to stdout dies. Running `q` against the real `write` command would settle it.",
      "body": "Pressing `q` while the footer shows `writing the recap of …` quits at once with exit 0. There is no warning that work is under way. The write's process lives in its own session (`Setsid`) and nobody waits on it, so it is orphaned with its stdout pipe closed.\n\nI ran it with a stand-in `write` that sleeps, prints a line, then writes the file. After `q`, `ps` showed `recap write --day 2026-11-09` still alive. It then died on its first print to the closed pipe, and `day-2026-11-09.md` was never written. For a write that spends the model call before printing, the money is gone and there is no recap. Nothing in \"What this does not do\" covers it.\n\nFix: while `UnderWay()` is non-empty, have `q` ask (`a recap write is under way; quit anyway? [y/N]`). That guard likely belongs in gotermboard's quit handling for any `BusyBoard`. Or run the write with stdout and stderr sent to a file under the cache dir, so it survives the board quitting, and say so in the README."
    }
  ]
}

This review does not gate merge.

@datapointchris datapointchris left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR review — synthesis

Used: the 13 findings of round 1, from 8 review(s), and nothing else
Standards: none, because this step reads only the findings
Findings: 0 correctness · 2 design · 0 prose

Design and architecture

synthesis.1 explains G1.2 · system-design.1

Nothing owns the write command's life once it starts. The tab's guard is in-memory and the child runs in its own session under context.Background(). Quitting prs with q orphans the child and closes its stdout (G1.2). A second prs process or a scheduled write can start the same window's write as well (system-design.1). The recap program's atomicity is not shown anywhere.

synthesis.2 explains stale-prose.3 · stale-prose.4

Prose in the README and schema states as prs's own fact something that another owner holds: the list program's order, the board's detail-read behaviour, the timer interval, and the placeholder set. Each sentence goes stale when that owner changes.

Shapes as JSON
{
  "shapes": [
    {
      "number": "synthesis.1",
      "section": "design",
      "explains": [
        "G1.2",
        "system-design.1"
      ],
      "body": "Nothing owns the write command's life once it starts. The tab's guard is in-memory and the child runs in its own session under `context.Background()`. Quitting prs with `q` orphans the child and closes its stdout (G1.2). A second prs process or a scheduled write can start the same window's write as well (system-design.1). The recap program's atomicity is not shown anywhere."
    },
    {
      "number": "synthesis.2",
      "section": "design",
      "explains": [
        "stale-prose.3",
        "stale-prose.4"
      ],
      "body": "Prose in the README and schema states as prs's own fact something that another owner holds: the list program's order, the board's detail-read behaviour, the timer interval, and the placeholder set. Each sentence goes stale when that owner changes."
    }
  ],
  "duplicates": []
}

This review does not gate merge.

@datapointchris

Copy link
Copy Markdown
Owner Author

PR review round 1

Ran: G1, useless-test, code-architecture, fleet-patterns, system-design, stale-prose, failure-paths, contracts, synthesis
Did not run: none
Rules: 1 proposed, filed into the fleet inbox as proposals.

reviewer model effort duration uncached input cache write cache read output findings review
G1 opus high 5m 49s 62 74,269 1,717,949 21,338 2 review
useless-test opus high 2m 37s 20 31,177 242,878 5,255 1 review
code-architecture opus medium 1m 01s 14 32,054 166,492 4,075 2 review
fleet-patterns opus medium 5m 16s 40 53,092 797,570 7,556 1 review
system-design sonnet medium 0m 37s 14 16,464 109,686 2,157 1 review
stale-prose opus medium 2m 11s 30 46,537 533,699 9,284 4 review
failure-paths sonnet medium 0m 47s 12 23,632 98,055 2,858 1 review
contracts opus medium 3m 46s 36 33,950 484,177 8,685 1 review
synthesis sonnet medium 0m 09s 2 10,422 0 908 2 review
total 230 321,597 4,150,506 62,116 15
Round record
{
  "grammar": 6,
  "round": 1,
  "repo": "datapointchris/prs",
  "pr": 16,
  "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
  "experiment": "none",
  "dispatch": "20261108T165850Z-pr-review",
  "roster": "44e66c70a21627e0922b633d0e7db86363a01f6c",
  "harness": "69825f8d525ae93bd2ae6cb72a11e88419ad73f6",
  "machine": "archlinux",
  "started": "2026-11-08T16:59:01Z",
  "finished": "2026-11-08T17:05:23Z",
  "reviewers": [
    {
      "label": "G1",
      "agent": "pr-reviewer",
      "model": "opus",
      "effort": "high",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/general.md",
      "prompt_sha": "00271016dd7d",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 349,
      "tokens": {
        "input": 62,
        "cache_write": 74269,
        "cache_read": 1717949,
        "output": 21338
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 62,
          "cache_write": 74269,
          "cache_read": 1717949,
          "output": 21338
        }
      },
      "session": "01a1bc19-50ac-7d4a-bafd-bf078120886d",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818781290",
      "review_id": 5818781290,
      "findings": {
        "correctness": 2,
        "design": 0,
        "prose": 0
      },
      "numbers": [
        "G1.1",
        "G1.2"
      ],
      "rules": {
        "proposed": 1,
        "keys": [
          "rule/tui/a-letter-a-tool-keeps-for-itself-means-one-thing-on-every-one-of-its-tabs"
        ],
        "filed": true,
        "why": "",
        "path": "/home/chris/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records/G1.rules.json"
      },
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:02Z",
      "finished": "2026-11-08T17:04:53Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "useless-test",
      "agent": "pr-useless-test-reviewer",
      "model": "opus",
      "effort": "high",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/useless-test.md",
      "prompt_sha": "f0b52bbf8a48",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 157,
      "tokens": {
        "input": 20,
        "cache_write": 31177,
        "cache_read": 242878,
        "output": 5255
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 20,
          "cache_write": 31177,
          "cache_read": 242878,
          "output": 5255
        }
      },
      "session": "01a1bc19-53c5-79df-be5a-e12d3f4bc41f",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818772628",
      "review_id": 5818772628,
      "findings": {
        "correctness": 0,
        "design": 1,
        "prose": 0
      },
      "numbers": [
        "useless-test.1"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:28Z",
      "finished": "2026-11-08T17:02:10Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "code-architecture",
      "agent": "pr-code-architecture-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "283f4014efc3",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 61,
      "tokens": {
        "input": 14,
        "cache_write": 32054,
        "cache_read": 166492,
        "output": 4075
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 14,
          "cache_write": 32054,
          "cache_read": 166492,
          "output": 4075
        }
      },
      "session": "01a1bc19-506b-7480-8eef-077235161004",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818765997",
      "review_id": 5818765997,
      "findings": {
        "correctness": 0,
        "design": 2,
        "prose": 0
      },
      "numbers": [
        "code-architecture.1",
        "code-architecture.2"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:02Z",
      "finished": "2026-11-08T17:00:05Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "fleet-patterns",
      "agent": "pr-fleet-patterns-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "efa99c642e6d",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 316,
      "tokens": {
        "input": 40,
        "cache_write": 53092,
        "cache_read": 797570,
        "output": 7556
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 40,
          "cache_write": 53092,
          "cache_read": 797570,
          "output": 7556
        }
      },
      "session": "01a1bc19-4dfe-7a3d-83bc-1e777d360942",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818779388",
      "review_id": 5818779388,
      "findings": {
        "correctness": 0,
        "design": 1,
        "prose": 0
      },
      "numbers": [
        "fleet-patterns.1"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:01Z",
      "finished": "2026-11-08T17:04:19Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "system-design",
      "agent": "pr-system-design-reviewer",
      "model": "sonnet",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "bc2eb8615f41",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 37,
      "tokens": {
        "input": 14,
        "cache_write": 16464,
        "cache_read": 109686,
        "output": 2157
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-sonnet-5-5": {
          "input": 14,
          "cache_write": 16464,
          "cache_read": 109686,
          "output": 2157
        }
      },
      "session": "01a1bc19-56c8-763a-8d8d-8c5f2cf849fc",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818766334",
      "review_id": 5818766334,
      "findings": {
        "correctness": 1,
        "design": 0,
        "prose": 0
      },
      "numbers": [
        "system-design.1"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:28Z",
      "finished": "2026-11-08T17:00:11Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "stale-prose",
      "agent": "pr-stale-prose-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "4359b874a436",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 131,
      "tokens": {
        "input": 30,
        "cache_write": 46537,
        "cache_read": 533699,
        "output": 9284
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 30,
          "cache_write": 46537,
          "cache_read": 533699,
          "output": 9284
        }
      },
      "session": "01a1bc19-4eea-7850-a39a-ce6d4222d5b9",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818769628",
      "review_id": 5818769628,
      "findings": {
        "correctness": 0,
        "design": 0,
        "prose": 4
      },
      "numbers": [
        "stale-prose.1",
        "stale-prose.2",
        "stale-prose.3",
        "stale-prose.4"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:01Z",
      "finished": "2026-11-08T17:01:15Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "failure-paths",
      "agent": "pr-failure-paths-reviewer",
      "model": "sonnet",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "4239ed2be477",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 47,
      "tokens": {
        "input": 12,
        "cache_write": 23632,
        "cache_read": 98055,
        "output": 2858
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-sonnet-5-5": {
          "input": 12,
          "cache_write": 23632,
          "cache_read": 98055,
          "output": 2858
        }
      },
      "session": "01a1bc19-50ec-70d5-a3bc-32b6e1aff82d",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818765336",
      "review_id": 5818765336,
      "findings": {
        "correctness": 1,
        "design": 0,
        "prose": 0
      },
      "numbers": [
        "failure-paths.1"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:03Z",
      "finished": "2026-11-08T16:59:52Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "archlinux"
    },
    {
      "label": "contracts",
      "agent": "pr-contracts-reviewer",
      "model": "opus",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/persona.md",
      "prompt_sha": "3cf737a7c111",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 226,
      "tokens": {
        "input": 36,
        "cache_write": 33950,
        "cache_read": 484177,
        "output": 8685
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-opus-5-5": {
          "input": 36,
          "cache_write": 33950,
          "cache_read": 484177,
          "output": 8685
        }
      },
      "session": "01a1bc19-5192-79d3-8ad8-02c5ee3b14a7",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818776362",
      "review_id": 5818776362,
      "findings": {
        "correctness": 0,
        "design": 1,
        "prose": 0
      },
      "numbers": [
        "contracts.1"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/fleet/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T16:59:28Z",
      "finished": "2026-11-08T17:03:22Z",
      "account": "live",
      "api_key_source": "none",
      "machine": "queue"
    },
    {
      "label": "synthesis",
      "agent": null,
      "model": "sonnet",
      "effort": "medium",
      "experiment": "none",
      "arm": "default",
      "reason": "STATIC",
      "prompt_profile": "pr-reviewer-prompts/synthesis.md",
      "prompt_sha": "8832c4b84e09",
      "status": "ran",
      "why": "",
      "attempts": 1,
      "seconds": 9,
      "tokens": {
        "input": 2,
        "cache_write": 10422,
        "cache_read": 0,
        "output": 908
      },
      "tokens_from": "result",
      "tokens_by_model": {
        "claude-sonnet-5-5": {
          "input": 2,
          "cache_write": 10422,
          "cache_read": 0,
          "output": 908
        }
      },
      "session": "01a1bc1e-e36e-77f3-ab01-bce865cbd51c",
      "worklog": "1333.1",
      "worklog_refused": "",
      "claude_code": "2.1.319",
      "review_url": "https://github.com/datapointchris/prs/pull/16#pullrequestreview-5818782877",
      "review_id": 5818782877,
      "findings": {
        "correctness": 0,
        "design": 2,
        "prose": 0
      },
      "numbers": [
        "synthesis.1",
        "synthesis.2"
      ],
      "rules": {},
      "head": "dfedd4bab2c20dd33dd70d36143f396a9ef585e8",
      "kept": false,
      "records": "/home/chris/.cache/run-pr-reviewers/datapointchris-prs-pr16-round1-20261108T165850Z/records",
      "started": "2026-11-08T17:05:10Z",
      "finished": "2026-11-08T17:05:23Z",
      "account": "fleetizen",
      "api_key_source": "none",
      "machine": "archlinux"
    }
  ],
  "rules": {
    "proposed": 1,
    "keys": [
      "rule/tui/a-letter-a-tool-keeps-for-itself-means-one-thing-on-every-one-of-its-tabs"
    ],
    "unfiled": []
  },
  "kept": []
}

…t run

A write waits on a model call and on whatever admits it, so it now runs
in a session of its own with its output in a file under
$XDG_CACHE_HOME/prs/recap-writes. Quitting prs leaves it to finish, and
a failure names its log and says its last line. Nothing bounds it, so no
write is killed half done.

Each run of the list marks when it began, and every recap read before
that is due. R, the timer and a write's end then reach every recap, and
a read under way when a write ended is read again.

w asks with enter meaning no, because a reflex enter would spend a model
call. y writes the cursor's window.

Window actions and recaps fill and check their placeholders through one
set, and prs config show takes each optional tab's verdict from the tab
list.

Agent-Name: claude-code-unattended
Agent-Session: 7e57e57e-0638-4af5-be9f-487326ef59f6
Agent-Run: 01a1bc48-d561-7128-89d3-20d5223331eb
Worklog: 1333.1
@datapointchris
datapointchris merged commit 5ac397b into main Nov 8, 2026
2 checks passed
@datapointchris
datapointchris deleted the prs-fleet-tab branch November 8, 2026 18:07
datapointchris added a commit to datapointchris/dotfiles that referenced this pull request Nov 8, 2026
prs draws a recaps tab only where its config names the commands behind
it, so until this lands the tab never shows on any machine. This names
the three commands in prs's config template: `list` prints each window
recorded over the last 90 days as `<kind> <label>`, newest first, `show`
prints one window's recap, and `write` dispatches one with a model call.

## What to look at

1. `configs/common/.config/prs/config.yml.tmpl`, the `recaps` block.
`needs: [fleet, jq]` keeps the tab off a machine the same template
reaches without them, because the list runs through `sh -c` and hides
both from prs.
2. The `list` script's `set -o pipefail`. Without it, fleet failing
ahead of jq exits 0 with nothing printed, and the tab reads that as no
recap written.

## How it was verified

The template was rendered with `REPOS_REGISTRY` filled, and loaded by
the prs binary built from datapointchris/prs#16:

```text
$ XDG_CONFIG_HOME=<rendered> prs config show | tail -4
recaps    drawn
  list    sh -c 'set -o pipefail; fleet ledger recap --since 90d --json | jq -r '\''sort_by(.window.since) | reverse | .[].window | "\(.kind) \(.label)"'\'''
  show    fleet ledger recap --{kind} {label}
  write   fleet agents dispatch recap --{kind} {label}
$ XDG_CONFIG_HOME=<rendered> prs tui --view recaps --json | jq -c '[.windows[] | "\(.kind) \(.label)"]'
["day 2026-11-06","day 2026-11-05","day 2026-11-04","week 2026-W45","week 2026-W44"]
```

Without `pipefail`, the list run with fleet's daemon unconfigured
printed fleet's error on stderr and exited 0. With it, the same run
exited 1.

## What changes

- `prs tui` gains a recaps tab on every machine with fleet and jq, after
the next `dotfiles apply`.
- A machine missing either draws no tab, and nothing else changes there.

## Risk and rollback

prs 0.15.0 and earlier refuse a config key they do not know, and this
adds `recaps`:

```text
$ XDG_CONFIG_HOME=<rendered> prs config show
Error: reading .../prs/config.yml: yaml: unmarshal errors:
  line 37: field recaps not found in type config.file
```

So this merges only once the prs release carrying datapointchris/prs#16
is installed. A revert removes the block, and the tab with it.

Session 7e57e57e-0638-4af5-be9f-487326ef59f6
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant