Skip to content

Let the human answer the agent from the board: tick bullets, reply to questions, mid-turn hook - #30

Merged
than merged 11 commits into
mainfrom
interactive-line-updates
Sep 29, 2026
Merged

than merged 11 commits into
mainfrom
interactive-line-updates

Conversation

@than

@than than commented Sep 29, 2026 •

Copy link
Copy Markdown
Owner

The viewer becomes the human's channel back to the agent. The agent owns the board and every move on it; the human says "I did this" and answers questions, and a hook hands both to the agent mid-turn.

What the human does

  • Tick a bullet. Click • and it becomes ✓ (the file gets - [x]); click again to undo. x does the same on the selected item.
  • Answer a question. An indented Ask: line is drawn as ? …. Click it, or press a, and a reply line opens under the item; Enter writes it as an Answer: line, drawn ↳ …. The reply line is pre-filled with the current reply and says type your reply… ⏎ send · esc cancel when empty.
  • Whatever is clickable turns solid under the pointer. Items are spaced one blank line apart. No buttons, no emoji.
  • The viewer never moves an item between sections; filing stays with the agent.
  • u, or a click on the status message, undoes the last change and refuses if the file changed since. Every click reports its outcome in the status bar.
  • Mouse capture is on by default (M toggles, --no-mouse starts without it); Shift-drag (Option in iTerm2) still selects text. Keys: ]/[ select, x ticks, a replies, }/{ jump between open questions.

Delivery

  • sidecar diff --mid-turn runs as a PostToolUse hook and prints hook JSON only when the human replied or ticked; it stays silent for anything else, including the agent's own edits, and advances the snapshot only when it reported.
  • sidecar init installs it beside the per-prompt hook, and the CLAUDE.md note teaches agents the convention.
  • sidecar diff reports edited 🧠: "Title" — replied "…", — ticked, or — unticked.

Writes

Each action re-reads the file, finds the item by section label and exact text, applies one text transform, and swaps the file in by rename, keeping its mode. An agent edit elsewhere survives; if the item itself changed, nothing is written.

Design: docs/superpowers/specs/2026-09-29-interactive-line-updates-design.md.

Testing

go test ./... passes, including a sweep that clicks the bullet and the question at pane widths from 24 to 200. I also drove the real TUI through a PTY: hover repainted the bullet, a click ticked it and u undid it, a click on the question opened the reply under the item, Enter wrote it, and sidecar diff --mid-turn reported both.

Known limits: a click on a link while capture is on may not reach the terminal's own link handling; the mid-turn report advances the snapshot whole.

🤖 Generated with Claude Code

than and others added 2 commits September 29, 2026 14:23
The viewer selects an item with ] and [, then writes one line change back:
x ticks a checklist item, 1-9/y/n/d answer an Ask: prompt, and d moves an
item to Done. M turns on mouse mode for clicks. Writes locate the item by
exact text and swap the file in by rename. sidecar diff names a tick or an
answer so the agent reads the decision on its next turn.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
A click on the status bar row no longer maps to content below the fold.
The note now says what the agent does with an answer, and lists Ask: among
the lines an entry may carry.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

The design is solid overall. editItem re-reads the file and matches by exact text, the atomic rename keeps the watcher happy (the temp file's name doesn't match the board, so the watcher ignores it), and every edit runs on the Update goroutine, so no new concurrency is added. The highlight goes through applyLineBg, which pads to the width rather than past it, and it uses a hex color. go vet and go test -race ./... pass locally. I found two correctness problems worth fixing before merge.

a. The selection follows an index, not an item, so the conflict check is bypassed after a reload

fixItemCursor (ui.go) keeps itemSec/itemIdx as long as those indexes are still in range. itemKey then takes whatever item sits at that index in the new m.board and passes that item's Raw to editItem. That text always matches the file, so errBoardChanged can only fire during the ~100 ms debounce window, before the watcher reloads.

Concrete case: the user presses ] on - [ ] Ship it (Needs you, index 0). The agent then writes - New agent item above it. On reload the cursor stays at index 0, and the tint moves to the agent's new item. The user presses d and sends the agent's item to Done. A section being added or removed above the selection does the same thing across sections.

Fix: store the selected item's section label and Key (or Raw) when it is selected. In fixItemCursor, look them up again in the new board and deselect if they're gone. This is what the PR description ("if the item itself changed, nothing is written") and the spec's known-limits section already assume.

b. writeAtomic replaces a symlinked board with a regular file

