playbook: publish every client edit, and re-converge when one is rejected (#1103, #1104, #1089) - #1116
Merged
Merged
Conversation
…cted Three paths mutated the Playbook buffer and returned without publishing, so the change lived only in that client — and because the next keystroke's anchored edit was then derived from a base the daemon never received, it failed too, and so did every one after it. The buffer kept accepting input and looked correct the whole time. - Paste (#1103): on_paste now runs the same tail handle_playbook_key does — insert, follow scroll, publish cursor, flush. - Template buttons: apply_playbook_template had the same shape, so a brand-new Playbook filled from the empty state never left the client. - Rejected live edits (#1089): the failure was dropped on the floor, which left saved_markdown stale — on_playbook_state then stopped adopting remote changes to protect the "unsaved" edit, severing the document in both directions. It now falls back to the same 3-way merge C-x C-s performs, which rebases, keeps both sides, and reports the merge. Also normalize CR line endings (#1104). Terminals send CR for the line breaks inside a bracketed paste, and stored verbatim they are not line breaks to anything downstream: the renderer paints them as nothing and block splitting is newline-based, so a pasted block collapsed to one visible line and one addressable block. Normalized on the client so the caret is right immediately, and in both daemon write paths so the guarantee belongs to the store rather than to each client. Drops the #[ignore] from the two regressions added in #1114 and adds a third covering the rejected-edit recovery. Specs 0171 (edits publish and re-converge) and 0172 (stored Playbooks are newline-normalized).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1103, #1104 and #1089, and turns on the regressions #1114 landed
#[ignore]d.(Replaces #1115, which GitHub auto-closed when its base branch was deleted on the #1114 merge. Same commit, rebased onto
main.)These are one bug wearing three hats. A Playbook is a document a human and an agent hold at once, so an interactive client owes it two things: publish every local mutation, and re-converge when a publish is rejected. The TUI was doing neither on some paths, and the failure mode was invisible — a correct-looking buffer that had stopped talking to the daemon in both directions.
#1103 — paste never published
on_pasteinserted into the buffer and returned. Every keystroke path snapshots the buffer and callsflush_playbook_live_edit; this one didn't, so a pasted block lived only in the client. Worse, the next keystroke's anchored edit was then derived from a base the daemon never received, so it failed too — and so did every one after it.Paste now goes through
insert_playbook_text_synced, which inserts, follows the scroll, publishes the cursor, and flushes — the same tailhandle_playbook_keyruns.#1104 — CR line breaks stored verbatim
Terminals routinely deliver the line breaks inside a bracketed paste as
\r(it's what a tty expects for Enter, and what tmux'spaste-buffersends without-r). Nothing normalized them. Stored verbatim they aren't line breaks to anything downstream: the renderer paints them as nothing, so the block collapses to one visible line, and block splitting is newline-based, so it collapses to one addressable block — one shimmer target, one selection-Run target, one opaque line for the agent.Normalized in two places, deliberately:
insert_playbook_text— so the caret lands where the user expects immediately rather than after a round trip, and every ingress path is covered (bracketed paste, clipboard yank, clip insertion).#1089 — rejected live edits were swallowed
let Ok(result) = … else { return };dropped the failure. That leftsaved_markdownstale, soon_playbook_statestopped adopting remote changes (correctly, to protect what looked like unsaved work) — and the document was severed in both directions while the buffer kept accepting input.A rejected edit now falls back to
save_playbook_popup, the same three-way mergeC-x C-sperforms: it rebases onto the latest document, keeps both sides, re-armssaved_markdown, and sets its own status so the user sees that a merge happened. No new machinery.A third instance, found while writing the spec
apply_playbook_template— clicking a template button in the empty state — had exactly the same shape: replace the buffer, return, never publish. So a brand-new Playbook, the very first thing a user does with one, existed only in that client. Fixed the same way.Tests
The two regressions #1114 landed lose their
#[ignore]and pass:playbook_recovers_from_a_diverged_bufferis new here, covering #1089. Verified it fails without the fix, for its own reason:It opens the divergence with
C-x u, which today mutates without publishing (#1088). The assertion is written to hold after that's fixed too — there is then simply no divergence to recover from, and the typed character still lands.Specs
0171-playbook-client-edits-publish-and-reconverge— the rule these three bugs each broke, and the reason the second half (re-converge) exists: a client that stops publishing doesn't lose one edit, it loses all of them, silently.0172-playbook-stores-newline-normalized-markdown— why the store owns the newline guarantee rather than each client.Binaries
crates/cliandcrates/daemonboth change, and both live in the singleconstructbinary:/Users/moon/agentd/.claude/worktrees/fix-playbook-paste-sync/target/debug/constructTo try it: open a Playbook, paste a multi-line Markdown block from your terminal, and check
construct playbook get <session>— the structure survives and the daemon has it without saving.