[allenhutchison]

Human overview

P2 of planning/tui-room-todos-notes.md: in the TUI's Todos pane, space/x marks a todo done, d dismisses, u reopens and n adds one (title, optional due date, and in a room an optional member to ask). Works on a room's list, on your own list, and on rows in your rooms' sections of your own Todos.

Closes #171. Refs #157.

Human required (optional)

• Decide the one departure below: a room's add takes a bare day only, as todos create --room --due does.

AI reviewer

• due.py (new): parse_due moved verbatim out of cli.py (call sites renamed); state.py imports it, not cli.
• state.py: todo_owner (room-section rows write with their room's group_id), apply_todo/drop_todo/placeholder_todo (returned TodoEntry replaces the pending row; an add shows a pending:N placeholder), parse_add_due, assignable_members (no Pepper, agents or yourself), reselect (cursor stays at its place after a row leaves the list), TITLE_MAX = 500. AppPaneState.pending holds ids with a write in flight.
• app.py: new one-at-a-time todo loop through _start -> worker -> _post. Success applies the answer, bumps the apps generation (a read already in flight cannot bring the old row back) and re-reads. Failure drops the pending mark (and an add's placeholder only) and calls _action_failed. n runs title, due date, then (room only) a PickerScreen over the roster; Esc at any step cancels. Own Todos always adds to your own list.
• apps_pane.py: bindings (Todos only), … pending mark. screens.py: PromptScreen gains max_length and allow_blank.
• Departure from the doc's wording: §6.7 says the due date is parsed by _parse_due. For a room's add I followed what todos create --room --due actually does (bare YYYY-MM-DD, sent as due), since the CLI rejects a time with --room; your own list takes both forms via parse_due. The member picker reads the room roster (group_roster, the same one @-mentions and --assignee use), cached per room, rather than the room-info card.
• Look hardest at: _swap/apply_todo when a poll lands between a write and its answer; the pending: prefix check in _todo_failed (an earlier draft dropped the real row on failure; there is a test).
• Verification: ruff format --check, ruff check, ty check clean; uv run pytest --cov 2685 passed, 2 skipped, total coverage 95.2% (floor 88). One snapshot (test_your_own_todos_with_a_room_section_and_the_reply_strip) updated: the footer now lists Done/Dismiss/Reopen/Add. Rebased onto origin/main after fix(tui): dropped DM turn takes the reply after its own user row #170 merged. The /simplify and code-review self-review passes were not run.
• SKILL.md documents no TUI keys, so only README changed.

Checklist (repo-specific)

• uv run ruff format --check and uv run ruff check clean (no new lint ignores)
• uv run ty check src/pepper_cli/ clean
• uv run pytest passes; coverage floor in .claude/maintainerd.json not lowered
• Server-contract changes: no new routes or wire fields; existing PepperClient methods only
• No token or credential is logged, printed, or put in a fixture
• README updated; bundled SKILL.md has no TUI keys, so unchanged

🤖 Generated with Claude Code

https://claude.ai/code/session_01QNDeh8M8jKfHhtWGJwkxTm

Confidence Score: 5/5

The PR appears safe to merge; all three previous findings are addressed.

Summary

Adds done, dismiss, reopen, and add actions to the Todos pane for personal and room lists.

• The Todos pane lets people update and add their own and room todos.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Todo action] --> B{Request already running?}
  B -->|Yes| C[Show notice]
  B -->|No| D[Mark row pending]
  D --> E[Send write]
  E -->|Success| F[Apply returned todo]
  F --> G[Read list again]
  E -->|Failure| H[Clear pending mark and show error]
  H --> I[Remove placeholder only for failed adds]

Loading

_{Reviews (2) · Last reviewed commit: "fix(tui): no member picker while your ow..." · Reviewed by Greptile}

[vycari-review]

Vycari Review (shadow)

Shadow round 2 on 8c54c81. This comment is non-gating and needs no response: it is collected to measure agreement with the current reviewer. The round's verdict is the Vycari Review (shadow) check run.

Open: P0 0 · P1 4 · P2 3 · confirmed fixed 2 · withdrawn 0

Judge's notes

Still open: a lost save reply is shown as a failure and never re-read, so retrying can make a duplicate, and a room todo is dropped silently if the pane closes while the member list loads. New: a save answered while a pop-up is open leaves the row stuck pending, and a re-read during an add shows the new todo twice. The one-at-a-time problems from last round are fixed.

P1 · VR-8a24c434 · A todo write answered while a prompt is open is dropped: the row stays pending and the Todos pane stops re-reading

src/pepper_cli/tui/app.py lines 2227–2238 · symbol PepperApp._todo_written · lens fail-open · status new

Every worker result goes through _start -> _delivered/_failed -> _finish. _finish clears the in-flight token, then returns False when self._mounted() is False. _mounted() checks self.query(Sidebar). Textual's App.children is only the active screen (textual/app.py:993-999), so while a ModalScreen is active (the PromptScreen for the add's title or due date, or the PickerScreen for the member) query(Sidebar) is empty and _mounted() is False. The todo write's deliver/on_error callbacks (_todo_written, _todo_failed) therefore never run. _todo_written is the only place that calls pane.pending.discard(...), so the row keeps its '…' mark and pending id forever. _todo_verb returns silently for any id still in pane.pending, so the row cannot be retried either. Because _delivered/_failed return before _schedule, the 'apps' loop that re-reads the pane is also not re-armed by that result. The PR makes this reachable: the add flow and n open modals on the Todos pane, and nothing stops a user from pressing space and then n while the first write is still in flight.