The README says "the viewer still opens a symlinked board that already exists." writeAtomic runs os.Stat (which follows the link) to read the mode, but then runs os.Rename(tmp, path) onto the link itself. The first tick therefore turns the symlink into a regular file with the new content. The target (the other checkout's board) never gets the edit, and from then on the two boards have silently diverged.

Fix: resolve the link before writing, e.g. if r, err := filepath.EvalSymlinks(path); err == nil { path = r } at the top of writeAtomic, and create the temp file in filepath.Dir of the resolved path. The watcher still sees an event, because the link path in the watched directory now points at new content. If the target lives in another directory, the 1 s stat fallback catches the change. Alternatively, refuse to edit a symlinked board, as init does.

Minor

  • applyItemHighlight takes a total parameter that it never uses; the only caller passes 0. Drop it.
  • itemKey clears itemSec/itemIdx on d even when the move fails (for example, "already there" on an item that is already in Done). Clear the selection only on success.
  • moveItem removes a section's only item and leaves both blank lines around it (## A / blank / blank / ## B), and it doesn't put back the nothing yet placeholder. The rendered spacing is still compact, but the next sidecar diff will show an empty section rather than the placeholder convention. Consider inserting emptySectionPlaceholder when the source section becomes empty.
  • normalizeItem now gives - [ ] foo and - [x] foo the same Key in one section. Semdiff matching already has to handle duplicate keys, but this makes duplicates a little more likely. Just flagging it.

Once (a) and (b) are addressed, this looks good to me.

@than
than force-pushed the interactive-line-updates branch from d01769b to f23e38d Compare September 29, 2026 18:42
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

The core of this is solid. editItem re-reads the file and matches the exact item before writing, the transforms are pure and tested, and writeAtomic renames within the watched directory. The watcher filters by basename, so the .sidecar-edit-* temp file doesn't cause spurious reloads, and the rename-swap/delete/recreate handling is unchanged. Scroll offset is still kept across reloads. The highlight goes through applyLineBg at renderWidth(), so nothing renders wider than the pane. The status bar hint is truncated through the existing truncateTo path. Mouse mode is off by default. go vet and go test -race ./... pass.

There are two real issues.

a) The selection follows an index, not an item, so an agent edit can make the next keypress act on a different item.
fixItemCursor only checks that (itemSec, itemIdx) is still in range after a reload. Suppose the agent inserts or removes an item above the selected one in the same section. The watcher reloads, and the selection silently moves to a neighbour. The user then presses d or x and acts on an item they didn't pick. The exact-text check in editItem can't catch this, because it.Raw comes from the already-reloaded m.board. That makes it the neighbour's current text, and it matches. This is the exact race the "conflict refusal" is meant to prevent.
Fix: store the selected item's section label and Raw (or Key) when it is selected. In fixItemCursor, look that item up again and drop the selection if it's gone. Then pass the stored Raw to editItem, not the reloaded one. The refusal then covers the item the user actually picked.

b) d (move to Done) leaves a stale highlight.
In itemKey, m.apply(...) calls reload(true) and then compose() while itemSec/itemIdx still point at the old position. After the move, that position usually holds the next item in the section, so the highlight is drawn there. The code then sets m.itemSec, m.itemIdx = -1, -1 but never recomposes. The viewport keeps a tinted item with no selection behind it, and the hint is gone, until some other action recomposes. Fix: clear the selection before calling apply, or call m.clearItemCursor() after it instead of assigning the fields directly.

Nits

  • applyItemHighlight has an unused total int parameter, and compose passes 0 for it. Drop it.
  • apply: use errors.Is(err, errBoardChanged), not ==.
  • humanChange reports an answer being set, but not an Answer: line being removed. That's probably fine, since only the agent removes answers.

@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Overall this is well scoped. The write path is clean: it writes a temp file in the same dir, chmods, renames, and the watcher only matches the basename, so .sidecar-edit-* noise is ignored. The parent-dir watch, scroll preservation (apply goes through reload), and the width cap are all intact. The item highlight goes through applyLineBg, which only pads up to renderWidth(), and the status-bar hint is truncated, so nothing overflows. Everything runs on the Update goroutine, so there are no new races. One real correctness bug and a few smaller items.

Must fix: selection is by index, so it can land on the wrong item after an agent edit

fixItemCursor only bounds-checks itemSec/itemIdx. selected() then reads the item's Raw from the freshly reloaded m.board. So editItem's "board changed under the cursor" guard only works if the watcher reload hasn't landed yet. Once it has, it passes every time.

Scenario: select Ship it with ]. The agent inserts - [ ] Agent inserted above it in the same section. The fileEvent reloads the board, but the index (0,0) is still in range. Now x ticks Agent inserted, and d sends the wrong item to Done. The tint moves too, but that's easy to miss in a pane the agent is rewriting. TestEditRefusedWhenAgentChangedItem passes only because it never delivers a fileEventMsg between the select and the keypress.

Fix: store the selection's identity, e.g. selLabel, selRaw (or Key), when selecting. In fixItemCursor, find it again in the new board by label + Raw. If it isn't found, drop the selection with a notice like "item changed — deselected". Keep passing the stored selRaw as oldRaw to editItem, so the conflict check compares against what the human actually saw. Please add a test that sends fileEventMsg between ] and x.

Should fix

  • a. writeAtomic replaces a symlinked board with a regular file. os.Stat follows the link, but os.Rename(tmp, path) swaps out the link itself. init refuses symlinked boards, but the viewer opens any path. Resolve with filepath.EvalSymlinks(path) before the temp and rename, or refuse to edit when Lstat shows a symlink.
  • b. moveItem into a truly empty section loses the blank line before the next heading. For "## A\n\n- x\n\n## B\n\n## C\n\n- z\n" moved to B, I traced the output by hand as "## A\n\n\n## B\n\n- x\n## C\n…". The item butts against ## C, and A is left with a double blank. It parses fine, which is all TestMoveItemIntoTrulyEmptySection checks. But it degrades the agent's source file on every move and breaks compact spacing. Emit a blank after the block when the next line isn't blank. When the removed block leaves two adjacent blanks, drop one.
  • c. d now shadows the viewport's half-page-down while an item is selected. Someone scrolling with d/u after pressing ] sends an item to Done. That's a write from a key that was a no-op scroll a second ago. Consider requiring D for the move, or at least list the collision in the help text.

Nits

  • The total parameter of applyItemHighlight is unused (always passed 0). Drop it.
  • apply compares err == errBoardChanged. Use errors.Is so a future wrap doesn't silently skip the reload.
  • setAnswer deletes any body line starting with Answer:, not only the one under Ask:. That's fine given the convention, but it could be scoped to the line right after Ask:.
  • normalizeItem now gives - [ ] foo and - foo the same Key, so both in one section collide in semanticDiff. That's an edge case and acceptable, but worth a line in the comment.

