dsh-emacs

Imported from vritser/dsh-emacs (AGENTS.md). Install upstream with npx skills add vritser/dsh-emacs. Copyright stays with the author.

AGENTS.md — dsh-emacs development rules

Emacs client for the dsh server: chat UI, session/workspace management, RPC over HTTP with a WebSocket event stream. Entry file dsh-emacs.el; module ownership map in docs/architecture.md.

• Baseline: Emacs 28.1 (;; Package-Requires: ((emacs "28.1")) in every file header). Do not silently raise it; a raise must update all headers, README, and CHANGELOG in the same change.
• Do not use Emacs 29+/30+ APIs without a version guard or a local fallback.

Core principles

• Question every abstraction: add a layer, file, or indirection only when it solves a current problem; prefer simpler code and clear ownership over speculative structure.
• Refactors must produce net value: a concrete gain in simplicity, code size, robustness, or test value. Moving code, renaming layers, or adding wrappers without making the system easier to understand is not a refactor.
• Root out helper stacking: one-use wrappers, pass-through functions, and accessor piles around a single call site are debt — fix the missing ownership boundary, don't add another helper tier.
• Delete, don't deprecate: remove unused code entirely; no compatibility shims or "removed" tombstones. The one sanctioned compat seam is dsh-protocol--struct (accepts wire alist or struct, for legacy fixtures).
• Prefer lightweight Elisp shapes: let*, pcase-let, alists/plists for short-lived context; cl-defstruct is reserved for protocol payloads that cross module boundaries (that is exactly dsh-emacs-protocol.el).
• Reduce code by improving the model: simpler state and control flow, not dedup-for-its-own-sake or file extraction to look tidy.

Verification (run after every change — never skip)

One-shot gate: scripts/verify.sh — the aggregated "definition of done" (check-lisp over dsh-check:files, checker and runner self-tests, production byte-compile with free-variable warnings as failures, full unit suite, clean load silent, git diff --check, tree junk scan). Exit 0 only when everything passes; any FAIL is a blocker (see rule 6 below).

CI runs the same gate on every push and pull request (.github/workflows/ci.yml) on Emacs 28.1 and a current stable release. A red run is a blocker even when the local gate passed; fix it before merging.

The manual loop, in order:

1. Syntax check with scripts/check-lisp.el — MANDATORY, always first. The only sanctioned judge of elisp syntax; every change must pass it. Two-phase (forward-sexp walk + real read with sentinel), so it rejects everything load rejects: imbalance, stray/missing closers, unterminated strings, #| block comments, ]/[ cross-closing.

   emacs -Q --batch -l scripts/check-lisp.el            # all files in dsh-check:files
   emacs -Q --batch -l scripts/check-lisp.el -- foo.el  # single file
   # exit 0 = all pass; exit 1 = failure; exit 2 = usage error

   Pass files explicitly; with no files it acts on the default list in dsh-check:files (a warning is printed in that case).
2. Fixing syntax errors — by hand, from the diagnostic report, in order (the playbook below); then re-run step 1 until exit 0.
3. Full unit tests: emacs -Q --batch -l test/dsh-test.el; checker self-tests: emacs -Q --batch -l test/check-lisp-test.el; runner/coverage self-tests: emacs -Q --batch -l test/dsh-test-runner-test.el
4. Clean load: emacs -Q --batch -L . -l dsh-emacs.el should print nothing and exit 0
5. For function-level repros prefer a minimal batch: emacs -Q --batch -L . --eval '(...)'
6. Failures stop the pipeline: any FAIL from steps 1–5 is a blocker — fix it and re-run from step 1 before moving on; never carry a failed check into later steps or into the final summary.

check-lisp repair playbook

check-lisp never rewrites files. On failure it prints every problem at once, ordered by root cause (unterminated string [first] → missing closers [second] → stray closers [third]), with type / line / column / absolute offset / context. Fix in that order and re-run after every edit: one error can mask or cause others (a missing closer swallows following forms, an unterminated string hides everything after it, and positions in a stale report drift once you edit). Never batch-apply a whole report blindly.