Evidence: app.py:405-412 (_finish clears in_flight then returns False if not _mounted()); app.py:424-471 (_delivered/_failed return before deliver/on_error and before _schedule when _finish is False); app.py:2001-2014 and 2227-2238 (todo write and apps read go through this path); textual/app.py:993-999 (App.children is the active screen only, so query(Sidebar) is empty under a ModalScreen); app.py:2202-2205 (_todo_verb returns while todo.id in pane.pending); app.py:2255-2264 (n opens PromptScreen with no check for an in-flight write other than the busy flag, which is already cleared by _finish). Not checked: whether a Sidebar-scoped query has any other path; the test in the pilot harness would confirm it.

Failure scenario

In a room's Todos pane, the user presses space on 'Book the dog sitter'. The POST is slow. The user presses n, which opens the 'New todo' PromptScreen. The POST succeeds on the server while the prompt is open. _finish returns False because the active screen is the modal, so _todo_written never runs. The user presses Esc to close the prompt. The row still shows '… Book the dog sitter', the header still counts it as open, and it does not change until the pane is closed and reopened. Pressing space on it again does nothing (todo.id is in pane.pending). Pressing r re-reads, but the stale pending mark stays. If the write had failed instead, no notice appears and the mark stays. The same drop applies to an add's placeholder, which then stays as a permanent 'pending:N' row kept by keep_placeholders.

How this was verified (traced-callers)

Read _finish and _mounted. n checks the busy flag, which stays set until _finish runs, so the add prompt cannot open during a write. Another modal opened during the write can still trigger the drop. Reachable from: TUI Todos pane: a write in flight while a modal screen opens.

Proving test

Textual pilot test on room_server(): set server.gates['POST {ROOM_TODOS}/t-sitter/done'] to a threading.Event; open_todos; press space; wait until '… Book the dog sitter' is shown; press n and wait for PromptScreen; set gate; wait until the server has recorded the write (len(server.todo_writes)==1) and a short pause; press Escape to close the prompt; then await until 'Todos  2 open' is in pane_text(app). On the PR head the assertion fails: the pane still shows '… Book the dog sitter' and 'Todos  3 open'.

P1 · VR-c8b9e9a0 · A poll double-counts a committed add awaiting its response

src/pepper_cli/tui/state.py lines 1391–1394 · symbol keep_placeholders · lens concurrency-time · status new

The read result is combined unconditionally with every pending placeholder. If the server has committed the add but its POST response has not arrived, the GET already contains the created todo, so the pane displays both that row and its placeholder.

Evidence: _create_todo adds the placeholder before starting the POST (src/pepper_cli/tui/app.py:2355-2372). A concurrent refresh delivers the committed server row through _got_todos, which calls keep_placeholders (app.py:2027-2032). The unconditional concatenation retains both identities; pane_rows renders both and open_todo_count counts both (src/pepper_cli/tui/state.py:1579-1603). _swap removes the duplicate only after the POST response arrives (state.py:1359-1361), so that deduplication does not protect the intervening reads.