than and others added 2 commits September 29, 2026 14:44
a opens a free-text answer in the status bar for any item with an Ask: line;
an Ask: with no options is an open question. } and { jump between unanswered
questions, and the status bar counts how many are waiting.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The board keeps the plain "Ask: ✅ Done | ❌ No" line; the viewer renders each
option as a shaded button, plus a [ ✎ ] button for a free-text answer, and
ticks the recorded answer. In mouse mode a click on a button answers. Letter
keys match an option by its letters, so an emoji option like ✅ Done still
answers to d.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

Overall this is well structured. Each edit is a pure transform, the file is re-read and matched by label and exact Raw, a mismatch is refused, and the tests are good. go vet and go test -race ./... pass. The watcher is unaffected: the temp file sits in the watched directory but is filtered by basename, and the final rename is exactly the swap the watcher already handles. Rendering is unchanged. The highlight uses applyLineBg with a hex color at renderWidth(), so nothing gets wider than the pane. I found two real problems.

a. writeAtomic replaces a symlinked board with a regular file (edit.go:243)

The README says the viewer "still opens a symlinked board that already exists". os.Rename(tmp, path) replaces the link, not its target. The first x, d, or answer in a linked board breaks the link: this checkout gets a standalone copy, and the shared target never sees the edit. That is data divergence with no error shown. os.Stat for the mode already follows the link, so only the rename target is wrong.

Fix: resolve before writing.

if real, err := filepath.EvalSymlinks(path); err == nil {
	path = real
}

Do this at the top of writeAtomic, so the temp file is created next to the real file and the rename stays on one filesystem. Please add a test next to the file-mode test.

b. Selection is kept by index, so a reload can silently retarget the next keypress (ui.go fixItemCursor)

fixItemCursor only clears the selection when (itemSec, itemIdx) is out of range. Say the agent inserts or removes an item above the selected one in the same section. After the reload, the same indices point to a different item. x or d then acts on that item. The conflict check does not catch this, because itemKey passes the new board's Raw to editItem, which matches. So the stated guarantee ("a conflicting one is refused") holds for the file, but not for what the user meant to act on. With the agent writing on every turn, a keypress racing a reload is realistic, not theoretical.

Fix: remember the selected item's Key, or its label plus Raw, and re-find it in fixItemCursor. Clear the selection if it's gone. apply can keep using it.Raw as it does now.

Nits (optional)

  • apply: use errors.Is(err, errBoardChanged) instead of ==.
  • applyItemHighlight has an unused total parameter (called with 0). Please drop it.
  • moveItem leaves the source section's trailing blank line behind. Moving the only item out of a section can leave two blank lines in the file. It renders fine, but it goes against the compact-source spirit. Collapsing one adjacent blank line when the block is removed would fix it.

The known limit (an agent write to the same item inside the read→rename window gets lost) is documented and acceptable for this scope.

@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

The core of this PR is solid. editItem re-reads the board and matches items by exact text. Transforms are pure functions and the file is swapped in by rename. Keeping mouse mode opt-in protects native selection. buttonize keeps the line count the same, so applyCollapse indices stay valid. The watcher is untouched: temp files are named .sidecar-edit-*, so they never match the watched filename, and the rename fires an event that reloads as a no-op. Scroll clamping still goes through reload. The status bar output is still truncated to the pane width. go vet is clean and go test -race ./... passes.

Two correctness problems should be fixed before merge:

a) The selection is kept by index across reloads, so an action can hit the wrong item

fixItemCursor (ui.go) only checks that itemSec/itemIdx are still in range. When the agent inserts or removes an item above the selection, the same index now points at a different item after the reload. selected() returns that item. Its Raw matches the disk, so editItem's conflict guard passes and the write lands on the wrong item.

It is most likely during free-text typing (a), which takes seconds, and the agent may edit the board in that time. If n, x or d arrives just after a reload, it has the same problem. If the selection drops out of range instead, enter in typeKey silently discards the typed answer and shows no notice.

The errBoardChanged guard only covers the race between m.board and the disk. It does not cover the selection drifting across reloads.

Fix: remember the selected item's identity (section label + Raw, or Key) and look it up again in fixItemCursor after each reload. Clear the selection if it's gone. In typeKey, show a notice such as "item changed — answer not written" instead of dropping the text silently. A test would be: select "Pick a path", rewrite the file with a new item above it, send fileEventMsg, press n, and assert that the answer went to "Pick a path".

b) writeAtomic replaces a symlinked board with a regular file

The README says the viewer still opens a symlinked board that already exists. os.Rename(tmp, path) replaces the link itself. After the first tick, this checkout has a private copy of the board, the shared target no longer gets the viewer's writes, and the other checkout never sees the answer. (os.Stat follows the link, so the mode is copied from the target, which hides the problem.)

Fix: resolve the path first:

if real, err := filepath.EvalSymlinks(path); err == nil {
	path = real
}

Then create the temp file in filepath.Dir(path) of the resolved path. Alternatively, refuse writes to symlinked boards the way init does. Either is fine, as long as the link isn't silently broken.

Minor

  • applyItemHighlight takes a total parameter that is never used, and the call site passes 0. Remove it.
  • In mouse mode, chipAt looks for chip text on one rendered line only. When glamour wraps a long Ask: line in a narrow pane, a chip split across two lines can't be clicked. The keys still work, so this is fine to leave, but consider documenting it next to the collapsed-section limit in the design doc.

Rendering style has no regressions: chips are inline code spans with the existing hex Code style, nothing new uses a 256-color palette, and the item highlight uses applyLineBg at renderWidth(), so it never goes past the pane width.

