uni: Refactor udb dap messages

[JosephAbdo]

No description provided.

[Jafaral]

@greptile review

[greptile-apps]

Greptile Summary

This PR refactors the UDB DAP adapter layer by introducing a new DAP_Message wrapper class, splitting JRPC_HTTPSocket into a base JSON_HTTPSocket (which returns raw tables) and a subclass JRPC_HTTPSocket (which returns JRPC_Message objects), and replacing the hand-rolled HTTP framing loop in get_request() with a clean delegation to json_handler.get_msg(). It also adds a standalone test_adapter interactive client for manual DAP testing.

Key changes and observations:

• DAP_Message.from_table() correctly implements the \t["type"] guard and get_command() correctly uses \t["command"] | t["event"], addressing all accessor correctness issues raised in previous reviews.
• get_request() is greatly simplified and correctly uses if msg := DAP_Message().from_table(t) then suspend msg to avoid suspending a stale value on an invalid message.
• get_body() always returns a table() (never &null), making the \response_body & guard in the runInTerminal handler redundant and potentially misleading — the effective guard is the member() check alone.
• Responses and events are still assembled and sent via raw writes(sock, ...) rather than json_handler.send_msg(), creating an asymmetry where receive goes through JSON_HTTPSocket but send bypasses it.
• test_adapter.icn drains follow-up messages with a short 200 ms timeout loop rather than a fixed read count, which is a more robust approach than the previous design.
• Minor: dap_message.icn and test_adapter.icn are missing a trailing newline.

Confidence Score: 4/5

• Safe to merge; all critical issues from previous reviews have been addressed and no new blocking bugs were introduced.
• The core correctness fixes (from_table guard, get_command null-guard, simplified get_request) are properly implemented. Remaining concerns are a redundant \response_body guard (cosmetic), the send/receive asymmetry (design debt), and the implicit null-socket pattern in the test client (minor). None are blocking for merge.
• No files require special attention for merge, though uni/udb/adapter.icn carries the send/receive asymmetry worth tracking for a follow-up.

Important Files Changed

Filename	Overview
uni/udb/dap_message.icn	New DAP_Message wrapper class; correctly implements from_table guard and \t["command"] fallback as recommended by previous reviews.
uni/lib/jsonrpc.icn	Splits JRPC_HTTPSocket into a base JSON_HTTPSocket and a subclass; inheritance structure is correct, chunk-based reads are an improvement.
uni/udb/adapter.icn	Refactored to use DAP_Message objects; get_request() simplified, Content-Length header space fix applied, watchpointsList typo corrected. Responses are still sent via raw sock rather than json_handler, which is a minor inconsistency.
uni/udb/test_adapter.icn	New interactive DAP test client; drains messages with a short 200ms timeout loop, which is better than a fixed read count.
uni/udb/Makefile	Adds dap_message.icn to the udb build and introduces a test_adapter build target; dependency declarations look correct.
uni/ulsp/server.icn	Inline comments added for clarity only; no functional changes.

Sequence Diagram

sequenceDiagram
    participant VSCode as VS Code Client
    participant JSON_HTTPSocket
    participant get_request as Adapter.get_request()
    participant DAP_Message
    participant process_request as Adapter.process_request()

    VSCode->>JSON_HTTPSocket: HTTP-framed DAP JSON
    JSON_HTTPSocket->>JSON_HTTPSocket: get_http_header()
    JSON_HTTPSocket->>JSON_HTTPSocket: get_data()
    JSON_HTTPSocket->>JSON_HTTPSocket: jtous(body)
    JSON_HTTPSocket-->>get_request: raw table t

    get_request->>DAP_Message: DAP_Message().from_table(t)
    alt t["type"] present
        DAP_Message-->>get_request: self (valid msg)
        get_request-->>process_request: suspend msg
    else t["type"] absent
        DAP_Message-->>get_request: fail (skipped)
    end

    process_request->>process_request: msg.get_seq() / get_command() / get_arguments()
    process_request->>VSCode: writes(sock, build_response/build_event)

Loading

Prompt To Fix All With AI

This is a comment left during a code review.
Path: uni/udb/adapter.icn
Line: 171

Comment:
**`get_body()` always returns a table, making `\response_body` redundant**

`msg.get_body()` is declared as `return \t["body"] | table()` — it never returns `&null`. This means `\response_body` on this line is always non-null, so the guard is unconditionally true and provides no protection. The only real filter is the `member(response_body, "shellProcessId")` call, which is correct.

This is not a runtime bug today, but the misleading `\response_body &` check implies `response_body` can be null (which it cannot), and could lead a future developer to trust it as a meaningful guard.

Consider removing the redundant check to keep the intent clear:

```suggestion
         "runInTerminal" : { if member(response_body, "shellProcessId") then shellProcessId := response_body["shellProcessId"]}
```

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: uni/udb/adapter.icn
Line: 180-184

Comment:
**Sending still uses raw `sock` instead of `json_handler`**

`get_request()` now receives messages through `json_handler` (`JSON_HTTPSocket`), but every response and event is still written by constructing a `"Content-Length: " || len || "\r\n\r\n" || body` string manually and calling `writes(sock, ...)` directly. `JSON_HTTPSocket.send_msg(t)` performs exactly that framing in one place.

Using `json_handler` uniformly for both receive and send would make the transport layer self-contained and reduce the number of places that hand-assemble HTTP-framed messages. For example, `build_response` / `build_event` could return plain tables and let `send_msg` handle serialisation, or the framing could at least delegate to `json_handler.send_msg`.

This is a design suggestion rather than a blocking issue, but the asymmetry means a future framing change still needs to touch multiple methods.

How can I resolve this? If you propose a fix, please make it concise.

---

This is a comment left during a code review.
Path: uni/udb/test_adapter.icn
Line: 1-3

Comment:
**`DAP_TestClient` relies on implicit null-socket initialisation**

`DAP_TestClient` inherits `JSON_HTTPSocket` but does not declare its own `initially`. When `DAP_TestClient()` is called with no arguments (as in `DAP_TestClient().connect(port)`), Unicon invokes the inherited `JSON_HTTPSocket.initially(addr, opt)` with both parameters as `&null`. The `if string(addr)` branch fails, so the else branch executes `sock := &null`, leaving `sock` null until `connect()` overwrites it.

This works today, but adding a no-arg `initially` that skips the parent's socket assignment (or documents the intentional deferred-connect pattern) would make the initialisation path explicit and prevent confusion if the parent's `initially` changes in the future.

How can I resolve this? If you propose a fix, please make it concise.

_{Reviews (4): Last reviewed commit: "udb: fix DAP message handling and JSON-R..." | Re-trigger Greptile}

[Jafaral]

@greptile review

[Jafaral]

@greptile review

[Jafaral]

@greptile review