• Stray )/] — delete the exact character at the reported line/column.
• Missing closers — the report lists the opener stack (innermost first, with coordinates). Placement is an intent judgment: appending at EOF silently merges later top-level forms into the open one — read the stack and the context before choosing where to close. After fixing, compare the top-level signature line (printed for every file) before/after: a fix that shrinks the form count has swallowed forms — reopen and re-place the closers.
• Unterminated string — the blocker: everything after the quote is treated as string content (invisible), but any strays before it are reported too. Fix the string first (the [first] item), then re-run to see the rest.
• Balanced-but-read-rejected (#| comments, ]/[ cross-closing, dangling #') — reported with the original error and its line. Only the raw error: line keeps the raw format: scan-error is (message start end) with start an absolute character offset; invalid-read-syntax is (object line col). Everything else prints line / column / offset directly — no raw-error decoding needed.
• After fixing, review git diff.

Elisp editing discipline

• Change one logic block at a time; run the mandatory syntax check (verification step 1) immediately after each change, before moving on
• Structural rewrites (changing call nesting depth, e.g. (cdr (assq 'k v)) → (accessor v)) must keep the paren net balance equal between old and new snippets (( = +1, ) = -1, ignoring comments and strings); equal balance is necessary but not sufficient — the read-level check via scripts/check-lisp.el is final
• Never count long ) runs by eye; rely on read-level balance (a clean batch run of scripts/check-lisp.el means the file is balanced)
• Compile-time issues (undefined functions/variables, macro misuse): emacs -Q --batch -L . -f batch-byte-compile <file> (Errors always matter; among Warnings, reference to free variable is a real bug and fails scripts/verify.sh — it is what an unescaped " inside a docstring looks like after the string closes early, and no read-level check can see it. The other warning classes — docstring width, a third-party function "not known to be defined" — stay informational)
• A variable read before it can be bound is one bug in three shapes, all of which byte-compile flags: a bare quote closing a docstring early; a let whose init reads a sibling binding from the same let (inits run before the bindings, so use a nested let and keep the paren net equal); and a defvar placed after its first reader, which makes the reader treat the name as lexical (move the declaration above the reader)
• Intentional lazy/cyclic module boundaries: add declare-function / defvar forward declarations rather than new top-level requires; a declare-function pointing at another package's private (--) symbols is a design smell — move the interface to the owner module instead
• Scratch/probe files and test fixtures live in temp locations (make-temp-file or outside the repo tree), never in the tree itself; check-lisp is diagnostics-only and never rewrites files — any tool that does write repo files is an anomaly, and the result is reviewed via git diff.

Naming and style

• Public symbols use dsh-emacs-; private, dsh-emacs--. No double dash for anything user-facing (commands, options, faces). Exceptions by design: dsh-emacs (the mode) and dsh-protocol-* / dsh-protocol--* (struct accessors and helpers in the protocol module).
• Files of the same dsh-emacs-* subsystem may call each other's -- symbols (they are one package), but must carry declare-function / defvar declarations so byte-compilation stays honest.
• dsh-check:* is the internal namespace of scripts/check-lisp.el (a dev tool, not package API).
• Lowercase words separated by hyphens; no snake_case or camelCase. Predicates: single-word names end in p, multi-word in -p.
• Prefix unused lexical variables and arguments with _.
• Emacs indentation is authoritative: indent with spaces, no hard tabs, no manual alignment against the text; keep lines near 80 columns where feasible; trailing closers stay together; blank line between unrelated top-level forms.
• Prefer flat control flow: if-let*, when-let*, pcase, pcase-let over deep let → if nesting; when for one positive branch, unless for one negated branch; no redundant progn.
• Destructure instead of repeated accessors (pcase-let over multiple nth / plist-get); cl-loop over dolist + manual accumulators; dolist (not mapcar) when the result is discarded.
• #'function-name when passing a function value in executable code; named functions (not forwarding lambdas) for hooks, advice, and long-lived registrations.
• Macros only for syntax: if a function can express the behavior, use a function; every new/modified macro declares its cl-edebug spec and an indent property when the body isn't a plain call shape.
• State placement: defcustom under defgroup dsh-emacs for user options, plain defvar for shared state, defvar-local for buffer state (chat buffers keep their session/process state buffer-local).
• Comments and docstrings may be English or Chinese — the tree mixes both; match the file you are editing.

Architecture and boundaries

• One responsibility per file, per the module map in docs/architecture.md: wire payloads → dsh-emacs-protocol.el; push-frame dispatch and transport → dsh-emacs-events.el; buffer rendering → dsh-emacs-render.el; faces/theme → dsh-emacs-faces.el; session list UI → dsh-emacs-session.el; mode-line → dsh-emacs-modeline.el; server lifecycle → dsh-emacs-server.el. Do not mix protocol parsing with rendering.
• dsh-emacs.el is the entry point and session/command coordinator — new self-contained feature surfaces belong in the owning module, not in a growing grab bag. dsh-emacs-server.el owns HTTP/RPC transport and authentication; RPC consumers require it directly instead of depending back on the entry point.
• Required same-package functions are called directly, with declare-function at intentional cyclic boundaries. Reserve fboundp for optional external integrations, version/build capabilities and genuinely dynamic dispatch; missing package definitions must not silently suppress work.
• Loading a file must not change editing behavior (no modes enabled, no hooks fired); registration side effects allowed at load are package-level ones (auto-mode-alist, add-to-list 'minor-mode-map-alist, …). The clean-load check (verification step 4) must stay silent.
• Reuse stock Emacs infrastructure (completing-read, read-only-mode, standard hooks) before inventing parallel mechanisms.

Protocol-layer constraint

• On the inbound/decoding path, wire field names (sessionId, archivedSessionIds, …) must appear only in the --from-alist constructors inside dsh-emacs-protocol.el; business code reads decoded fields through dsh-protocol-* accessors exclusively.
• On the outbound path, session/workspace/job/skills/commands/goals arguments, provider configuration/credentials, interaction replies/wait claims, reference queries, nested queue actions/attachment parts and RPC/mux envelopes belong to protocol request/content/action/outcome/JSON encoders. Do not construct these wire bodies inline in production callers (rationale: postmortem/140). Local staging metadata and display-only synthetic events are not RPC payloads; literal ERT/E2E wire fixtures remain independent by design. RPC/mux JSON uses dsh-protocol-write-json, not a caller-configured json-encode.
• Business functions that must accept legacy test fixtures use the dsh-protocol--struct compat gate to take either a wire alist or an already-converted struct; wire arrays are vectors and are normalized to lists inside the structs

Diagnosis and error handling

• Find the root cause before changing behavior: do not patch UI timing, cache invalidation, or command flow until you can name the failing layer (transport / protocol / events / render / modeline / UI) and explain why it is responsible.
• One failed fix narrows the hypothesis; two stop the patching loop: after two failed fixes on the same issue, switch to diagnosis only — no third speculative patch.
• Fix the right layer: if the real problem is in the RPC/events stream or the protocol structs, move the fix there instead of compensating in the renderer or mode-line.
• Errors must surface, not hide: no condition-case nil swallowing internal failures; only boundary code (RPC callbacks, process filter, top-level command) converts exceptions into messages.
• user-error for user-caused problems, error for programmer bugs; error text describes the current problem, not a lecture about what to do next.
• Flag compensating code (silent fallbacks, re-reading data a caller already has, timing hacks) as design debt in a postmortem (see §Postmortems below) — do not let debt discovery delay the current change.

Testing

• Tests must fail when the code is wrong: assert specific, distinguishable output values; if breaking the function under test doesn't turn the test red, the test is worthless.
• Red before green for real bug fixes: write the failing reproduction first; updating an existing test that already proves the path is sufficient for small expectation changes.
• Match test weight to change size: comments, docs, and mechanical refactors are not red/green exercises; for pure cosmetic tweaks (face, padding, truncation, display order) don't add tests unless the text carries a real contract (state visibility, command availability, previously regressed behavior).
• Unit tests live in test/dsh-test-<subsystem>.el (ERT, no server needed). Each subsystem requires test/dsh-test-support.el for shared fixtures; test names start with dsh-emacs-test- and carry :tags '(dsh-unit). test/dsh-test.el is only the loader and batch entry point; keep emacs -Q --batch -l test/dsh-test.el working. Loading subsystem files defines tests without executing them. Add new cases to their owner, not the runner or shared support.
• For an individual interactive test, add the checkout's test/ directory to load-path, load its subsystem file with M-x load-file, then use M-x ert to select its name. To run a subsystem in a fresh batch:

  emacs -Q --batch -L test -l test/dsh-test-protocol.el -f ert-run-tests-batch-and-exit

  Tools loading the full suite bind dsh-emacs-test-no-run to t, then call dsh-emacs-test-run with an optional ERT selector. It rejects an empty selection and returns statistics without exiting; the default selector is (tag dsh-unit). Tests must not depend on another test having run first.
• Use unconditional should, should-not, or should-error assertions. A conditional body that never reaches its assertion can still pass ERT. Use skip-unless for a genuinely absent optional dependency, not for a broken or stale installed library. Keep fixtures and cleanup local to each test so individual runs and reruns are meaningful.
• Checker self-tests live in test/check-lisp-test.el; isolated runner and coverage exit-contract tests live in test/dsh-test-runner-test.el (tag dsh-runner, excluded from unit-suite discovery). scripts/check-lisp.el discovers all test/dsh-test-*.el files for syntax checking, including support and runner self-tests. test/dsh-e2e.el drives a real dsh server and is deliberately outside the verify gate; run it manually when touching the event stream / RPC round-trip paths.
• Mock the RPC seam (dsh-emacs--rpc-async et al.) instead of spawning servers in unit tests; fixtures are wire-shaped alists fed through the compat gates.
• Optional coverage report (testcover line/branch): emacs -Q --batch -l scripts/check-coverage.el reports per-definition coverage and defs below 80%. Low coverage is informational, not a blocker; unexpected test failures still exit nonzero, after printing the report. dsh-emacs-protocol.el is excluded (testcover's edebug copy breaks cl-defstruct values). Rationale: postmortem/118-ert-subsystem-tests.md.

Documentation and changelog

• Any user-visible change (behavior, keybindings, defaults, defcustoms) adds a CHANGELOG.md entry in the same change, under ## X.Y.Z - Unreleased (the planned next version — pre-1.0: fixes → patch, features → minor; unreleased sections stay undated, the date is filled in only when the release is cut). If a Breaking Changes category is needed, it goes before Added. Single-concern entries: one bold lead + what users experience, not a commit dump.
• Pre-1.0 breaking changes (renamed/removed public options without aliases) are listed under Breaking Changes of the next minor; no compat shims are retained (see Core principles).
• Keybinding / default / customization changes also update README.md and the matching docs/*.md (customization, modeline, slash-commands, …) in the same change.
• If code and docs diverge, treat code as source of truth and fix the docs immediately; never invent commands, capabilities, or behaviors in docs — verify them from code or tests.

Postmortems (decision records)

The postmortem/ directory is the historical layer: numbered, one-page records of decisions and lessons (template: postmortem/000-template.md). Docs describe the current state; postmortems keep the "why" that survives refactors and renames.

• Read the relevant records before significant changes — they are constraints and rationale, not proof the code still matches them.
• Write a postmortem when: adding or changing a user-visible workflow; choosing between non-obvious architectural approaches; abandoning or reverting an approach; removing a public option or command; deliberately deferring a known limitation. Bug-fix-only changes do not need one.
• Postmortems explain why, not the code: Background / Decision / Why / Consequence / Known limitations, with commits as evidence.
• They are historical records — do not rewrite old ones to match current behavior; when a later change supersedes one, write a new entry and add a short "Superseded by NNN" note at the top of the old file.
• Link both ways: CHANGELOG entries for user-visible changes carry (rationale: postmortem/NNN); the postmortem cites the commits it records.

Commit

• Auto-commit is disabled: only commit when the user explicitly says so; keep changes in the working tree until then. git add (staging) without committing is fine when needed.
• One atomic commit per topic: changes mixing several concerns (e.g. a feat and a fix) are split into one commit per concern first; a subject that needs several topics is a sign to split further.
• Conventional Commits, English single-line summary — keep it short: one concise clause <type>: verb + what changed, e.g. fix: restore mode-line busy after stream reconnect. Do not enumerate details in the subject line (long multi-clause summaries get reworded).
• Local, unpushed history may be rewritten (reword/split/rebase) when the user asks for it.
• Standard types (Angular convention, all usable):
  • feat: new feature / fix: bug fix
  • docs: documentation (README/AGENTS/IMPLEMENTATION)
  • style: formatting, refactor: (no behavior change), perf: perf
  • test: tests, build: build, ci: CI, chore: misc, revert: rollback

Known pitfalls (do not repeat)

• One paren off in a big form can leak a clause or handler into the body: a (t …) cond clause absorbed as a second body form runs as a plain function call — symptoms: void-function t / void-variable. The read-level check catches the imbalance; the runtime symptom tells you which form to re-read.
• If check-lisp.el itself is unbalanced, it cannot bootstrap (the diagnostics build has no fixer): locate the imbalance with the last loadable revision (git show :scripts/check-lisp.el) or a mechanical scanner, then edit precisely — never by hand-counting.

Definition of done (before finishing any change)

• scripts/verify.sh exits 0 — one-shot gate for every machine-checkable step
• git diff reviewed line-by-line for unintended changes (substantive review — not machine-checkable)
• CHANGELOG / affected docs updated in the same change when user-visible
• Nothing committed — commits happen only when the user explicitly asks (enforced by the harness's auto-commit disable, not by this file)

GUI testing the Emacs client (from a headless shell)

What actually works when verifying front-end behaviour (ask prompts, completion front-ends, posframes) on this machine. Editor-only lessons: batch tests and the repo gate are covered by §Verification above.

Launch a real GUI Emacs from the shell

/opt/homebrew/bin/emacs -Q -l /tmp/probe.el      # macOS window server is reachable

-Q skips package-initialize, so load what the probe needs explicitly, then add the checkout to load-path:

(require 'package)
(setq package-user-dir (expand-file-name "~/.emacs.d/elpa"))
(package-initialize)
(require 'vertico) (require 'vertico-posframe) (require 'posframe)
(vertico-mode 1) (vertico-posframe-mode 1)
(add-to-list 'load-path "/Users/ed/playground/dsh-emacs")
(require 'dsh-emacs)

GUI stdout is not visible: write every observation to a file (append-to-file) and read it after the process exits.

Drive the real reader

• Start the work from emacs-startup-hook / run-with-idle-timer so the first frame exists; then schedule input with run-with-timer and (execute-kbd-macro (kbd "2,3 RET")).
• Observe from advice-add :around on the function under test (the reader, completing-read-multiple, read-string) — that is what showed the real return values and the actual prompt text.
• Dump state, do not screenshot: log minibuffer-contents, completion-all-sorted-completions, vertico--candidates, vertico--index, (current-message), and the final answer. screencapture captures whatever window is frontmost (it grabbed a browser) and needs a window id it cannot get reliably headless.
• timeout N /opt/homebrew/bin/emacs … and pkill keep a probe that blocks on a read from hanging the session.

Limits of headless-probe timing

• Once a child frame (posframe) holds focus, the window server throttles the process: timer callbacks were observed 59 s apart, so "after the posframe appears" probes are unreliable. Assertions that depend on current-message or frame visibility during a read do not work there — verify those interactively.
• redisplay, frame visibility and echo-area state are not observable mid-read; do not claim they were measured.
• Probing the status quo is fine (the frame list, answers, candidate lists); perception questions (does it flicker?) require a human session.

Only the real frontend run finds these

Unit tests that mock completing-read keep passing while the GUI shows:

• a bare option number arrives unexpanded ("2", not "2. Beta"), so resolution must accept it;
• require-match + a frontend preselect turns an empty RET into the first candidate, so "empty input = skip" needs require-match nil and vertico-preselect 'prompt;
• a partly-matched comma-separated answer silently loses the unmatched part;
• a bare-letter skip key shadows typing that letter inside the answer;
• the answer order follows the input order unless the caller sorts by the question's option order.

Conclusion: mock the framework and run the real front-end path; treat the GUI run as the acceptance test.

Checking frontend behaviour without guessing

Read the installed source (~/.emacs.d/elpa/<pkg>-*/ for vertico, vertico-posframe, posframe; the Emacs tree for icomplete.el, crm.el). Settled this way: crm-prompt is "[%d] %p" (so CRM, not the caller, prefixes [comma-separated list]), vertico--move-to-front runs after sorting (so vertico-sort-function cannot stop a default being promoted), icomplete--sorted-completions deliberately bubbles a matching default, and string-prefix-p is case-sensitive while compare-strings folds case.