samiandClaude Sonnet 5
4b279e8271
The daemon's persistent state. SQLite in WAL mode, foreign keys on,
5 s busy timeout so a writer waits rather than SQLITE_BUSY under the
RPC loop.
- store/sqlite — RAII Db/Stmt over the C API; errors returned as
DbResult<T> (std::expected), never thrown — the RPC loop must not
unwind. transaction() helper: BEGIN / fn / COMMIT, ROLLBACK on error.
- store/migrations/0001_initial.sql — the eight tables from the brief:
settings, categories, queues, tasks, segments, rules, history,
pairings. Notable choices:
* tasks columns project onto proto TaskSummary with no computation;
requested vs effective segments/buffer split per ADR 0010/0012;
pause_reason column per ADR 0013.
* segments end_byte is NOT constrained >= 0 so a whole-file
zero-length download is one row with end_byte = -1 (ADR 0010 B3a).
* pairings stores only token_sha256 — the plaintext token is
returned once from session.pair and never persisted (CLAUDE.md §4).
* indices on tasks(state), (category_id), (queue_id, queue_position),
(created_at), (completed_at) for the "1000 tasks, download.list
under 50 ms" DoD.
* six built-in categories + a Main queue seeded.
- store/migrations — runs every embedded migration past PRAGMA
user_version, each in its own transaction, forward-only. SQL files
are embedded at build time by cmake/embed_migrations.cmake.
Test veloxd.store_migrations (ASan+UBSan and TSan clean): fresh DB ->
head, all tables present, seed rows, FK cascade (segment orphan
rejected, task delete cascades), the end_byte=-1 zero-length case,
idempotent re-run, and forward-only from every released user_version.
Also: daemon/docs/proto-requests-m1.md — P1 marked landed on lane/proto
as 1.4.0 (HandlerError/HandlerResult), to be adopted in rpc/ once that
merges to main; P2 resolved.
Not linked into the running daemon yet — the store is wired to the
dispatcher when download.add/list/get get real bodies, next.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_01Upd9WhG9oppieig5nRDLig
93 lines
4.7 KiB
Markdown
93 lines
4.7 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.
|
|
|
|
---
|
|
|
|
## Status
|
|
|
|
- **P1 — landed** on `lane/proto` as `contracts/` **1.4.0** (commit `5e3e215`), as the
|
|
`HandlerError` / `HandlerResult<T>` sketch below. Wire is byte-identical; C++-binding
|
|
bump only. `rpc/` adopts it (the predicted `Result<T>` → `HandlerResult<T>` swap on the
|
|
`on_*` overrides) **once `lane/proto` merges to `main`** — not against the unmerged
|
|
branch. `uds_roundtrip`'s `-32603`-collapse guard flips to `-32010` in the same change.
|
|
- **P2 — resolved.** `session.hello.version-mismatch`'s `data.expected` is now `$any`;
|
|
the error-fixture compare is on `code` only, so `rpc/` echoes `kProtocolVersion` there.
|
|
PROTO's writeup: `contracts/proto-answers-daemon-m1.md`.
|
|
|
|
---
|
|
|
|
## 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.
|