Skip to content

[Audit] DAP client — 7 verified findings (4 medium, 3 low) #579

Description

@cuttlefisch

Batched findings for DAP client from the pre-v0.15 codebase audit (branch audit/pre-v015-review).

Each was produced by an end-to-end capability trace, then independently re-verified by a reviewer whose
brief was to refute it
. Only findings that survived refutation appear here, at the verifier's corrected
severity — across the audit, 102 claims became 89 confirmed and 29 were downgraded.

7 findings — 4 medium, 3 low. Tick them off individually; split any one
out into its own issue if it needs real design work.


1. Breakpoint/stopped-line source paths are canonicalized on some paths and not others, so gutter markers silently disagree with real breakpoints

bug · medium

Impact. Open a file relatively (mae src/main.rs, or :e src/main.rs). The human presses SPC d b on line 10 -> key "src/main.rs". The AI calls dap_set_breakpoint(source:"src/main.rs", line:10) -> key "/home/u/proj/src/main.rs". Result: two entries in DebugState.breakpoints for the same physical line, two separate SetBreakpoints intents sent to the adapter under two different source paths, dap_remove_breakpoint from the AI cannot remove the human's breakpoint (returns an empty remaining_lines and reports success), and the gutter renders a for only one of them. std::fs::canonicalize also resolves symlinks, so even an absolute buffer path under a symlinked project root mismatches. The stopped-line marker fails outright whenever the adapter reports a path spelling different from the buffer's (or falls back to source.name, a bare filename), which is the normal case for debugpy/lldb.

Evidence