than and others added 2 commits September 29, 2026 15:00
Every item outside Done and Shipped gets [ ✅ Done ] and [ 💬 Reply ] below it.
Reply takes free text, pre-filled with the current reply, and lands as an
Answer: line; an agent's narrative Ask: line shows as the question. Mouse
capture is on by default (--no-mouse and M release it). A new PostToolUse hook,
sidecar diff --mid-turn, hands a reply to the agent after its next tool call.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@than than changed the title Act on board items from the viewer: tick, answer, send to Done Let the human answer the agent from the board: Done and Reply buttons, mid-turn hook Sep 29, 2026
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

This is a solid feature overall. The edit path re-reads the file, matches the item by its exact text, and writes a temp file then renames it, which is the right design. The watcher ignores the .sidecar-edit-* temp files because it filters on the base name. go vet and go test ./... pass. I found a few correctness problems worth fixing before merge.

a. The selection is stored by index, so a reply or Done can land on the wrong item (high)

itemSec/itemIdx are positions. After a reload, fixItemCursor (ui.go) only checks that they are still in range. selected() then reads whatever item now sits at that position, and editItem matches it by that item's current Raw. So the exact-text check always passes, and the "conflicting edit is refused" guarantee never applies to the UI.

Scenario: the human clicks Reply on item 3 and starts typing. The agent inserts a new item above it and the watcher reloads. The human presses Enter, and the Answer: is written under a different item. The same thing happens with d, x, and 1–9 after any agent edit that shifts items.

Fix: when an item is selected, store its section label and Raw (or Key). On each reload, look it up again by that identity. If it's gone, clear the selection and show a notice. When typing is in progress and the item disappears, keep m.input and say so. Right now typeKey's Enter drops the typed reply silently when selected() fails.

b. writeAtomic replaces a symlinked board with a regular file (high)

The README says "the viewer still opens a symlinked board that already exists." os.Rename(tmp, path) replaces the link itself, so the first tick or reply detaches the board from its target. The other checkout keeps the old file and the two boards silently diverge.

Fix: resolve the path first (if r, err := filepath.EvalSymlinks(path); err == nil { path = r }) and create the temp file in the resolved directory.

c. Clicks during typing change what Enter answers (medium)

Update handles tea.MouseMsg before checking m.typing. While typing, a click on another item changes the selection, and Enter writes the draft there. A click on Done or a choice button runs apply with a draft still open. Ignore left clicks (or treat them as cancel) while m.typing is true.

d. The mid-turn snapshot advances past changes it didn't report (medium)

midTurnReport calls writeSnapshot(snap, raw) with the whole file but reports only the reply and tick lines. Any other change since the last prompt drops out of the per-prompt diff for good. That includes a hand edit the human made in an editor, an item they moved, or text they added. The doc comment says "Anything else … leaves the snapshot alone, so the per-prompt diff still reports it", which is only true when nothing was reported at all.

The PR lists this as a known limit, but it undercuts what sidecar diff is for. A better fix is to write prev with only the reported items' lines swapped for their new versions. Or keep a separate "delivered answers" record, so the answers aren't delivered twice, and leave previous.md to the per-prompt hook.

e. Mouse capture on by default reverses a documented guarantee (design, your call)

The old README and comment said sidecar doesn't capture the mouse so clickable links keep working. Bare URLs as clickable OSC 8 links are a stated rendering requirement. With capture on by default, clicking a link does nothing in several terminals, as the PR admits. Shift-drag is a workaround for selecting text, but it doesn't bring link clicks back everywhere.

Consider defaulting to off, with --mouse or M to turn it on, or at least make a deliberate call. Either way, the header comment in interact.go ("mouse clicks work only while mouse mode (M) is on, so native text selection and clickable links stay the default") and the mouseClick doc ("Only wired while mouse mode is on") are now stale.

Minor

  • apply: use errors.Is(err, errBoardChanged) rather than ==.
  • moveItem removes only the item's lines. If items are separated by blank lines, the source section ends up with two blank lines in a row in the file. Glamour collapses them when rendering, so this is cosmetic, but the file drifts from the compact style.
  • buttonize counts on glamour keeping a row of chips on one line. At narrow widths a long Ask: a | b | c row wraps inside a chip. styleButtons and chipAt then stop matching it, so those buttons can't be clicked. Nothing becomes wider than the pane, so this is not a regression, but it's worth a test at width≈30.

Aside from (a)–(d), the width handling, the hex colors, and URL handling look unchanged.

@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

This is a well-built feature. Edits re-read the board, match the item by exact text, refuse when the item changed, and swap the file in by rename. The watcher still only reacts to the board's own filename, so the .sidecar-edit-* temp file is ignored and the rename is handled like any agent write. Reload still goes through reload(true), so scroll is kept. go test ./... passes. The problems I found are below.

a. The button restyle can make a line wider than the pane (hard requirement)
styleButtons rebuilds each button-only line as indent + strings.Join(parts, " "). The markdown from codeSpans joins buttons with one space, and the Code style in style.go adds no padding, so glamour wrapped that line with 1-column gaps. The restyle adds one column for every gap. An Ask: a | b | c | … row that glamour filled to exactly the pane width comes back wider than the pane. That brings back the wrap / fake-double-space bug. The fix is to keep the separator glamour used: join with " ", or restyle each button where it already sits instead of rebuilding the line. Please also add a test that renders an Ask row with many options at several widths and asserts visibleWidth(line) <= width. No existing test covers this.

b. writeAtomic replaces a symlinked board with a regular file
The README says the viewer "still opens a symlinked board that already exists." The first Done, Reply, or tick from the viewer calls os.Rename(tmp, path) on the link, which cuts the board loose from its target without any message. Resolve the path first with filepath.EvalSymlinks(path), then create the temp file next to the resolved path and rename onto it.

c. The Done button is misread when an Ask option is also "✅ Done"
chipAt tries the option buttons first and uses strings.Index on the whole line. With Ask: ✅ Done | ❌ Hold, the option button text [ ✅ Done ] is the same as doneChip. A click on the Done button in the action row then records Answer: ✅ Done as a choice and doesn't move the item. The normalizeOption comment uses "✅ Done" as its own example, so this case is expected. Only test the option buttons when the clicked line is the Ask row, or match the action row separately. The same collision would happen with an option named 💬 Reply.

d. --mid-turn reports the agent's own ticks as the human's
midTurnReport treats any — ticked / — unticked in semanticDiff output as something the human did. An agent ticking its own - [ ] subtasks will fire the hook and get told "The human answered on the sidecar board while you worked". The report also moves the snapshot forward over the whole file, so any other agent edits made in that window drop out of the next per-prompt diff. The design doc lists that snapshot behavior as a known limit, but the false trigger makes it happen much more often. Options: report only replied mid-turn, or report ticks only when the viewer made the change. For example, the viewer could write a marker, or the report could skip ticks the agent made itself. Matching on substrings of the formatted diff output is also fragile: a title that contains — replied matches. Having semanticDiff/humanChange return a structured flag would be sturdier.

e. Stale comment
The header comment in interact.go still says "mouse clicks work only while mouse mode (M) is on, so native text selection and clickable links stay the default." This PR makes capture the default, so the comment now says the opposite of the code.

Minor

  • midTurnReport throws away the json.Marshal error. It can't fail with these types, so that's fine, but a comment saying so would help.
  • applyItemHighlight has an unused total parameter (the caller passes 0).

Overall this is close to mergeable. Items a and b need fixing first; c and d are worth fixing in this PR too.

Buttons rest as grey blocks and turn solid under the pointer. They are drawn
after rendering, on their own line, so glamour can neither fuse a row onto an
item's last line nor wrap one mid-label; a test now clicks every button at
pane widths from 24 to 200. Reply is typed under its item. u, or a click on the
status message, undoes the last write, and refuses if the file changed since.
Every click reports its outcome in the status bar.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

The feature holds together, and go vet plus go test -race ./... pass. Writing edits as a pure text transform on the file as it is on disk right now, then swapping it in by rename, is the right design. The watcher is untouched, so rename-swap, delete, and recreate handling doesn't regress. A few correctness problems are worth fixing before merge.

Selection is kept by index, so a reply can land on the wrong item (high)

fixItemCursor (ui.go) only drops the selection when itemSec/itemIdx go out of range. Suppose the agent inserts or removes an item above the selected one while the human is typing a reply, or before they press d/x/1-9. The same indices then point at a different item. apply passes that item's current Raw to editItem, which finds it, so the reply or the Done move goes to the wrong entry with no error. The errBoardChanged guard can't catch this because the stale identity was never kept.

The mouse path has the same problem: a click on another item while typing is true re-points the selection (mouseClick sets itemSec/itemIdx without leaving typing mode), and Enter then sends the half-typed reply to the clicked item.

Fix: keep the selected item's (section label, Key or Raw) and resolve it again in fixItemCursor after every reload, clearing the selection (and abandoning the typing) when it no longer matches. Also cancel typing, or ignore clicks, while m.typing is set.

A button row can be wider than the pane

packButtons always places the first chip on a row, even when indent + visibleWidth(chip) > width. An Ask: option longer than the pane (e.g. Ask: Ship it to production tonight | Hold until Monday's review in a ~30-col pane) produces a line wider than the pane. That brings back the wrap bug the tool exists to prevent. [ 💬 Edit reply ] plus the indent is also 20 cols, and nothing guards it at very narrow widths.

Fix: truncate a chip's label with … to width - len(indent) before placing it. chipAt/chipTextAt must then match on the truncated text, so build the drawn chip strings once and reuse them for hit-testing. Add a width test with a long option, like the other "never wider than the pane" tests.

Viewer writes replace a symlinked board with a regular file

The README says the viewer "still opens a symlinked board that already exists". writeAtomic renames a temp file onto path, which replaces the symlink itself. The first Reply or Done silently unlinks the board from the shared target, and the watcher keeps showing the now-forked copy. Fix: filepath.EvalSymlinks(path) at the start of writeAtomic (and create the temp file in the resolved file's directory), or refuse to write and say so.

A stray click on the status bar undoes the last reply

In mouseClick, y == m.vp.Height && len(m.undo) > 0 undoes whenever the undo stack is non-empty, not only while the "u to undo" notice is showing. The stack is never cleared on a successful write-free reload, so a click on the status bar minutes later reverts the human's reply. This only fails safe if the agent has written since. Fix: allow the click-to-undo only while the notice from that write is still displayed (e.g. keep an undoable bool that is cleared when notice changes).

--mid-turn also advances the snapshot past everything it didn't report

midTurnReport filters the report down to replies and ticks but then calls writeSnapshot(snap, raw) with the whole file. Any other change in that same window, such as a hand edit the human made in their editor or an item they added, is then gone from the next per-prompt sidecar diff. The function's own comment says "Anything else stays silent and leaves the snapshot alone, so the per-prompt diff still reports it", which is only true when nothing was reported. The PR lists this as a known limit, but it's a silent loss on the main hook path. A cheap fix: write a snapshot that is prev with only the reported items' new text applied (editItem-style replacement by Key). Otherwise at least fix the comment so it doesn't promise that.

Smaller issues

  • apply can record a stale undo entry. apply reads before in a separate os.ReadFile from the one editItem does. If the agent writes between the two reads, the undo entry's before is wrong, and undo would restore content the edit never started from. Have editItem return the raw it read and the next it wrote, and use those for the undo entry.
  • Errors are dropped. before, _ := os.ReadFile(...) / after, _ := ... in apply and out, _ := json.Marshal(...) in midTurnReport ignore their errors. The first two go on to feed undo.
  • pendingQuestions and moveQuestionCursor disagree. pendingQuestions skips Done/Shipped, but moveQuestionCursor doesn't. With Done expanded, } can land on an item the "N awaiting you" count excludes. Use actionable(...) in both.
  • Debug text is left in. m.notice = fmt.Sprintf("click %d,%d", ...) ("proof the click arrived") and the click X,Y — … notices look like debugging output that reached user-facing text.
  • Dead code. In typingRow, _ = it is dead; use if _, _, sel := m.selected(); sel.

Rendering style looks fine: all new colors are hex, the button rows add no blank lines, and bare URLs aren't touched.

…ound

A blank line now separates every item, buttons rest as grey text and get a
background only under the pointer, and the reply line says how to send it.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

Overall this is well built. Edits are located by section label plus exact item text and swapped in by rename, and a conflicting edit is refused. The watcher is untouched: it filters on basename, so the .sidecar-edit-* temp files never trigger a reload. Mouse-wheel events still reach vp.Update. --mid-turn advances the snapshot only when it reports something. A few things are worth fixing before merge.

Must fix: option buttons can exceed the pane width

packButtons (buttons.go) wraps between chips but never shortens a single chip. When one chip is wider than width - 2, the row comes out wider than the pane. Choice text comes straight from the agent's Ask: line, so this is easy to hit. For example, Ask: Ship it to production right now please | Hold at a 30-column pane gives a 45-column row (" [ Ship it to production right now please ]"). Nothing downstream clips it (composeMarked doesn't truncate), so this brings back the wrap bug the README forbids. Fix: in packButtons, truncate the label to width - indent - 4 with an ellipsis before styling. chipAt and chipTextAt match on the chip text, so run the same truncation there, or keep a list of display strings alongside the options. Please add a narrow-width test with a long option.

Should fix

  • Undo can drop an agent edit. apply reads before, then editItem reads the file again, writes, and apply reads after. If the agent writes in either gap, the undo entry holds the wrong snapshot, and u quietly restores a file without the agent's change. The cur == e.after guard doesn't catch this. The window is small, but the fix is too: have editItem return the raw it read and the next it wrote, and store those as the undo entry.
  • Writing through a symlink replaces the link. init refuses symlinked boards, but sidecar notes.md can open any file. If that file is a symlink (dotfiles, synced notes), the first Done, Reply, or tick renames a regular file over the link and detaches it from its target. Resolve with filepath.EvalSymlinks before CreateTemp/Rename, or refuse the edit with a notice.
  • Duplicated helper. writeAtomic (edit.go) nearly duplicates writeFileAtomic (diffcmd.go); only the mode handling differs. Add a mode parameter to one of them and drop the other.
  • Leftover debug notice. Update sets m.notice = "click X,Y" on every left press. The header-collapse path in mouseClick never overwrites it, so clicking a header leaves · click 12,4 in the status bar. Drop the pre-set, and set a notice only on the paths where nothing happened, which mouseClick already does.

Minor

  • midTurnReport ignores the json.Marshal error. It can't fail for this map, so that's fine, but writeSnapshot failing after the JSON is already printed means the next tool call delivers the same reply again. That's acceptable if you want it to fail open; a comment saying so would help.
  • The mid-turn filter matches the substrings " — replied ", " — ticked", and " — unticked" in the formatted semanticDiff lines, so an item title containing one of them would be picked up by mistake. Having humanChange return a flag, or filtering on the structured items, would be more robust. This is low priority.
  • Any click on the status-bar row runs undo whenever the undo stack is non-empty, even if the message has already changed to something else. It's documented, but that's a write triggered by a stray click. Consider acting only while the notice still contains u to undo.

Spacing (one inserted blank, only when the next line isn't blank), hex-only colors, scroll preservation through reload, and the watcher all look fine to me.

…estion inline

Drops the button rows, the emoji, the choice options, and every viewer-side
move between sections — the agent owns the board's motion. A click on a bullet
turns it into ✓ (- [x]) and back; an Ask: line shows as a ? question that opens
a reply under the item; the reply shows as ↳. Whatever is clickable turns solid
under the pointer, and items are spaced one blank line apart.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@than than changed the title Let the human answer the agent from the board: Done and Reply buttons, mid-turn hook Let the human answer the agent from the board: tick bullets, reply to questions, mid-turn hook Sep 29, 2026
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

Scope is large but coherent. go vet is clean and go test -race ./... passes. The watcher is untouched: writeAtomic renames into the watched directory, and the watcher already handles that. Scroll offset still goes through reload, and the width math in typingRow stays inside renderWidth. Colors are all hex. A few real problems:

a. Undo can clobber the agent's edit (TOCTOU in apply)

interact.go apply() reads before, then editItem reads the file again, writes it, and then apply reads after. If the agent writes in either gap:

  • If the agent writes between before and editItem, then before is the pre-agent file. Undo passes its cur == e.after check and restores before, which silently reverts the agent's write.
  • If the agent writes between writeAtomic and the after read, then after includes the agent's edit, and undo again drops it.

The undo guard exists precisely to prevent this. Fix: have editItem return the (before, after) it actually read and wrote, and drop the two extra os.ReadFiles. That also removes the swallowed before, _ := / after, _ := errors.

b. u changes meaning depending on hidden state

u is the viewport's half-page-up. With this PR it becomes "undo" whenever m.undo is non-empty, and the stack never expires; it is only cleared on a failed undo. Say someone ticks something, scrolls around for ten minutes, then presses u to page up: that silently reverts the old tick. It's written to disk, and the mid-turn hook may already have reported it. Clicking anywhere on the status bar has the same problem (mouseClick: y == m.vp.Height && len(m.undo) > 0), long after the "u to undo" notice is gone.

Fix: only honor undo while the notice offering it is showing, e.g. keep an undoArmed flag that is cleared whenever notice is cleared. Otherwise, use a key that doesn't collide with scrolling.

c. Mid-turn hook treats any - [x] as the human's

midTurnReport reports every tick and tells the agent "The human answered on the sidecar board…". - [x] is ordinary markdown, and agents write it on their own, especially when marking something done. The same applies to an agent that writes an Answer: line. The hook then feeds the agent its own edit back as a human decision and advances the snapshot, so the per-prompt diff never shows it either. The CLAUDE.md note says "read that as 'I did this'", which makes this worse.

It can't be fully solved without provenance. A cheap improvement: have the viewer record what it wrote, e.g. keep the last viewer-written content or hash in .sidecar/, and have --mid-turn report only when the current file matches a viewer write. At minimum, tell agents in the note never to write [x] or Answer: themselves.

d. The mid-turn filter is string matching on formatted output

strings.Contains(l, " — replied ") / " — ticked" also matches an item whose title contains those strings, and any future wording change breaks it silently. humanChange already computes the facts. Have semanticDiff (or a small sibling) expose them as a bool or struct instead of re-parsing the prose.

e. Smaller items

  • Stale hover after reload: m.hover.line survives reload/rerenderCollapse. After the file changes, paintHover can bold or underline an unrelated line, or recolor the first • on it. Reset m.hover in reload, the way toggleMouse does.
  • Wrapped questions: colorMarks/isQuestionLine only recognize the first rendered line of a wrapped Ask:. Continuation lines aren't colored, hover-painted, or clickable. stripANSI + Render also drops any OSC 8 hyperlink inside a question or answer line.
  • Leftover debug notices: "click %d,%d", "click %d,%d — the status bar is not clickable" and "click %d,%d — nothing to click on that line" read as debug output. A plain message, or none, would be better.
  • Shadowed type name: runViewer shadows the type model with model := newModel(...). Rename it to m.
  • reload baseline parse: reload parses m.prevBaseline twice (once in the if for displayText, again as baseBoard, _ :=). Hoist it.
  • Stale comment in main.go: it mentions "buttons, checkboxes", but the PR deliberately has no buttons.
  • Manual-install snippet: in writeReconcileHook's "unexpected shape" branch, the printed snippet covers only the UserPromptSubmit entry, so a manual install never gets the PostToolUse hook.

Items a–c should be fixed before merge. Item a is a correctness bug in exactly the property the undo guard promises, and b and c can put wrong state on disk or in the agent's context without the user noticing.

… move items with j/k

The question and reply now draw as indented blocks that wrap with a hanging
indent in one color; a click or hover on any of their lines counts. The bullet
target is four cells wide and paints as such under the pointer. j and k move
between items (and scroll a board with none), space ticks, enter replies, and
tab returns to moving between sections.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

go vet and go test ./... both pass. The watcher is untouched, and viewer writes go through a same-directory rename, so rename-swap handling is unaffected. Scroll offset still survives reloads. The new colors are all hex. The feature works, but a few correctness problems should be fixed before merge.

a) A reply can land on the wrong item (the worst one)

The selection is stored only as (itemSec, itemIdx). After a reload, fixItemCursor only bounds-checks it. typeKey then resolves the target with m.selected() when Enter is pressed. Example:

  1. The human opens a reply on item A (index 0) and starts typing.
  2. The agent inserts item B above A in the same section. The watcher reloads, and index 0 is now B.
  3. On Enter, editItem is called with B's Raw. It matches, so Answer: … is written under B, and the mid-turn hook reports it as B's answer.

editItem's "item changed, nothing written" check is bypassed, because the caller passes whatever item is at that index now. It doesn't pass the item the human started typing on. The same drift affects space/x on a keyboard selection.

Fix: When you select an item or open a reply, capture label and it.Raw. Apply against that captured pair. After a reload, look the item up again by Raw, or drop the selection and set typing = false. A missing item then gets the existing errBoardChanged path.

b) reflow can produce lines wider than the pane