Failure scenario

Start adding a todo to an empty list. Let the server commit it, delay the POST response, and refresh the pane. The pane shows the same todo twice, once open and once pending, and reports two open todos although only one exists. This lasts until the POST finishes.

How this was verified (traced-callers)

Read _got_todos (app.py:2030) calling keep_placeholders, and _swap in state.py. Nothing dedupes between a read and the POST answer. Reachable from: TUI Todos pane: poll or refresh during an add.

Proving test

Use a fake POST handler that commits the new todo before blocking its response. While blocked, refresh the pane and await the GET result. Assert that the title appears in exactly one todo row and the heading reports one open todo; both assertions fail on head.

P1 · VR-e3a7922b · An add is silently discarded if the Todos pane is closed while the room roster loads

src/pepper_cli/tui/app.py lines 2318–2356 · symbol _pick_assignee · lens fail-open · status still-open

In a room, `n` after the due-date step calls _add_asked. When the room's roster is not cached, _add_asked starts a fetch on the 'todo' loop (fetch_roster) whose deliver callback is _pick_assignee. Nothing blocks Esc on the pane while that request is in flight, and _finish does not check the pane. If the user leaves the pane before the roster arrives, self.pane is no longer `pane`. _pick_assignee then takes its `pane is not self.pane` branch and calls _create_todo, which returns at its first line `if pane is not self.pane: return`. So the add is dropped with no POST, no notice and no picker. The user has typed a title and a due date and sees nothing happen.

Evidence: app.py _add_asked: `if room is None or pane is not self.pane: self._create_todo(...)` and the roster branch calls `_start('todo', lambda: poller.fetch_roster(...), got, on_error=...)` with no pane check in `got` before `_pick_assignee`. _pick_assignee (app.py ~2315-2331): `if not members or pane is not self.pane: self._create_todo(...)`. _create_todo (app.py ~2335-2340): `if pane is not self.pane: return`, with no notify. _finish (app.py:405-412) only checks mounted, fatal and generation; the todo loop's generation is never bumped by pane changes (grep of generation += shows only apps, note, topic, older, refresh, shelf, turn). Checked for a protecting notice or cancel: none found in these functions or in _start/_delivered. Note the Esc key goes to the pane because no modal is open during the fetch (the picker is pushed only after the roster arrives).

Failure scenario

Room Todos pane open, roster not cached. User presses n, types title 'Call plumber', presses Enter, types 2026-11-05, presses Enter. The roster GET is slow. User presses Esc, which closes the pane (self.pane = None). The roster returns. _got_roster caches it, then _pick_assignee sees pane is not self.pane, calls _create_todo, which returns silently. No todo is created on the server, no pending placeholder appears, and no error or 'Still saving' notice is shown. The user believes the add happened or sees nothing, and the todo is lost.

How this was verified (traced-callers)

Read _add_asked, _pick_assignee, _create_todo: got callback has no pane check and _create_todo returns silently. Reachable from: n in room Todos, Esc during roster fetch.

Proving test

Room pane open with roster unseeded in app.rosters. Make the roster GET block on a threading.Event (server.gates on the roster route). Drive n, title, due date, Enter. Press Esc to close the pane. Set the gate. Assert that a POST to /api/v1/groups/{ROOM}/todos was made, or that a notify with 'Couldn't add' is shown. On the PR head no POST is sent and no notice appears, so the assertion fails.

P1 · VR-fcc3ab99 · A todo write whose answer was lost is shown as failed and never re-read; a retried add creates a duplicate

src/pepper_cli/tui/app.py lines 2241–2253 · symbol _todo_failed · lens concurrency-time · status still-open

_todo_failed treats every failed write as not applied: it discards the pending mark, drops an add's placeholder, redraws, and shows "Couldn't ... ". It never re-reads the list, unlike _todo_written, which bumps the apps generation and re-reads. The write goes out as a single POST (api.create_todo / complete_todo etc., default httpx timeout via DEFAULT_TIMEOUT), and httpx.RequestError (ReadTimeout) is in _EXPECTED, so a timed-out or connection-dropped request reaches this path even when the server already committed the row or status change. Nothing reconciles the local view with the server until the next PANE_INTERVAL (30 s) apps poll.

