samiandClaude Sonnet 5
e60d6669d8
First real code in daemon/. veloxd now listens on
$XDG_RUNTIME_DIR/velox/velox.sock (0600, SO_PEERCRED same-UID check),
frames NDJSON, and routes every method through the generated
velox::proto::dispatch(). The CLI and GUI have a server to talk to.
Modules:
- rpc/ndjson.hpp — newline-delimited framing, 8 MiB frame cap, CRLF-
tolerant, partial-tail buffering. Header-only, tested.
- rpc/event_loop — single-threaded poll(2) reactor; never blocks the
loop. stop()/wake() are async-signal-safe (eventfd).
- rpc/runtime_dir — $XDG_RUNTIME_DIR/velox resolution, 0700, owner-checked;
refuses an insecure fallback rather than using /tmp.
- rpc/uds_server — listener + non-blocking per-conn read/write with
backpressure; handles session.hello (protocol-major
check -> -32001, sessionId, transport=uds) and
session.subscribe in the server layer; routes the
rest through dispatch().
- rpc/dispatcher — VeloxDispatcher : proto::Dispatcher, all 39 methods.
download.list answers an empty table; the rest return
"not implemented" (-> -32603) until the store lands.
- main.cpp — abstract-namespace single-instance lock, signal ->
clean shutdown, socket unlinked on exit.
Tests (ASan+UBSan and TSan clean):
- veloxd.ndjson — framing edge cases
- veloxd.uds_roundtrip — real socket: hello ok / version mismatch / empty
list / -32601 / -32700 / pipelined requests, and a
guard on the -32603 collapse documented in P1.
Known gap, filed not worked around: daemon/docs/proto-requests-m1.md P1 —
the generated Dispatcher has no error channel below -32603, so handlers
cannot yet return -32010/-32011/-32013 with their data payloads. The
server layer handles -32001/-32002/-32003 around dispatch(); genuine
in-handler errors collapse to -32603 until PROTO gives handlers a real
error return. Three error fixtures are non-conformant until then.
Not in this drop: rpc/ws_server (next; needs the store for hashed pairing
tokens), store/, sched/, cli/. WS reuses this event loop.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig
80 lines
4.1 KiB
Markdown
80 lines
4.1 KiB
Markdown
# DAEMON → PROTO — requests against `contracts/` (and its codegen)
|
|
|
|
Status: **open**. Raised by lane DAEMON while building `rpc/` against `1.3.0`.
|
|
PROTO owns `contracts/`, including `contracts/codegen/`. Ranking per
|
|
`contracts/README.md` rule 4: a codegen output-shape change that every server must
|
|
adopt is effectively **major for the C++ binding** even when the wire is untouched —
|
|
it needs a version note and a regen, not a silent change.
|
|
|
|
---
|
|
|
|
## P1. The generated `Dispatcher` has no error channel below `-32603` — **blocking a conformant server**
|
|
|
|
`velox::proto::Dispatcher`'s 39 methods each return `Result<T>` =
|
|
`std::expected<T, ParseError>`, and `dispatch()` maps **every** handler error to
|
|
`ErrorCode::InternalError` (`-32603`):
|
|
|
|
```cpp
|
|
auto r = handler.on_download_get(*p);
|
|
if (!r)
|
|
return make_error(id, ErrorCode::InternalError, r.error().message,
|
|
nlohmann::json{{"path", r.error().path}});
|
|
```
|
|
|
|
So a handler cannot return any of the contract's own error codes. The error fixtures
|
|
in `contracts/fixtures/errors/` that a live server must satisfy (DAEMON DoD: "passes
|
|
the full conformance suite as a server, over both transports") include:
|
|
|
|
| Fixture | Expected code | `data` | Originates |
|
|
|---|---|---|---|
|
|
| `download.get.not-found` | `-32010` | `{taskId}` | inside the handler |
|
|
| `download.add.invalid-path` | `-32011` | `{path}` | inside the handler (after canonicalization) |
|
|
| `download.probe.probe-failed` | `-32013` | `{httpStatus}` | inside the handler |
|
|
| `session.pair.rate-limited` | `-32014` | `{retryAfterSec}` | server-side gate, but cleanest expressed as a handler result |
|
|
| `session.hello.version-mismatch` | `-32001` | `{expected, actual}` | can be done server-side around `dispatch()` |
|
|
| `session.hello.not-paired` | `-32002` | — | server-side WS auth gate, around `dispatch()` |
|
|
|
|
`-32001`, `-32002`, `-32003` DAEMON can and will handle in the server layer that wraps
|
|
`dispatch()` (`-32003` is already in `dispatch()` itself). But `-32010`, `-32011`,
|
|
`-32013` are per-method **handler outcomes** — the daemon knows "no such task" only
|
|
after the store lookup, "outside allowed roots" only after `realpath()`. There is no
|
|
correct way to surface them today except misreporting as `-32603`, which the TS
|
|
conformance replay will reject on the `code` compare.
|
|
|
|
**Requested:** give the generated handler methods an error return that carries an
|
|
`ErrorCode`, a message, and a free-form `data` object. Shape is PROTO's call; a
|
|
minimal one that keeps `ParseError` for the parse path and adds a handler-error type:
|
|
|
|
```cpp
|
|
struct HandlerError {
|
|
ErrorCode code{ErrorCode::InternalError};
|
|
std::string message;
|
|
nlohmann::json data{nullptr};
|
|
};
|
|
template <class T> using HandlerResult = std::expected<T, HandlerError>;
|
|
// Dispatcher::on_* return HandlerResult<T>; dispatch() forwards code/message/data
|
|
// straight into make_error() instead of hard-coding InternalError.
|
|
```
|
|
|
|
`FixtureDispatcher` and `conformance_main.cpp` would need the trivial follow-on edit
|
|
(they only ever return success today, so it is a type-name swap).
|
|
|
|
Until this lands, DAEMON's `rpc/` server layer handles `-3200x` around `dispatch()`
|
|
where it can, and every genuine in-handler failure collapses to `-32603` with a clear
|
|
message — visibly non-conformant on three error fixtures, tracked here, not worked
|
|
around by inventing a side channel.
|
|
|
|
---
|
|
|
|
## P2. `SessionHelloResult.transport` and `-32001` `data.expected` — minor clarifications
|
|
|
|
- `session.hello.version-mismatch`'s `data.expected` is `"1.0.0"` in the fixture, i.e.
|
|
the daemon's *current* protocol version string, not a bare major. DAEMON will echo
|
|
`kProtocolVersion` (`"1.3.0"`) there unless PROTO wants the fixture's literal
|
|
`"1.0.0"` preserved — flag if the conformance compare is exact on that field rather
|
|
than structural.
|
|
- `SessionHelloResult.transport` is `std::optional` — DAEMON intends to always populate
|
|
it (`"uds"` / `"ws"`) so a client knows its privilege level up front, as the field's
|
|
own description invites. No change requested; noting the intent so a later "why is
|
|
this always set" review has the answer.
|