docs(signel): harden initiate-message spec to Ready

I wrote an initiate-message workflow spec on top of the existing Signal client design doc, covering the keymap, name-based picker, message-to-self, and the whole flow. A follow-up review caught three blockers I'd hand-waved: signel had no JSON-RPC success-result dispatch path (so cj/signel--fetch-contacts couldn't actually receive listContacts results), D4's "auto-connect when linked" didn't define what "linked" meant or how process death surfaced, and the contact cache had no ownership or invalidation story.

I verified the central one against the fork. signel--dispatch handles only "receive" and error responses, so success results were dropped. Then I folded all three into an Architecture additions subsection: a request-callback table keyed by JSON-RPC id with cleanup on success/error/reconnect, a cj/signel--ensure-started contract with branches for live process / account-set / account-nil, and a cj-owned cj/signel--contact-cache separate from signel's receive-time map.

A second review pass caught the remaining sync/async boundary. completing-read is synchronous and the fetch is async. I resolved it with pre-warm on connect plus a bounded accept-process-output fallback for cold caches, so the warm case feels instant and a dead daemon can't hang Emacs.

The follow-up re-review then converged to Ready-with-caveats and surfaced one concrete code finding I'd missed: the #2 input-clobber fix has to cover both signel--insert-msg AND signel--insert-system-msg, since both delete from the prompt line through point-max. The pieces-to-build entry and the prompt-preservation regression test now name both paths.

A few smaller tightenings landed in the same pass. The listContacts assumption is now a researched fact (verified on 94 contacts), the two open questions on account source and fork remote are marked decided (defcustom in the gitignored local config, local checkout with no remote for now), and a forward-flag in the scope summary names the four notification details to spec before that later slice starts.

docs/design/signal-client-review.org carries the review as the closure record. todo.org gets two tasks: a [#B] for the JSON-RPC success-result dispatch (the first build step), and a [#D] for groups in the picker as a vNext after the 1:1 flow is stable.

Spec is Ready. Implementation order is pinned to the Pieces-to-build list. RPC dispatch first, then the guard, then fetch/cache, then the picker and keymap, then the clobber fix.

1 files changed, 89 insertions, 0 deletions

diff --git a/docs/design/signal-client-review.org b/docs/design/signal-client-review.org
new file mode 100644
index 00000000..34b4bbda
--- /dev/null
+++ b/docs/design/signal-client-review.org
@@ -0,0 +1,89 @@
+#+TITLE: Review: Signal client in Emacs (forked signel)
+#+DATE: 2026-05-27
+#+STARTUP: showall
+
+* Scope reviewed
+
+- =.ai/workflows/spec-review.org=.
+- =docs/design/signal-client.org=, including the base design, open-question dispositions, initiate-message workflow, architecture additions, accepted caveats, test plan, scope summary, and readiness rubric.
+- =modules/signal-config.el=, including =cj/signal--parse-contacts=, notify-suppression helpers, private config loading, and current =use-package signel= wiring.
+- =~/code/signel/signel.el=, including =signel-start=, =signel--send-rpc=, =signel--dispatch=, =signel--handle-error=, =signel--handle-receive=, =signel--insert-msg=, =signel--insert-system-msg=, =signel--send-input=, =signel-chat=, and dashboard commands.
+- =tests/test-signal-config.el=, covering contact parsing and notify-suppression helpers.
+- =todo.org= Signal parent task and child tasks for contact picker, JSON-RPC result dispatch, notifications, input clobber, link command, wiring, and the vNext group picker.
+
+* Implementation-readiness
+
+=Ready=.
+
+The initiate-message spec is implementation-ready. It identifies the user-facing flow, v1 scope, deferred work, data ownership, sync/refresh behavior, error behavior, observability, performance shape, implementation order, and test strategy. The previous blockers have been folded into the spec: JSON-RPC result dispatch is step 1, the sync picker / async fetch gap is resolved with pre-warm plus bounded blocking, daemon/account guard behavior is concrete, contact cache ownership is explicit, and input preservation covers both message and system-error insertions.
+
+* Overall assessment
+
+The spec now leaves an implementer with an ordered build plan instead of hidden architecture choices. The current fork still lacks JSON-RPC success-result dispatch and still clobbers prompt input in =signel--insert-msg= / =signel--insert-system-msg=, but those are named implementation tasks with clear contracts and tests, not open design questions.
+
+The remaining notification details are explicitly flagged as out of scope for the initiate-message workflow and as a prerequisite for the later notification slice, which is the right boundary.
+
+* High-priority findings
+
+None.
+
+* Medium-priority findings
+
+None blocking or action-changing for this implementation pass.
+
+* UX observations
+
+- The primary flow is concrete: =C-; M m= opens a name picker, selection opens =signel-chat=, then the user types and sends.
+- Message-to-self has both discoverable and fast paths: a pinned picker row plus =cj/signel-message-self=.
+- Cold-cache behavior is defined: pre-warm on connect/restart, bounded wait on first command, useful timeout message.
+
+* Architecture observations
+
+- The callback/result map fits signel's existing async process filter.
+- Separating =cj/signel--contact-cache= from =signel--contact-map= avoids mixing complete contact-list data with opportunistic receive-time sender names.
+- The dependency order is correct: result dispatch, daemon guard, fetch/cache, commands, keymap, then clobber fix.
+
+* Robustness and performance observations
+
+- Cache-on-first-use plus explicit refresh is appropriate for the verified 94-contact scale.
+- Empty successful contact results are distinct from RPC/startup failures.
+- The bounded =accept-process-output= cold path prevents a wedged daemon from hanging Emacs indefinitely.
+
+* Test strategy recommendations
+
+Follow the spec's TDD list:
+- JSON-RPC result callback routing and cleanup on success/error/reconnect.
+- Fetch-result parsing and cache population.
+- Picker label resolution and =signel-account= message-self recipient.
+- =cj/signel--ensure-started= branches.
+- Cache invalidation and empty-success vs failure.
+- Prompt-input preservation for both incoming messages and system-error insertion.
+
+Keep the manual live checklist for pick/open/type/send, auto-connect, Note-to-Self, and incoming-message/system-error clobber cases.
+
+* Suggested spec edits
+
+None required for initiate-message implementation readiness.
+
+* Agreed decisions
+
+- Fork signel and own the internal edits.
+- Use =C-; M= as the Messages prefix.
+- Build JSON-RPC success-result dispatch before the picker.
+- Resolve synchronous picker over async fetch with pre-warm plus a bounded cold-cache wait.
+- Cache contacts on first use, support explicit refresh, and invalidate on reconnect.
+- Store picker contacts in a cj-owned cache separate from =signel--contact-map=.
+- Provide both pinned "Note to Self" and a direct message-self command.
+- Auto-start when configured/linked; otherwise raise a useful setup error.
+- Fix the input-clobber bug across message and system-error insertions.
+- Keep v1 picker to 1:1 contacts; defer groups.
+
+* Open questions
+
+None blocking.
+
+* vNext candidates
+
+- Include groups in the picker by merging =listGroups= with =listContacts= after the 1:1 flow is stable. Already tracked in =todo.org= as =[#D]=.
+- Add the QR-based =cj/signel-link= command after the initiate-message path ships.
+- Define the notification slice details before that later task starts: exact =notify= command shape, fallback when =notify= is missing, body truncation, and whether Signal message text is shown verbatim.