Evidence: app.py:2241-2253 (_todo_failed: pending.discard, drop_todo only for pending: ids, _render_pane, _action_failed; no _loops['apps'].generation bump, no _run('apps')). Compare app.py:2227-2239 (_todo_written does the generation bump and _run('apps')). poller.add_todo/todo_verb (poller.py ~281-313) make single POST/verb calls with no retry or idempotency key. Default timeout: api_client.py:1754-1755 (DEFAULT_TIMEOUT). _EXPECTED includes httpx.RequestError (app.py:99). PANE_INTERVAL = 30.0 (state.py:64) is the only reconciliation. Protections looked for: no idempotency key in create_todo payload, no re-read on failure, no in-flight dedupe for adds. The PR's tests cover only fail-before-commit (fail[...] = 500 with no server change), not commit-then-fail.

Failure scenario

User is in a room's Todos pane and presses n, enters 'Pay rent' and a date, and the POST /groups/{id}/todos commits on the server but the response is lost (ReadTimeout). _todo_failed drops the placeholder and notifies "Couldn't add the todo (...)". The user believes nothing was created, presses n again and retypes 'Pay rent': a second todo is created, so the room has two copies. For space/d on a row: the server marks it done, the client restores the row as open with no re-read, and it shows open for up to 30 s, so a second press sends a redundant write and reports a different error or shows the wrong status.

How this was verified (traced-callers)

Read _failed/_todo_failed: on_error returns False from _action_failed path, no _run('apps'); only _todo_written re-reads. Reachable from: n key in room Todos pane.

Proving test

Fake server (tests/tui_support.py) where POST .../todos commits the row and then returns a response that httpx times out or a 503. Open a room's Todos, press n, type title and a date, press Enter to skip the member pick. Assert the notice says couldn't add, then assert that the apps list was NOT re-read (reads(server, ROOM_TODOS) unchanged after the failure) and that the created row is absent from pane.todos. Then press n again with the same title and assert the fake now holds two rows with that title. On head the second assertion holds, which is the duplicate.

P2 · VR-177ef06a · After an add is answered, the cursor lands on whatever row is at the placeholder's index, not on the created todo

src/pepper_cli/tui/app.py lines 2227–2239 · symbol PepperApp._todo_written · lens concurrency-time · status new

_create_todo sets pane.selected to the placeholder key 'todo:pending:N' so the new row is selected while the write is in flight. When the answer lands, _todo_written swaps the placeholder for the server's row (id new-X), so 'todo:pending:N' no longer exists in the new rows. state.reselect then falls back to the row now at the placeholder's old index (min(old index, len-1)). The created row is only selected when the server appends it at the end of the list; when the server's sort places it earlier (for example a timed due date that sorts above other open todos), the cursor lands on a different existing todo.

Evidence: Read app.py 2187-2253 (_todo_written, _todo_failed), 2352-2375 (_create_todo sets pane.selected = f"todo:{row.id}" before the write), state.py reselect (~1617 area of the PR diff, the old-index fallback). Checked for a remap of the placeholder key to the created row: none; apply_todo (state.py 1365) swaps the row but does not touch pane.selected. The fallback is the design for a removed row (a verb that moves a row to Closed), and it is applied here to an add whose key was replaced, so the selection is not remapped to the new row. The tests (test_tui_todo_actions_state.py reselect test) only cover a removed row, not a placeholder swapped for a real id.

Failure scenario

Room Todos list is [Book sitter (open), Call plumber (open), Water plants (open)] sorted by due date. User presses n, adds 'Pay rent' due 2026-11-01. The placeholder is appended at index 3 and selected. The server answers with a row that sorts to index 0 among open todos. _todo_written removes the placeholder, the rows become [Pay rent, Book sitter, Call plumber, Water plants], and reselect returns new[min(3, 3)] = 'todo:Water plants'. The cursor now sits on 'Water plants' instead of the todo just created. A next press of space or d acts on 'Water plants', which the user did not choose.

How this was verified (traced-callers)

Read _create_todo, which sets pane.selected to the placeholder key, then _todo_written and reselect. Nothing remaps the selection to the created row, and the order comes from todo_sections sorting by due date. Reachable from: TUI Todos pane, key n.

Proving test

In a room Todos pane with three open todos, press n and enter a title and a due date that the fake server sorts to the top; after the answer lands, assert app.pane.selected == the created todo's key (todo:new-N). On head it is the key of the row at index 3 instead.

P2 · VR-ba64b362 · Room adds reject the decided timed due-date format

src/pepper_cli/tui/state.py lines 1424–1431 · symbol parse_add_due · lens spec-adversary · status still-open

The room branch rejects every date containing a time, despite the accepted P2 design explicitly allowing YYYY-MM-DD HH:MM for adds. This prevents creating a room todo with a timed deadline through the new flow.

Evidence: The accepted design requires both date and datetime input in planning/tui-room-todos-notes.md:315-318 and decision 7 at line 476. The existing CLI's room restriction is real (cli.py:3301-3311), but does not implement that decided TUI behavior. ApiClient.create_todo accepts due_date with group_id (api_client.py:2025-2067), and the room create route uses TodoCreateRequest, whose schema includes due_date. The PR's stated departure is not an accepted scope decision.

Failure scenario

In a room's Todos pane, add a title and enter 2026-11-05 10:00 as its due date. The TUI reports an error and repeats the prompt instead of proceeding with the timed deadline.

How this was verified (read-only-reasoning)

parse_add_due room branch rejects times, matching the CLI. Reachable from: n in room Todos with a timed due date.

Proving test

Add a room todo with due input 2026-11-05 10:00, choose no assignee, and assert that creation sends a timezone-aware due_date preserving 10:00. Head rejects the input before creation.

P2 · VR-cc44babc · Reopening a pane loses the write's refresh

src/pepper_cli/tui/app.py lines 2230–2232 · symbol PepperApp._todo_written · lens concurrency-time · status still-open

The pane identity check returns before invalidating reads or requesting a refresh. A newly opened pane showing the same owner can therefore retain a pre-write result after the write succeeds.

Evidence: _hide_pane and _show_pane invalidate only apps/note reads and create a new pane object (app.py:1906-1938). The todo callback retains the original pane (app.py:2223). Its identity guard skips both the apps generation bump and reread (app.py:2231-2239). No todo event supplies another refresh; PANE_INTERVAL is 30 seconds (state.py:64).

Also reported here: A poll resets the cursor during completion: Reselection uses the rows and selection present when the write response arrives. A poll that observes the committed mutation first has already removed the selected open row and reset selection to the first row, so the original cursor position cannot be recovered.

Evidence: _got_todos replaces the list and calls settle_selection (app.py:2027-2032); settle_selection selects the first row when the previous key disappears (state.py:1591-1607). Closed todos disappear from visible rows while the fold is shut (state.py:1564-1580). _todo_written captures its before snapshot only afterward, and reselect preserves the newly selected A (app.py:2233-2236; state.py:1457-1464). The generation bump occurs too late to reject an already delivered poll.

Failure scenario

Start completing a todo, close and reopen its Todos pane while the POST is pending, and let the reopened pane read the old state. When the POST succeeds, its callback returns because it holds the previous pane object. The reopened pane continues showing the completed todo as open until another refresh, normally up to 30 seconds later.

How this was verified (read-only-reasoning)

_todo_written returns at pane identity check before generation bump. Reachable from: space, Esc and reopen pane while POST pending.

Proving test

Gate a completion POST before mutation, close and reopen the same Todos pane, and allow its GET to return the open todo. Release the POST and assert that the reopened pane updates without a manual refresh or polling interval. The assertion fails on head.

Confirmed fixed

• VR-8f65afbf · The todo loop's one-at-a-time guard is shared with the roster fetch, so a second write can start while the first is still in flight
• VR-fcc15b9d · A typed new todo is dropped with only a notice when another write is still saving at the picker step

[allenhutchison]

Round 1 summary (Greptile 4/5 on 126a91c, read from the PR description block):

• Pending adds disappear during reads: fixed in 437bf76 (a read keeps the placeholders still pending)
• Add dialogs can overlap: fixed in 5c95773 (n waits while a todo request, a roster read included, is in flight)
• Picker offers an invalid choice: fixed in 8c54c81 (no member step, and no roster read, while your person id is unknown)

Each fix has a regression test. Pre-flight on 8c54c81: format, lint, ty, 2690 tests passing, coverage 95.2% (floor 88).