merge: ADR 0014 — generated binding changes and versioning
This commit is contained in:
@@ -64,6 +64,10 @@ subset, `fixtures/` documents the fixture shape and the placeholder rules.
|
|||||||
renaming, retyping, or changing a default → **major** bump and a written migration note
|
renaming, retyping, or changing a default → **major** bump and a written migration note
|
||||||
in `docs/adr/`. `session.hello` rejects a major mismatch with error `-32001` and a
|
in `docs/adr/`. `session.hello` rejects a major mismatch with error `-32001` and a
|
||||||
message the GUI renders as "Velox needs updating".
|
message the GUI renders as "Velox needs updating".
|
||||||
|
"Retype → major" is about the **wire** — a field a client parses off the socket. A
|
||||||
|
change that leaves the wire byte-identical but breaks a *generated binding's* source
|
||||||
|
API (a C++ virtual's return type, a struct name) is a **minor** bump plus a migration
|
||||||
|
note — see `docs/adr/0014-generated-binding-changes-and-versioning.md`.
|
||||||
5. **Changes arrive as a PR to `contracts/` alone**, containing: schema edit + fixtures +
|
5. **Changes arrive as a PR to `contracts/` alone**, containing: schema edit + fixtures +
|
||||||
regenerated code + `VERSION` bump. Lanes rebase onto it. This is the only synchronization
|
regenerated code + `VERSION` bump. Lanes rebase onto it. This is the only synchronization
|
||||||
point in the whole project — keep it cheap and frequent rather than big and rare.
|
point in the whole project — keep it cheap and frequent rather than big and rare.
|
||||||
|
|||||||
@@ -56,12 +56,16 @@ reason.)
|
|||||||
`Result` → `HandlerResult` automatically; `conformance_main.cpp` only ever inspects
|
`Result` → `HandlerResult` automatically; `conformance_main.cpp` only ever inspects
|
||||||
`dispatch()`'s JSON output and needed no change.
|
`dispatch()`'s JSON output and needed no change.
|
||||||
|
|
||||||
**Version:** minor, 1.3.0 → 1.4.0. The wire is byte-identical — no schema, fixture, or
|
**Version:** minor, 1.3.0 → 1.4.0. The wire is byte-identical — no schema, fixture,
|
||||||
OpenRPC change — but every implementer of `Dispatcher` must swap `Result` → `HandlerResult`
|
or OpenRPC change — but every implementer of `Dispatcher` must swap `Result` →
|
||||||
on their `on_*` overrides or they won't compile, and a version bump is how lanes are told
|
`HandlerResult` on their `on_*` overrides or they won't compile, and a version bump is
|
||||||
to regenerate and adapt. Not major and not an ADR: one lane consumes this binding, it's
|
how lanes are told to regenerate and adapt. Minor, not major: a major would make
|
||||||
the lane that asked, and there's no contested design here — the shape is the one you
|
`session.hello` refuse a client whose wire behaviour is unchanged. The rule — a
|
||||||
proposed. `kProtocolVersion` moves to `"1.4.0"` with it.
|
generated-binding API break with an unchanged wire is minor + migration note, because
|
||||||
|
`VERSION` is the protocol version, not the C++ ABI — is written up as
|
||||||
|
`docs/adr/0014-generated-binding-changes-and-versioning.md` (this instance is
|
||||||
|
mechanical; the ADR records the rule for the next one, which GUI will also consume
|
||||||
|
since it already links `velox::proto`). `kProtocolVersion` moves to `"1.4.0"` with it.
|
||||||
|
|
||||||
## P2 — clarifications
|
## P2 — clarifications
|
||||||
|
|
||||||
|
|||||||
@@ -0,0 +1,78 @@
|
|||||||
|
# ADR 0014 — Versioning a generated-binding break with an unchanged wire
|
||||||
|
|
||||||
|
**Status:** accepted · **Date:** 2026-09-10 · **Lane:** PROTO
|
||||||
|
**Prompted by:** the 1.4.0 bump (DAEMON's P1 — `HandlerResult` / `HandlerError` on the
|
||||||
|
generated C++ `Dispatcher`), recorded here as a rule rather than left as a one-off.
|
||||||
|
|
||||||
|
## Context
|
||||||
|
|
||||||
|
`contracts/README.md` rule 4 says: optional field or new method → **minor**; rename,
|
||||||
|
remove, retype, or default change → **major** + an ADR. Read literally, the 1.4.0 change
|
||||||
|
retyped every `Dispatcher::on_*` method (`Result<T>` → `HandlerResult<T>`) and so should
|
||||||
|
have been major with an ADR.
|
||||||
|
|
||||||
|
It was shipped as a minor bump with a migration note and no ADR. That call was right, but
|
||||||
|
the reasoning only lived in a commit message and a `proto-answers-*` doc. The next
|
||||||
|
generated-binding change — and there will be one; the C++ and TypeScript emitters are
|
||||||
|
young — would get argued from rule 4's literal text again, and rule 4 would read as though
|
||||||
|
PROTO quietly exempts itself whenever it's inconvenient.
|
||||||
|
|
||||||
|
## Decision
|
||||||
|
|
||||||
|
**`VERSION` tracks the wire protocol, not any binding's API or ABI.** Two clients at the
|
||||||
|
same `VERSION` speak the same JSON-RPC: same methods, same params and result *shapes*, same
|
||||||
|
event payloads, same error codes on the wire. That is the whole promise `session.hello`
|
||||||
|
enforces (major-only), and it is the only thing `VERSION` is allowed to mean.
|
||||||
|
|
||||||
|
From that, the versioning rule for a change that leaves the wire byte-identical but alters
|
||||||
|
the **generated binding's source-level API** (a return type, a struct name, a signature —
|
||||||
|
anything that makes a regenerated `core/generated/` or `extension/src/shared/protocol/`
|
||||||
|
stop compiling against last version's hand-written code):
|
||||||
|
|
||||||
|
- **Minor bump.** Not major. A major bump makes `session.hello` reject a client that is
|
||||||
|
wire-compatible — actively worse than the problem, since the wire didn't change.
|
||||||
|
- **A migration note** in the same PR: what to regenerate, and the mechanical edit each
|
||||||
|
consumer makes (e.g. "swap `Result` → `HandlerResult` on every `on_*` override"). The
|
||||||
|
`proto-answers-*` doc for the requesting lane is the normal home; `contracts/README.md`'s
|
||||||
|
version banner carries the one-line summary.
|
||||||
|
- **An ADR only when the change encodes a design decision** — a new type's shape, a new
|
||||||
|
contract, a semantic split. A purely mechanical shape change (this one: the error-channel
|
||||||
|
type DAEMON proposed, applied as sketched) does not need one; it needs the migration
|
||||||
|
note. "One lane consumes it today" is **not** a reason to skip the ADR when the change is
|
||||||
|
a design decision — GUI already links `velox::proto`, and the next such change will have
|
||||||
|
more than one consumer from day one.
|
||||||
|
|
||||||
|
**Rule 4's "retype → major" is about the wire.** Retyping a field on `TaskError`,
|
||||||
|
`TaskSummary`, a method's params or result — anything a client parses off the socket — is
|
||||||
|
major, because a peer at the old version mis-parses it. Retyping a generated C++ virtual's
|
||||||
|
return type is not that: no byte on the wire moved, and the compiler catches every call
|
||||||
|
site at build time. The two are different failure modes and get different rules.
|
||||||
|
|
||||||
|
## A regeneration hazard worth naming
|
||||||
|
|
||||||
|
`nlohmann::json` brace-initialization from `nullptr` produces the **array `[null]`**, not
|
||||||
|
JSON `null`. `HandlerError`'s `data` member is written `nlohmann::json data = nullptr;`
|
||||||
|
(copy-init) for exactly this reason; a member written `data{nullptr}` compiles, passes a
|
||||||
|
casual read, and makes every defaulted `HandlerError{}` emit `"data":[null]` in the error
|
||||||
|
response. It was caught here only by an end-to-end assertion on `dispatch()`'s output.
|
||||||
|
|
||||||
|
Anyone hand-editing `contracts/codegen/gen_cpp.py` around json defaults or literals: prefer
|
||||||
|
`= nullptr` / `= {}` / `json::object()` / `json::array()` over brace-init with a single
|
||||||
|
element, and keep the `dispatch()`-output assertions that would catch a regression.
|
||||||
|
|
||||||
|
## Consequences
|
||||||
|
|
||||||
|
- The 1.4.0 bump stands as a minor with a migration note; this ADR is its written backing.
|
||||||
|
- A future generated-binding retype cites this ADR instead of relitigating rule 4.
|
||||||
|
- `contracts/README.md` rule 4 gets a pointer here so the "major + ADR" line isn't read in
|
||||||
|
isolation.
|
||||||
|
|
||||||
|
## Alternatives rejected
|
||||||
|
|
||||||
|
**Make it major, per rule 4's literal text.** Rejected: `session.hello` compares majors and
|
||||||
|
would refuse a client whose wire behaviour is unchanged. The rule's purpose is to stop
|
||||||
|
incompatible peers connecting; applying it here would block compatible ones.
|
||||||
|
|
||||||
|
**Leave the reasoning in commit messages and `proto-answers-*` docs, as done for 1.4.0.**
|
||||||
|
Rejected: those are per-instance and get lost. The rule needs one durable home, which is
|
||||||
|
what `docs/adr/` is for (`ADR 0001`).
|
||||||
Reference in New Issue
Block a user