reflow never breaks a word. The first word on each row is always appended (fresh), so any token longer than width-4 produces an over-wide line. A bare URL is the usual case, and the board style tells agents to use them. Example: Ask: see https://github.com/…/pull/30/files/<long sha> at a 40-col pane gives a row of about 90 cells. That brings back the terminal line-wrap / fake-double-spacing bug.

It also regresses URL rendering. rewriteMarks replaces glamour's output for Ask/Answer lines, so a URL there loses the OSC 8 link and the pane-fit shortening the README describes.

Fix: Hard-break over-long tokens with ansi.Hardwrap/ansi.Wrap on the joined text. Or keep glamour's rendered row content and only re-indent and re-color it. Add a width-sweep assertion (visibleWidth(l) <= width) to TestControlsWorkAtEveryWidth using a question that contains a long URL.

Smaller: at very narrow widths, typingRow uses room := max(4, width-8), so for width < 11 the row is wider than width.

c) writeAtomic replaces a symlinked board with a regular file

The README says the viewer still opens a symlinked board. But os.Rename(tmp, path) replaces the link itself, so the first tick breaks the link. The other checkout then silently stops seeing the board. The temp file is also created next to the link, not next to the target.

Fix: At the top of writeAtomic, resolve path with filepath.EvalSymlinks and write to the resolved path. There's also already a writeFileAtomic in diffcmd.go. Consider giving it a mode parameter and reusing it instead of adding a near-copy.