crates/core/src/editor/dap_ops.rs:267-274 canonicalizes: "pub fn dap_set_breakpoint(&mut self, source_path: String, line: i64) -> Vec { … let abs_path = canonicalize_source_path(&source_path); self.mutate_breakpoint(abs_path, line, /* ensure_present = */ true) }" — and dap_remove_breakpoint:279-285 and dap_set_breakpoint_conditional:166 do the same.
But the cursor toggle does NOT: dap_ops.rs:220-239 "let file_path = self.buffers[buf_idx].file_path().map(|p| p.to_string_lossy().into_owned()); … let source_path = match (file_path, is_dap) { (Some(p), _) => p, … }; … let remaining_lines = state.toggle_breakpoint_at(source_path.clone(), line);" — the raw buffer path is used as the map key.
crates/core/src/buffer.rs:677 stores the path exactly as given: "file_path: Some(path.to_path_buf())," (no canonicalization; open_file_hidden at crates/core/src/editor/file_ops.rs:1286-1292 canonicalizes only for the already-open comparison).
The gutter then does an exact string lookup: crates/core/src/render_common/gutter.rs:138-140 "if let (Some(path), Some(state)) = (buf.file_path(), editor.dap.state.as_ref()) { let path_str = path.to_string_lossy(); if let Some(list) = state.breakpoints.get(path_str.as_ref()) {".
Stopped-line is worse — never normalized at all, and can be a bare filename: crates/mae/src/dap_bridge.rs:171 "let src = f.source.and_then(|s| s.path.or(s.name));" fed to dap_ops.rs:609 "state.set_stopped_location(src, line);" and compared by gutter.rs:148 "if src.as_str() == path_str.as_ref()".
The existing canonicalization regression tests (dap_ops.rs:1550-1646) deliberately cover set/remove/start but never the toggle path, and every toggle test uses an already-absolute unicorn path (dap_ops.rs:934 "/tmp/a.rs") so the gap is invisible (principle #14).

Verification

Every cited line checks out. dap_ops.rs:272 (dap_set_breakpoint), :283 (dap_remove_breakpoint) and :166 (dap_set_breakpoint_conditional) all call canonicalize_source_path, which is std::fs::canonicalize with a CWD-join fallback (dap_ops.rs:783-796). dap_toggle_breakpoint_at_cursor (dap_ops.rs:213-252) does NOT: it uses self.buffers[buf_idx].file_path().map(|p| p.to_string_lossy().into_owned()) verbatim as the map key (state.toggle_breakpoint_at(source_path.clone(), line)) and then hands the same raw string to push_set_breakpoints_from_state(source_path) (dap_ops.rs:323-349), which looks the key up by exact string and stamps it on the outgoing DapIntent::SetBreakpoints. Buffer::from_file stores the path exactly as given (crates/core/src/buffer.rs:677 file_path: Some(path.to_path_buf())), and the CLI passes the raw argv path (crates/mae/src/main.rs:482 Buffer::from_file(std::path::Path::new(&path))) — open_file_hidden (file_ops.rs:1284-1291) canonicalizes only for the already-open dedup comparison, never for storage. The gutter does an exact-string lookup on the buffer path (render_common/gutter.rs:137-140 let path_str = path.to_string_lossy(); if let Some(list) = state.breakpoints.get(path_str.as_ref())) and the stopped marker compares src.as_str() == path_str.as_ref() (gutter.rs:147-148), with src sourced from f.source.and_then(|s| s.path.or(s.name)) (crates/mae/src/dap_bridge.rs:171) — i.e. possibly a bare filename — and never normalized (dap_ops.rs:609 state.set_stopped_location(src, line)). The test-gap claim also holds: the four canonicalization tests at dap_ops.rs:1550-1600 cover canonicalize_source_path, dap_set_breakpoint, dap_remove_breakpoint only, and the toggle tests use the already-absolute /tmp/a.rs (dap_ops.rs:934). No open issue and no ROADMAP 'Architecture Debt' entry covers it.

Scope correction from verification: Real key-space divergence, but scoped to DAP breakpoint display/removal — no security or data-loss consequence, and it only manifests when the buffer's stored path is not already the canonical one (file opened by a relative path, or an absolute path under a symlinked root — canonicalize also resolves symlinks, so macOS /tmp -> /private/tmp mismatches too). Effects: (a) an AI/:debug-*-set breakpoint on a relatively-opened buffer is not rendered in the gutter, (b) dap_remove_breakpoint from the AI cannot remove a toggle-set breakpoint and still reports success, (c) a toggle-set breakpoint is sent to the adapter under a relative path that debugpy may not bind. The stopped-line marker mismatch is the same class but is a display-only miss. Fix is one call to canonicalize_source_path in dap_toggle_breakpoint_at_cursor plus normalizing set_stopped_location.


2. dap_set_breakpoint advertises a log_message logpoint parameter that is never read — a logpoint request halts the program and is reported as success

bug · medium

Impact. An agent calls dap_set_breakpoint(source, line, log_message: "x={x}") expecting a non-stopping trace point. It gets {"source":…,"line":…,"all_lines_for_source":[…]} and success: true, but a plain stopping breakpoint was installed. The debuggee halts where the agent believed it would only log; the agent's next dap_continue (15s deferred window) then races an unexpected stop. A failure is reported as a success, and the docs assert the capability exists.

Evidence

Advertised in the tool schema, crates/ai/src/tools/dap_tools.rs:57-61: ".prop(\n "log_message",\n "string",\n "Optional log message (logpoint) — instead of stopping, logs a message. Expressions in {braces} are evaluated.",\n)".
The implementation never reads it — crates/ai/src/tool_impls/dap.rs:89-131 pulls only source, line, condition, hit_condition, then returns success JSON built from source/line/all_lines_for_source/condition/hit_condition. log_message appears nowhere in the function.
There is no plumbing for it anywhere: the wire struct has three fields only — crates/dap/src/protocol.rs:149-155 "pub struct SourceBreakpoint { pub line: i64, … pub condition: Option, … pub hit_condition: Option, }" — and mae_core::dap_intent::BreakpointSpec / mae_core::debug::Breakpoint likewise carry no log-message field. A repo-wide grep for log_message|logMessage|logpoint hits only dap_tools.rs:58-60 and docs/COMPETITIVE_ANALYSIS.md:66, which claims the feature ships: "| Conditional breakpoints / logpoints | Y |".

Verification

Verified verbatim. crates/ai/src/tools/dap_tools.rs:57-61 declares .prop("log_message", "string", "Optional log message (logpoint) — instead of stopping, logs a message. Expressions in {braces} are evaluated."). execute_dap_set_breakpoint (crates/ai/src/tool_impls/dap.rs:89-131) reads only source, line, condition, hit_condition, dispatches to dap_set_breakpoint_conditional/dap_set_breakpoint, and builds the result JSON from source/line/all_lines_for_source/condition/hit_condition; the identifier log_message does not appear in the file. There is no plumbing anywhere: crates/dap/src/protocol.rs:147-155 pub struct SourceBreakpoint { pub line: i64, condition: Option<String>, hit_condition: Option<String> }, crates/core/src/dap_intent.rs:69-73 BreakpointSpec { line, condition, hit_condition }, crates/core/src/debug.rs:64-75 Breakpoint { id, verified, source, line, condition, hit_condition } — none carries a log message. A repo-wide grep for log_message|logMessage|logpoint hits only dap_tools.rs:58-60, crates/core/src/debug.rs:4 (an unrelated comment), and three docs that assert the capability ships: README.md:39 ("Breakpoints (conditional, logpoint)"), CLAUDE.md:458 ("Set breakpoint (conditional/logpoint)"), docs/COMPETITIVE_ANALYSIS.md:66 ("| Conditional breakpoints / logpoints | Y |"). Not tracked by any open issue.

Scope correction from verification: Confirmed as an unimplemented advertised parameter plus three overstated docs claims, not a high-severity defect: the tool's own top-level description only promises condition/hit_condition, and the failure mode is a silently-ignored optional argument (an ordinary stopping breakpoint gets installed). The narrow accurate claim is 'the log_message schema property and the README/CLAUDE.md/COMPETITIVE_ANALYSIS logpoint claims describe a feature that has zero implementation — the parameter is dropped without a warning'.


3. AI/MCP-originated DAP intents bypass the in-process Scheme DAP bridge, unlike the LSP path 16 lines below

bug · medium

Impact. During an active mae-scheme debug session, any AI tool call that queues an intent (dap_set_breakpoint, dap_continue, dap_step, dap_evaluate) is drained straight to dap_bridge::intent_to_dap_command and forwarded to run_dap_task, which for a StartSession tries to spawn a subprocess literally named mae-scheme (DapClient::start -> Command::new(&config.command) at crates/dap/src/client.rs:87-93) and fails with "failed to spawn debug adapter 'mae-scheme'". For continue/step during an already-open scheme session there is no session in the external task at all, so the manager replies Error { message: "no DAP session active" } (crates/dap/src/manager.rs:404-410), which try_resolve_deferred_dap's catch-all Error arm (ai_event_handler.rs:1268-1274) turns into a hard tool failure — while the human pressing SPC d c in the same session works fine. Debugging Scheme is a human-only capability by accident.

Evidence

crates/mae/src/scheme_dap_bridge.rs:27-29 documents the required ordering: "/// Call this BEFORE drain_dap_intents so scheme DAP intents never reach the\n/// external DAP task. …\npub(crate) fn drain_scheme_dap_intents(editor: &mut Editor, scheme: &mut SchemeRuntime) -> bool". All three real event loops honour it — crates/mae/src/terminal_loop.rs:357-358, crates/mae/src/gui_app.rs:407-408, crates/mae/src/headless_loop.rs:332-333.
The AI tool handler does not. crates/mae/src/ai_event_handler.rs:222-223: "if editor.has_pending_dap_intents() {\n crate::dap_bridge::drain_dap_intents(editor, ctx.dap_command_tx);\n}" and :229 for the deferred branch: "crate::dap_bridge::drain_dap_intents(editor, ctx.dap_command_tx);".
The LSP branch immediately below gets it right — ai_event_handler.rs:245-246: "crate::scheme_lsp_bridge::drain_scheme_lsp_intents(editor, ctx.scheme);\ncrate::lsp_bridge::drain_lsp_intents(editor, ctx.lsp_command_tx);" — which shows the DAP omission is an oversight, not a decision.
The adapter preset for scheme is explicitly in-process: crates/core/src/editor/dap_ops.rs:826-830 ""scheme" | "mae-scheme" => Some(DapSpawnConfig { command: "mae-scheme".into(), // In-process — no subprocess spawned".

Verification

Every quoted line checks out. crates/mae/src/scheme_dap_bridge.rs:27-29 really does say "Call this BEFORE drain_dap_intents so scheme DAP intents never reach the external DAP task", and drain_scheme_dap_intents gates on is_scheme_dap (adapter_name == "mae-scheme", :14-23). All three event loops honour the ordering: terminal_loop.rs:357-358, gui_app.rs:407-408, headless_loop.rs:332-333 (verified by grep — those are the only three paired call sites). crates/mae/src/ai_event_handler.rs:222-223 and :229 call crate::dap_bridge::drain_dap_intents(editor, ctx.dap_command_tx) with no preceding scheme drain, while the sibling LSP branch at :245-246 does call scheme_lsp_bridge::drain_scheme_lsp_intents first. dap_bridge::drain_dap_intents (dap_bridge.rs:10-25) has no target check of its own — it takes ALL pending intents unconditionally — so there is no upstream gate. crates/ai/src/tool_impls/dap.rs contains no mae-scheme/adapter_name guard either (grep: only "lldb" at :473). The preset at dap_ops.rs:826-830 is command: "mae-scheme".into(), // In-process — no subprocess spawned and no mae-scheme binary exists in either workspace (no such [[bin]] anywhere), so a StartSession routed to the external task would attempt Command::new("mae-scheme") and fail. Scheme DAP is a shipped feature (ROADMAP.md:316 "Phase 13g: LSP + DAP for mae-scheme — in-process Swank-style"), so this is a genuine principle-#3 asymmetry: the human can debug Scheme, the AI/MCP peer cannot. Not tracked in any open issue.


4. No end-to-end DAP test exists; the StartSession path — including every real subprocess and handshake behaviour — is exercised by nothing

test-gap · medium

Impact. Every bug in this report lives in exactly the code no test touches. The path-canonicalization split (finding 1) survives because the canonicalization tests at dap_ops.rs:1550-1646 cover set/remove/start and the toggle tests use pre-absolute unicorn paths. The AI scheme-bridge bypass (finding 4) survives because ai_event_handler.rs's DAP drain has no test. The dead terminate path (finding 6) is reported green by client.rs:1084 terminate_round_trip. A pure-mock suite that never spawns an adapter and never crosses the intent->bridge->task->event->apply loop confirms the implementation instead of trying to falsify it, which is precisely the failure mode principle #14 exists to prevent.

Evidence

Both manager tests explicitly route around the code under test. crates/dap/src/manager.rs:909-912: "/// Start a manager in its own task, then synthesize a completed session\n/// by hand-crafting a DapClient from streams and injecting it. This\n/// exercises the command/event translation without the StartSession\n/// spawn-subprocess path (which requires a real command on $PATH)." — and manager.rs:944-989 then reimplements the StartSession sequence inside the test task rather than calling handle_command's StartSession arm. The debugpy_style_deferred_launch_response regression test (manager.rs:1015-1117) does the same at :1045-1105. So the 340-line handle_command (manager.rs:207; ceiling is 80 per docs/AUDIT_METRICS.json, and it is not on any accepted-exception list) has its single largest arm — the ~130-line StartSession orchestration at manager.rs:233-364, with all five bail-out branches (launch/attach send failed, configurationDone failed, launch/attach rejected, response channel closed, response timed out) — covered by zero tests.
Downstream is equally untested by construction: crates/mae/src/dap_bridge.rs has test_count: 0, crates/core/src/editor/dispatch/dap.rs has test_count: 0, crates/ai/src/tools/dap_tools.rs has test_count: 0 (docs/AUDIT_METRICS.json).
The only cross-process coverage is a model-graded MCP plan, not a deterministic test: crates/ai/src/executor/self_test.rs:571-628 builds a "dap" category that is "conditional": true, gated on python3 -c "import debugpy", and grades with {"method": "output_contains", "substring": "stopped"} — five strictly happy-path steps (start / set_breakpoint / continue / output / disconnect). It never tests dap_step, a rejected breakpoint, an adapter that dies mid-session, a stale variables_reference, a launch that fails, or the 15s deferred timeout.
And because Scheme has no DAP primitives (finding 3), the mae --test tests/editor/ harness — which CLAUDE.md designates for real-event-loop behaviour — structurally cannot cover DAP: tests/ contains no DAP or breakpoint file (grep -rl 'dap\|breakpoint' tests/ is empty).

Verification

Verified. crates/dap/src/manager.rs:909-912 states in its own words that manager_with_session "exercises the command/event translation without the StartSession spawn-subprocess path (which requires a real command on $PATH)", and :944-989 re-implements the launch/configurationDone sequence inside the test task instead of calling handle_command's StartSession arm. docs/AUDIT_METRICS.json confirms test_count 0 for crates/mae/src/dap_bridge.rs, crates/core/src/editor/dispatch/dap.rs, crates/ai/src/tools/dap_tools.rs and crates/ai/src/executor/dap_exec.rs. grep -rli 'dap|breakpoint' tests/ returns 0 files, so the Scheme harness has no DAP coverage at all. crates/ai/src/executor/self_test.rs:571-628 is as described: "conditional": true, gated on python3 -c "import debugpy", five happy-path steps, output_contains: "stopped" grading, no step/failure/timeout cases.

One overstatement: "downstream is equally untested" is wrong for the AI tool-impl layer — crates/ai/src/tool_impls/dap.rs has 31 tests and crates/core/src/editor/dap_ops.rs has 51 (AUDIT_METRICS.json). The untested layers are specifically the transport orchestration (StartSession), the intent->command bridge, the keybinding dispatch arm, and the tool schema layer.

Scope correction from verification: Narrower accurate claim: the StartSession orchestration in manager.rs:233-364 (all five bail-out branches), dap_bridge.rs, dispatch/dap.rs and dap_tools.rs have zero tests, and no test anywhere spawns a real adapter deterministically. dap_ops.rs (51 tests) and tool_impls/dap.rs (31 tests) ARE covered, so the gap is the transport/bridge/dispatch seam, not the whole slice.


5. SPC d w / SPC d W are bound to commands whose keybinding dispatch can only print a usage string

bug · low

Impact. Pressing SPC d w in the which-key menu shows "add watch", then does nothing but flash a usage hint — the user must know to retype :debug-add-watch <expr> by hand. Watch expressions are also the one debug feature with no MCP tool at all, so the whole watch subsystem (dap_ops.rs:713-777, DebugState.watch_expressions, apply_watch_result) is reachable only by typing a full ex command. Both advertised keybindings are functionally dead.

Evidence

The bindings exist: modules/debug/autoloads.scm:38-39 "(define-key "leader" "d w" "debug-add-watch")\n(define-key "leader" "d W" "debug-remove-watch")".
But the keybinding dispatch arms are inert: crates/core/src/editor/dispatch/dap.rs:115-122 ""debug-add-watch" => {\n // Handled by ex-command parser with args\n self.set_status("Usage: :debug-add-watch ");\n}\n"debug-remove-watch" => {\n // Handled by ex-command parser with args\n self.set_status("Usage: :debug-remove-watch ");\n}".
The real implementations are argument-only, reachable solely from the ex parser: crates/core/src/editor/command.rs:962-981 (self.debug_add_watch(expression.to_string()) / self.debug_remove_watch(idx)).
The correct pattern for an argument-taking command bound to a key is used two arms earlier for debug-startdispatch/dap.rs:12-16 "self.set_mode(crate::Mode::Command);\nself.vi.command_line = "debug-start ".to_string();\nself.vi.command_cursor = self.vi.command_line.len();" — so the fix pattern already exists in the same 130-line file.
crates/core/src/editor/dispatch/dap.rs has test_count: 0 in docs/AUDIT_METRICS.json, so nothing catches this.

Verification

The code is exactly as quoted. modules/debug/autoloads.scm:38-39 binds d w/d W; crates/core/src/editor/dispatch/dap.rs:115-122 sets a status string only, with the comment "// Handled by ex-command parser with args"; the real implementations live at command.rs:962-981 behind the ex parser; and debug-start two arms up (dispatch/dap.rs:12-16) demonstrates the prefill pattern in the same file. docs/AUDIT_METRICS.json confirms crates/core/src/editor/dispatch/dap.rs has test_count 0. The watch parity half also checks out: crates/ai/src/tools/dap_tools.rs has 13 ToolDefBuilder::new entries and none is a watch tool (only dap_evaluate's context: "watch" enum value), and execute_command/execute_command_dispatch (core_exec.rs:36-49) calls dispatch_builtin(cmd) with no argument parsing, so the AI cannot reach debug-add-watch <expr> that way either.

But "functionally dead" overstates: the binding resolves, dispatches, and prints the exact ex syntax to type. It is a UX/consistency defect (one arm in a 130-line file not following the prefill pattern its sibling uses), not a dead binding.

Scope correction from verification: Narrower accurate claim: debug-add-watch/debug-remove-watch are the only argument-taking commands bound to leader keys that print a usage hint instead of prefilling the command line the way debug-start does (dispatch/dap.rs:12-16 vs :115-122). Separately and more substantively: the watch subsystem (dap_ops.rs:713-777) has no MCP tool and no Scheme primitive, so it is human-and-ex-command only — a real principle-#3 gap.


6. Debug adapter binaries and the adapter preset table are configurable only via environment variables and a hardcoded match, with no OptionRegistry entry

missing-config · low

Impact. A user with lldb-dap under a versioned path, or wanting a fifth adapter (delve, js-debug, netcoredbg), cannot express it via init.scm, (set-option!), :set or :set-save — the only levers are shell env vars set before launch (invisible to :describe-configuration and to audit_configuration) or a Rust patch. The 15s deferred timeout is the sharpest edge: any debuggee that takes longer than 15s to reach the next breakpoint makes the AI's dap_continue report a timeout failure while the session is still perfectly healthy, and there is no way to raise it.

Evidence

CLAUDE.md:339 is explicit: "Every option must be Scheme-accessible: If a behavior is configurable, it goes through OptionRegistry. No config.toml-only settings, no env-var-only settings, no compile-time-only flags for user-facing behavior."
crates/core/src/editor/dap_ops.rs:798-833: "/// Read an env var, falling back to a default string. Keeps the adapter\n/// preset table below compact and overridable without touching source.\nfn env_or(var: &str, default: &str) -> String { std::env::var(var).unwrap_or_else(|_| default.into()) }" then "fn default_spawn_for_adapter(adapter: &str) -> Option { match adapter { "lldb" | "lldb-dap" | "cpp" | "c" => Some(DapSpawnConfig { command: env_or("MAE_DAP_LLDB", "lldb-dap"), … }), "codelldb" => … env_or("MAE_DAP_CODELLDB", "codelldb") …, "debugpy" | "python" => … env_or("MAE_DAP_DEBUGPY", "python") …, _ => None } }".
crates/core/src/options.rs has no DAP option at all — the only debug-named entries are opt!("debug_mode", …, Some("editor.debug_mode"), …) at options.rs:159 (status-bar RSS/CPU) and opt!("debug_panel_split_ratio", …) at options.rs:361 (panel geometry).
Other unexposed magic constants in the same slice, none with a documented rationale: stack depth sess.client.stack_trace(tid, Some(64)) (crates/dap/src/manager.rs:428); launch-response wait tokio::time::timeout(std::time::Duration::from_secs(15), launch_rx) (manager.rs:314); initialized-event wait deadline = … + Duration::from_secs(2) (manager.rs:583); per-request timeouts 5s/10s/30s throughout crates/dap/src/client.rs; and the AI deferred-DAP timeout "if state.created_at.elapsed() > std::time::Duration::from_secs(15)" (crates/mae/src/ai_event_handler.rs:1385).

Verification

The factual core holds: crates/core/src/options.rs has no DAP option (grep for dap/debug returns only debug_mode at :159 and debug_panel_split_ratio at :361), default_spawn_for_adapter (dap_ops.rs:807-833) is a closed 4-arm match returning None for anything else, and the only callers are dap_start_with_adapter[_opts] (command.rs:901, tool_impls/dap.rs:76) — the DapSpawnConfig-taking dap_start_session is not reachable from Scheme, MCP or ex-commands, so a fifth adapter genuinely requires a Rust patch. There is also no [dap] config.toml section (unlike LSP, whose bootstrap.rs:1757-1761 documents a three-level chain: env var -> config.toml [lsp] -> defaults). The hardcoded 15s deferred timeout at ai_event_handler.rs:1385 is real and hard-fails (resolve_dap_deferred(..., false, ...) at :1458).

But the severity is inflated. env_or carries a written rationale (dap_ops.rs:798-800), the env vars are a documented public contract (CLAUDE.md:368 lists MAE_DAP_LLDB/CODELLDB/DEBUGPY) and ARE surfaced diagnostically (crates/mae/src/doctor.rs:269 names MAE_DAP_CODELLDB in its remediation text), so "invisible to introspection" overstates. The other cited constants are internal protocol timeouts inside crates/dap (manager.rs:314 launch wait, :583 initialized wait, :428 stack depth 64, client.rs per-request timeouts) — CLAUDE.md #7 explicitly exempts "constants that are truly fixed (buffer sizes, protocol limits)", so bundling them inflates the count.

Scope correction from verification: Narrower accurate claim: DAP adapters have no OptionRegistry entry AND no config.toml section (LSP has the latter), so adding an adapter or pinning a versioned binary path requires a Rust patch or a pre-launch env var; and the AI's deferred-DAP wait is a hardcoded 15s hard failure (ai_event_handler.rs:1385) with no lever. The crates/dap internal timeouts and the stack-depth 64 are protocol constants, not #7 violations.


7. The DAP terminate (soft-stop) request is fully plumbed but unreachable — dap_terminate has no caller on any surface

structural · low

Impact. Green mock tests (terminate_round_trip) make the soft-terminate path look shipped while no user or agent can invoke it, so a regression in it can never be caught by anything but that mock. Concretely there is no way to ask the debuggee to shut down gracefully (terminate) rather than being killed (disconnect{terminateDebuggee:true}) — which matters for debuggees with atexit/cleanup handlers. Six files carry code for a feature with zero reachable entry points.

Evidence

The producer is dead: crates/core/src/editor/dap_ops.rs:432-436 "/// Terminate (soft stop) the debuggee.\npub fn dap_terminate(&mut self) {\n self.dap.pending_intents.push(DapIntent::Terminate);\n self.set_status("[DAP] terminating...");\n}". A repo-wide grep for dap_terminate returns exactly two hits — this definition and the unrelated event handler apply_dap_terminated (dap_ops.rs:538). No command arm in crates/core/src/editor/dispatch/dap.rs, no arm in crates/core/src/editor/command.rs, no MCP tool in crates/ai/src/tools/dap_tools.rs, no Scheme primitive.
The rest of the chain exists and is maintained: DapIntent::Terminate => DapCommand::Terminate (crates/mae/src/dap_bridge.rs:106), the name table entry (dap_bridge.rs:42), the manager arm (crates/dap/src/manager.rs:535-541), the client request (crates/dap/src/client.rs:498-501), and a passing round-trip test (client.rs:1084-1091 terminate_round_trip).
"debug-stop" — the only user-facing stop — deliberately uses the hard path instead: crates/core/src/editor/dispatch/dap.rs:23-24 "if is_dap {\n self.dap_disconnect(true);".
Relatedly, the negotiated capability that would gate this is parsed and then never consulted: crates/dap/src/protocol.rs:108-109 "#[serde(default)]\n pub supports_terminate_request: bool," — its only readers in the whole repo are assertions inside crates/dap/src/client.rs:802.

Verification

Verified dead. Repo-wide grep for dap_terminate returns exactly crates/core/src/editor/dap_ops.rs:433 (the definition) and the unrelated apply_dap_terminated at :538. DapIntent::Terminate is produced nowhere; its only consumers are the crosswalk dap_bridge.rs:106, the name table dap_bridge.rs:42, and scheme_dap_bridge.rs:194. "debug-stop" (the only user-facing stop, dispatch/dap.rs:17-30, bound at modules/debug/autoloads.scm:29) uses self.dap_disconnect(true). supports_terminate_request (protocol.rs:108-109) is read only by an assertion at client.rs:802.

Severity is inflated, though. This is unreachable code, not a broken advertised capability: no command, keybinding, Scheme primitive or MCP tool ever promised soft-terminate, so nothing a user or agent can invoke is failing. Principle #5 is about module boundaries/bus factor, not dead branches. The real content is ~15 lines of dead plumbing plus a mock test that reports it green.

Scope correction from verification: Narrower accurate claim: DapIntent::Terminate is unreachable dead code (producer Editor::dap_terminate has zero callers) whose round-trip test at client.rs:1084 reports it green. No user-visible capability is broken — debug-stop deliberately uses disconnect{terminateDebuggee:true}. Fix is either wiring a debug-terminate command/tool or deleting the branch.

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:editorEditor core (buffers, dispatch, notifications)tech-debtRefactor / cleanup / consistency

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions