Skip to content

Commit ac50600

Browse files
committed
docs(adr): ADR-0170 — nREPL re-architecture, base-protocol fidelity (CIDER end-to-end)
Empirical probes confirmed 6 defect classes (read-loop stranding, 4KiB frame ceiling, broken session identity, malformed error replies, missing completion/lookup ops, describe drift+stale version). Decision: src/app/nrepl/ package (transport/session/ops/eval) + shared app/eval_session.zig (CLI REPL gains *1..*e) + runtime/introspect.zig. DA fork adopted Alternative 2 (finished-form-clean deepenings); alternatives + fact-corrections reflected in the ADR.
1 parent 94830c9 commit ac50600

1 file changed

Lines changed: 240 additions & 0 deletions

File tree

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,240 @@
1+
# ADR-0170: nREPL server re-architecture — base-protocol fidelity (CIDER end-to-end)
2+
3+
- **Status**: Accepted
4+
- **Date**: 2026-07-13
5+
- **Related**: ADR-0015 am2 (F142 re-introduction), ADR-0048 (nREPL state chart),
6+
ADR-0138 (`*out*` capture via binding frame), ADR-0019 (exit/error policy),
7+
D-117 (nREPL polish), D-118 (out/err capture), F-002 / F-009 / F-011 / F-014
8+
9+
## Context
10+
11+
The row-14.10 minimal nREPL server (`src/app/nrepl.zig`, one file, 7 ops)
12+
predates most of the runtime's current capabilities. A CIDER field report
13+
(REPL buffer RET dead, no completion, bare `NameError` errors) triggered an
14+
empirical probe of the live server (two independent bencode replay drivers,
15+
2026-07-13). Confirmed defects:
16+
17+
1. **Read-loop stranding**: the session loop does one `fillMore()` →
18+
one `decode` → one op per iteration, so it *blocks for new socket
19+
bytes even when complete bencode dicts already sit buffered*.
20+
Pipelined requests strand off-by-one (probe: of 2 requests written
21+
in one TCP segment, the second is answered only after a *third*
22+
write arrives). CIDER overlaps requests during connect (init eval +
23+
capability probes), so the init eval's `done` strands → the REPL
24+
prompt marker never renders → RET inserts a newline forever.
25+
`C-c C-e` works because it is a lone in-flight request.
26+
2. **4 KiB frame ceiling**: `decode` sees at most the fixed 4096-byte
27+
stream buffer; a larger message can never decode → `catch break`
28+
closes the connection (probe: 10.6 KiB eval → connection reset).
29+
`C-c C-k` (load-file) on any real file kills the connection.
30+
3. **Session identity broken**: every `clone` returns the *same*
31+
connection-constant id, and every reply stamps that constant
32+
instead of echoing the request's `session`. CIDER clones two
33+
sessions (main + tooling) over one socket and relies on distinct
34+
ids (nrepl-client.el:619-639; the CIDER mock server echoes the
35+
request session and mints a fresh `new-session` per clone).
36+
4. **Error replies malformed × 4**: (a) `err` text is the raw Zig
37+
`@errorName` (`"NameError"`) instead of the rich caret rendering
38+
the CLI already produces via `error_render.zig`; (b) `err` and
39+
`status ["error" "eval-error" "done"]` ride ONE dict, which
40+
CIDER's mutually-exclusive response `cond` mis-routes
41+
(nrepl-client.el:744-770); (c) a second bare `done` follows
42+
(double-done per request); (d) evaluation *continues* past a
43+
failing form (JVM nREPL stops).
44+
5. **Missing ops**: no `completions`/`complete` (CIDER gates
45+
completion on these being advertised — cider-completion.el:136-142),
46+
no `lookup`/`info`/`eldoc` (eldoc + doc popups dead).
47+
6. **describe lies**: `versions` hardcodes `"0.1.0-pre"` (real version
48+
= `build_options.version` = the `build.zig.zon` value, currently
49+
1.1.0); ops list is hand-maintained separately from the dispatch
50+
chain (drift by construction).
51+
52+
The runtime meanwhile grew the exact capabilities the server lacks:
53+
`error_render.zig` (rich caret rendering, EDN mode "for CIDER /
54+
editors" per its own doc comment), `line_editor.zig::handleTab`
55+
(Env-wide completion-candidate enumeration), `Var.meta`
56+
(`:arglists`/`:doc` for `clojure.repl/doc`). The single-file server
57+
simply never re-consumed them — the textbook early-asset inertia
58+
F-002 exists to overrule.
59+
60+
**Reference precedent**: babashka.nrepl (`~/Documents/OSS/babashka.nrepl`)
61+
is the JVM-less nREPL that drives CIDER fully. Its load-bearing idioms:
62+
a single `response-for` choke point that stamps *every* reply with the
63+
request's `session`+`id`; sessions as plain fresh-UUID tags; op
64+
multimethod with `:default` → `unknown-op`; the 3-message error
65+
protocol (`err` → `ex`/`root-ex`/`eval-error` → `done`); `completions`
66+
+ `lookup`/`info`/`eldoc` built from interpreter introspection. Its
67+
compromises (no stdin op, no mid-eval interrupt, no cider-nrepl
68+
extension middleware) are CIDER-graceful and adopted here.
69+
70+
## Decision
71+
72+
Rewrite the server as a package under `src/app/nrepl/` (precedent:
73+
`src/app/repl/` for the line editor), with one shared introspection
74+
module extracted to the runtime layer:
75+
76+
| File | Responsibility |
77+
|----------------------------|----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------|
78+
| `src/app/nrepl.zig` | Thin entry: bootstrap + `.nrepl-port` + sequential accept loop (concurrency model unchanged; CIDER uses one socket). |
79+
| `src/app/nrepl/transport.zig` | Framing + reply choke point. Growable receive buffer; decode **all** buffered messages before blocking (`bencode.DecodeError.UnexpectedEof` = truncated → wait for bytes; other decode errors = protocol error → close); message size cap. Every reply passes through `responseFor`, which echoes the request's `session` + `id` — ops cannot mis-stamp. |
80+
| `src/app/nrepl/session.zig` | Registry: `clone` → fresh unique id; `close`; `ls-sessions`. Per-session state: current-ns name + `*1`/`*2`/`*3`/`*e` Values, registered as permanent GC roots (`.dev/gc_rooting.md` discipline) and released on close. |
81+
| `src/app/nrepl/ops.zig` | `comptime` StaticStringMap op → handler. `describe` **derives** its ops list from the same table (drift impossible) and reports `versions` from `build_options.version`. Unknown op → `status ["error" "unknown-op" "done"]`. |
82+
| `src/app/nrepl/eval.zig` | Eval engine: per-form read/analyze/eval; honors the request `ns` key (binds the session ns; `status ["namespace-not-found" "done" "error"]` on miss); per-form `*out*` capture (ADR-0138 shape); updates `*1`/`*2`/`*3`/`*e`; **stops on first error** (JVM parity); errors render via `error_render` into `err`, then `{ex, root-ex, status [eval-error]}`, then `done` — three separate dicts, exactly one `done` per request. |
83+
| `src/app/eval_session.zig` | **Shared REPL eval engine** (DA deepening (a)): per-form read/analyze/eval with the D-430 AnalysisFrame bracket, `*out*` **and `*err*`** capture, `*1`/`*2`/`*3`/`*e` rotation over the interned dynamic Vars, stop-on-first-error, error rendering through `error_render` — driven through caller-supplied sinks. Consumers: `nrepl/eval.zig` (bencode sinks) and `repl.zig` (terminal sinks — the CLI REPL gains `*1`..`*e` in the same cycle; duplicating the rotation would be the F-011 violation the DA flagged). |
84+
| `src/runtime/introspect.zig` | Shared introspection: completion-candidate enumeration (extracted from `line_editor.zig::handleTab`'s collect* fns — same-layer move, Env is layer 0), returning **typed** candidates `{name, ns?, kind}` (fn/macro/var/namespace — the completions op needs the type; the line editor ignores it) + var-meta lookup (`:arglists`/`:doc` + macro flag from `Var`). Consumers: line editor (candidate cap 64), nREPL `completions`/`lookup`/`info`/`eldoc`, future `--list-vars`. |
85+
86+
**Op surface** (all advertised by the derived `describe`): `clone`,
87+
`close`, `describe`, `eval`, `load-file`, `interrupt`, `ls-sessions`,
88+
`completions`, `complete`, `lookup`, `info`, `eldoc`.
89+
Response shapes mirror babashka.nrepl (CIDER-proven):
90+
- `completions`/`complete` → `{completions: [{candidate, type, ns?}…], status [done]}`
91+
(`type` from Var flags: macro / function / var; namespace candidates → namespace).
92+
- `info` → flat `{ns, name, arglists-str, doc?, file?, line?}` + done;
93+
`lookup` → the same map nested under `info`; `eldoc` →
94+
`{ns, name, eldoc [[arg…]…], type, docstring?}` + done, `no-eldoc`
95+
status on miss.
96+
97+
**Core surface addition**: `*1` / `*2` / `*3` / `*e` are interned in
98+
`clojure.core` as dynamic vars in the upstream core.clj shape (JVM
99+
parity — today they don't resolve at all, which is itself a gap;
100+
interning is also what makes `(inc *1)` *analyzable*, so a
101+
session-struct-only shape would silently not resolve). Both REPLs
102+
bind and rotate them via `eval_session.zig` in this cycle.
103+
104+
**Session state rooting**: each session's held Values (`*1`..`*e`)
105+
are rooted via `GcHeap.pin`/`unpin` (the existing `permanent_roots`
106+
embedder surface — no new root class), swapped on rotation and
107+
released on session close; the `GC-ROOT:` marker + `.dev/gc_rooting.md`
108+
row land in the same commit per that SSOT's discipline.
109+
110+
**Transport memory contract** (DA finding #5): the pre-ADR server fed
111+
every decode/encode/print into the process-lifetime arena — unbounded
112+
growth over a long editor session. `transport.zig` owns a per-message
113+
scratch arena (bencode decode + reply encode + value-print buffers,
114+
reset after each reply flushes); reader forms / analysis nodes stay on
115+
the persistent node arena as today (they can be referenced by
116+
persisted defs — F-006's allocator layering). The bencode codec
117+
needs NO change: `DecodeError.UnexpectedEof` fires only on truncation
118+
(→ buffer more, up to the size cap), other decode errors are protocol
119+
errors (→ close). The size cap is load-bearing: a huge declared
120+
string length (`999999999:…`) reads as truncation forever without it.
121+
122+
**describe versions**: `{cljw: <build_options.version>, nrepl:
123+
{version-string: "1.3.1"}}` — deliberately **no** `clojure` key.
124+
This is the exact babashka shape (its describe payload is
125+
`{"versions" {"babashka" <ver>, "babashka.nrepl" <ver>}}`, no
126+
`clojure` key). CIDER keys its runtime detection on
127+
`versions.clojure`; claiming it flips CIDER to 'clojure runtime and
128+
triggers cider-nrepl middleware-missing warnings on every connect.
129+
'generic runtime (like let-go before its upstream CIDER patch)
130+
degrades gracefully while all op-gated features (completion, eldoc)
131+
work. Upstreaming a `cljw` runtime key to CIDER is a
132+
separately-tracked idea, not a blocker.
133+
134+
## Principled compromises (recorded, babashka-matching)
135+
136+
| Compromise | Rationale | Tracking |
137+
|---|---|---|
138+
| No `stdin` op | single-threaded `read-line` semantics need the concurrency area | D-117 residual |
139+
| No mid-eval `interrupt` (acks `done`+`session-idle`) | a hung eval owns the only thread; true interrupt = thread-per-session (gap area I) | D-117 (a) |
140+
| No cider-nrepl extension middleware (debugger/inspector/test-runner/apropos ops) | separate ~15 kLOC JVM-introspection project; CIDER degrades gracefully | out of scope (F-014 goal line) |
141+
| `out`/`err` streamed per top-level form, not per write | **A divergence FROM babashka** (bb's proxy Writer pushes an `out` response per write, mid-eval — the DA corrected the draft's "bb-proven" claim). ADR-0138 capture shape; user-visible symptom: a long-running form's prints arrive only when the form returns | D-118 re-narrowed to this |
142+
143+
## Consequences
144+
145+
- CIDER works end-to-end against the base protocol: REPL buffer
146+
evaluation, streaming out, completion, eldoc, doc lookup, load-file
147+
of real-size files, rich error text identical to the CLI.
148+
- `test/e2e/phase14_nrepl.sh` grows CIDER-replay coverage: distinct
149+
clone ids, session echo, pipelined burst, >4 KiB frames, the error
150+
triple, completions/lookup shapes, describe derivation. The old
151+
single-session happy-path e2e was green while CIDER was broken —
152+
the proxy-test lesson (memory `verify_actual_pattern_not_proxy`).
153+
- The CLI REPL gains `*1`/`*2`/`*3`/`*e` in the same cycle via the
154+
shared `eval_session.zig` engine (the F-011 dividend that proves the
155+
extraction earned its keep).
156+
- D-117 is re-narrowed by code-truth (its "CIDER ops LANDED" claim was
157+
false — only load-file had landed); D-118's remaining gap narrows to
158+
per-write streaming.
159+
- `structure_plan.md`'s single-file `nrepl_server.zig` note and the
160+
`runtime/cljw/repl/NReplServer.zig` reservation are superseded by
161+
this package shape (reservations are memos, F-002); if a
162+
`(cljw.repl/start-server!)` Clojure surface ever lands, the
163+
app-layer engine is reachable via the `driver.installVTable`
164+
precedent — no code now.
165+
- `line_editor.zig` sheds its private candidate enumeration for the
166+
shared `runtime/introspect.zig` (behaviour-preserving extraction,
167+
cap 64 kept at the caller).
168+
- The stale `"0.1.0-pre"` version lie is structurally impossible
169+
(single source: `build_options.version`).
170+
171+
## Alternatives considered
172+
173+
(Devil's-advocate subagent output, fresh-context fork 2026-07-13,
174+
reflected per CLAUDE.md § ADR-level designs. Its leading finding: no
175+
F-NNN blocks any shape. Its fact-checks — the draft's "per-form out is
176+
bb-proven" claim was FALSE (bb streams per write), `*err*` capture was
177+
missing, the "runs before bootstrap" argument for Zig-side introspection
178+
was factually wrong (the server is always post-bootstrap; the real
179+
argument is the keystroke path + F-011), the arena-growth gap, and the
180+
bencode-already-distinguishes-truncation observation — are folded into
181+
Context/Decision above.)
182+
183+
**Alternative 1 — smallest-diff (fix-in-place, one file).** Keep
184+
`nrepl.zig` single-file (~900 LOC): growable buffer, per-clone counter,
185+
3-message errors, stop-on-error, `build_options.version`, completions
186+
via `pub`-ing `line_editor`'s collect fns. Better: zero structural
187+
churn, each fix independently verifiable. Breaks: sharing completion
188+
logic by exposing private methods of a raw-terminal line editor is the
189+
ad-hoc coupling F-011 forbids; an app-layer home forecloses any future
190+
Layer-2 surface; the if-else op chain keeps the describe-drift class.
191+
DA verdict: "reaches a different, worse finished form — disqualified
192+
under F-002 since the probe evidence shows five orthogonal concern
193+
clusters (framing, sessions, ops, eval, introspection)".
194+
195+
**Alternative 2 — finished-form-clean (draft + four deepenings) —
196+
RECOMMENDED and ADOPTED.** (a) Extract the shared eval-session core
197+
NOW: once `*1`..`*e` are interned dynamic Vars, the rotation logic is
198+
identical for both REPLs; deferring it is "the Cycle-budget-defer smell
199+
wearing a 'correct sizing' costume" — nREPL-only `*1` (works in CIDER,
200+
absent in `cljw repl`) is indefensible under F-011. (b) Session state
201+
rooted via the existing pin surface + gc_rooting.md row, not a new root
202+
class. (c) `runtime/introspect.zig` with typed candidates; cite F-011
203+
commonization + the zone graph (NOT F-009, whose out-of-scope clause
204+
covers env.zig). (d) Transport scratch-arena split + `*err*` capture +
205+
honest per-form-out divergence row. Cost: substantially larger diff
206+
(repl.zig + bootstrap + gc_rooting.md ride along) — recommended anyway
207+
per F-002; the risk (eval-extraction disturbing the AnalysisFrame
208+
bracket placement, D-556 class) is covered by the existing smoke +
209+
phase14 e2e.
210+
211+
**Alternative 3 — wildcard: Clojure-side ops (the literal bb
212+
transplant).** Implement completions/lookup/info/eldoc in `.clj`
213+
(bb's `impl/sci.clj` shape) — maximum Clojure-level commonization,
214+
dogfoods cljw, no new Zig surface. Breaks: the line editor's TAB path
215+
would either eval per keystroke inside a raw-mode terminal (GC +
216+
output-capture + error isolation on the keystroke path) or keep its
217+
Zig walk — recreating the duplication across a language boundary where
218+
drift is harder to see; needs a generic Value→bencode walker; a throw
219+
inside a completions eval must not rotate the session's `*e`. DA
220+
rating: "the right *eventual* direction for lookup-class ops
221+
(data-shaped, latency-tolerant), wrong today for keystroke-shared
222+
completion enumeration" — recorded as a forward note: lookup-class ops
223+
MAY migrate to `.clj` when a `cljw.repl` ns lands (F-014 clause 4
224+
would accept a `(cljw.repl/start-server!)` surface; the app-layer
225+
engine would then be reached via the `driver.installVTable` precedent).
226+
227+
Judgment-call stress-tests adopted from the DA: transport owns the
228+
session/id echo with the clone edge (echo the parent session, emit the
229+
fresh `new-session`); the op table + describe derive from ONE comptime
230+
tuple (StaticStringMap does not iterate cleanly); aliases are separate
231+
entries pointing at one handler so describe advertises them all;
232+
`interrupt` acks `["done" "session-idle"]` when idle (spec) instead of
233+
bare done; per-session state is kept (JVM nREPL sessions are per-session
234+
binding maps — the F-011 oracle; bb's global `set!` across id-tags is
235+
bb's simplification and would let tooling-session evals rotate the
236+
user's `*1`).
237+
238+
## Revision history
239+
240+
- 2026-07-13: Proposed.

0 commit comments

Comments
 (0)