d) Undo can erase an agent edit

In apply, before and after are read with separate os.ReadFile calls outside editItem, and the errors are discarded.

  • If the agent writes between the before read and editItem's own read, before is missing that edit.
  • If the agent writes between the rename and the after read, after already contains it.

In both cases undoLast's cur == e.after guard passes, and restoring before drops the agent's write. This defeats the "undo never overwrites the agent" guarantee.

Fix: Have editItem return the raw it parsed and the next it wrote, and record those.

e) Mid-turn hook: minor

  • Claude Code can run PostToolUse hooks for parallel tool calls at the same time. Two diff --mid-turn runs can both read the old snapshot and deliver the same reply twice. That's probably acceptable, but it's worth a line under "Known limits".
  • If writeSnapshot fails, the hook's 2>/dev/null hides the error. The same reply is then re-sent after every tool call.
  • midTurnReport filters on the substrings " — replied " / " — ticked". An item title containing — ticked would match, and em dashes are common in this board's style. Having humanChange feed the filter directly (for example, a bool from semanticDiff) would be more robust than matching the rendered text.

Nits

  • reload parses m.prevBaseline twice: once for baseDisplay and again for baseBoard.
  • While the reply row is inserted in compose, lines below it are shifted by one against m.changed, headerLines and itemStarts. The change marks and cursor tint below the reply are off by one until typing ends. This is cosmetic.

Items a–c are the ones I'd block on. The rest is solid, and the tests cover it well.

…ing it

The viewer finds the OSC 8 link under the pointer in the rendered line, paints
it on hover, shows its full address in the status bar, and opens it on a click.
Only http, https, and mailto open. Clicks are held while a reply is being typed.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@claude

claude Bot commented Sep 29, 2026

Copy link
Copy Markdown

Review

The core is solid. editItem re-reads the file, matches the item by section and exact Raw, and swaps it in by atomic rename. That fits the parent-dir watcher (a rename-swap is exactly what it already handles), and all file IO happens inside Update, so there's no new concurrency. Scroll offset is still restored with SetYOffset(offset) after reload. normalizeItem ignoring the task marker means ticking reads as an edit, which is correct. The hook install is safe: mergeReconcileHook already strips sentinel entries from every event, so re-running init won't stack PostToolUse hooks. Colors are all hex.

A few real problems.

a) reflow can render wider than the pane (hard requirement)

marks.go reflow never splits a word, so if a single word is longer than the width, the row overflows. This is common in practice: a URL on an Ask: or Answer: line, e.g. Ask: does https://github.com/than/sidecar/pull/30/files#diff-… look right? at width 40, gives a row of about 70 columns. That brings back the wrap and fake double spacing the tool exists to prevent. The main renderer already hard-splits long URLs at the wrap point (style.go ~L569), so reflow needs to do the same, or cut each row with ansi.Truncate/hard-wrap to width. The width sweep in the tests only uses short question text, so it doesn't catch this. Add a case with a long URL.

b) URLs in Ask/Answer lines lose their link, and mouse capture is now on by default

reflow rebuilds the line from plainMark(askText(it)), which is plain text, so the OSC 8 wrapper that style.go put on a bare URL is gone. linkSpans only finds OSC 8 spans, and with capture on the terminal's own URL detection doesn't get the click. The result is that a link in a question can't be clicked at all in the default mode. Either keep the hyperlinks when reflowing, or don't reflow lines that contain a URL.

c) markBlock breaks on inline markdown other than ` / *

plainMark only strips backticks and asterisks. For Ask: is [the doc](https://…) and _this_ right …, glamour's rendered words don't match the raw words. markBlock then stops at the first line, so reflow replaces only that line with the raw markdown, and glamour's wrapped continuation lines are still there beneath it. The text shows up twice and garbled. At minimum, when the words don't all match, leave glamour's output alone rather than swapping a partial block.

d) A click anywhere on the status bar undoes

if y == m.vp.Height && len(m.undo) > 0 {
    m.undoLast()

This fires on any click on that row, whatever it currently says ("opened https://…", "3 awaiting you", a hint, and so on). One stray click silently reverts a reply the agent may already have received through the mid-turn hook. Tie it to what's showing, e.g. only when m.notice ends in u to undo, and ideally only over those columns. It also runs before the m.typing check. Clicking the bar while typing undoes and clears itemSec, then Enter finds selected() false and the typed reply is dropped with no message.

e) --mid-turn advancing the whole snapshot loses other changes

The PR lists this as a known limit, but the effect is worse than it sounds. Any non-reply change sitting in the same window gets folded into the snapshot and never shows up in the per-prompt diff. That includes the human moving an item in their editor and agent edits from another session. Since you already have prev/raw, a cheap fix is to write a snapshot that is prev with only the reported items' lines applied. If that's too much for this PR, add a comment at the writeSnapshot call saying it's deliberate.

Small: the substring filter in midTurnReport (" — ticked", etc.) also matches an item whose title happens to contain that text. Having humanChange return a flag, instead of re-parsing the formatted strings, would be more robust.

f) Leftover debug notice

m.notice = fmt.Sprintf("click %d,%d", …) // proof the click arrived and the "click %d,%d — the status bar is not clickable" message read as debugging output. Every real outcome already sets its own notice, so these can go.

Everything else (tick/undo guard against later agent writes, setAnswer placement, the file mode kept by writeAtomic) looks right. Fix (a) before merging. (b) through (d) are real UX bugs in new code.

@than
than merged commit 655af39 into main Sep 29, 2026
1 check passed
@than
than deleted the interactive-line-updates branch September 29, 2026 21:28
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