proto: answer CORE's freeze-blockers before 1.0.0 lands
Three corrections into 1.0.0, all of which would be major bumps once the contract has landed. It has not: main still carries 1.0.0-draft, so these are corrections to an unpublished version rather than changes to a released one. ADR 0010 records that and the reasoning behind each. B1 — TaskError.code was a bare integer, and the integer space in the contract is JSON-RPC's, which is a different thing; TaskError's own description said so while typing its code as one. Freeze TaskErrorCode: a string enum mirroring vdm::Error by name and in order, all 27 failure values, verified against core/include/vdm/util/error.hpp mechanically. ErrorCode says why a call failed; TaskErrorCode says why a download failed, and a download fails while every RPC succeeds. Adds TaskError.cause so max_retries_exhausted names what kept failing. B2 — TaskSummary.segments is now explicitly the effective count in use right now, after the per-host cap and the non-resumable demotion to 1. DownloadSpec.segments and download.update's patch say they are the requested value. B3 — Segment.endByte's "minimum: 0" contradicted the description's own empty-range encoding of startByte - 1, which is -1 for the first segment of every download. Empty ranges are no longer representable and are not needed. The range stays CLOSED and INCLUSIVE, matching the HTTP Range header the two fields are copied into verbatim, and that is now stated in the schema, the README, an ADR, a fixture assertion and a conformance check. CORE asked for half-open and gets a written notice rather than a silent schema edit. Segment state spells 'downloading' as CORE asked, not 'receiving'. check_contract.py now enforces segment contiguity, coverage of exactly [0, sizeBytes-1], downloadedBytes within the range size, and the entry count matching TaskSummary.segments. The download.get fixture claimed 8 segments while carrying 2; it now carries 8 contiguous ones covering the whole file. contracts/proto-answers-m1.md answers every item in core/docs/proto-requests-m1.md, including the ones not being landed now: B2a and F2 accepted as follow-ups, F1 answered with the notify path for M1, F3 already frozen as a Checksum object rather than a string, and D1 left for DAEMON to draft as the three-way ADR it is. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
This commit is contained in:
@@ -15,9 +15,15 @@ implementations disagree at once.
|
||||
|
||||
## Decision
|
||||
|
||||
`contracts/VERSION` is frozen at **1.0.0**. The surface is 38 methods, 9 events, 25 named
|
||||
`contracts/VERSION` is frozen at **1.0.0**. The surface is 38 methods, 9 events, 26 named
|
||||
types and the JSON-RPC envelope, exactly as `contracts/README.md` documents it.
|
||||
|
||||
Three corrections landed into 1.0.0 before it reached `main`, answering CORE's
|
||||
freeze-blockers: a `TaskErrorCode` wire taxonomy, the effective-vs-requested meaning of
|
||||
`TaskSummary.segments`, and the segment range convention. See ADR 0010 — including why
|
||||
correcting an unpublished 1.0.0 in place is not the major bump this ADR's own rule would
|
||||
otherwise demand.
|
||||
|
||||
**Semantics of a change:**
|
||||
|
||||
| Change | Bump | Also needs |
|
||||
|
||||
@@ -0,0 +1,112 @@
|
||||
# ADR 0010 — The wire failure taxonomy, and the segment range convention
|
||||
|
||||
**Status:** accepted · **Date:** 2026-09-09 · **Lane:** PROTO
|
||||
**Answers:** CORE's B1, B2 and B3 in `core/docs/proto-requests-m1.md`
|
||||
|
||||
## Context
|
||||
|
||||
The 1.0.0 freeze as first drafted had `TaskError.code` as a bare `integer` with no enum,
|
||||
`TaskSummary.segments` with no stated meaning, and a `Segment` range whose description
|
||||
contradicted its own bounds. CORE raised all three as freeze-blockers before building
|
||||
stage 6 on top of them. All three are retypes or meaning-pins, which rule 4 makes **major**
|
||||
bumps once the contract has landed.
|
||||
|
||||
## Decision
|
||||
|
||||
### 1. `TaskErrorCode` — a string enum, separate from `ErrorCode`
|
||||
|
||||
`TaskError.code` is now a `TaskErrorCode`: a string enum whose 27 values mirror
|
||||
`vdm::Error` in `core/include/vdm/util/error.hpp` one-for-one, by name and in order.
|
||||
`ok` has no wire spelling, because a `TaskError` only exists when something failed.
|
||||
|
||||
**These are two different code spaces and conflating them was the bug.** `ErrorCode` is
|
||||
the JSON-RPC space: it says why a *call* failed. `TaskErrorCode` says why a *download*
|
||||
failed. A task fails while every RPC involved succeeds — that is the normal case, not an
|
||||
edge case, and the type system now says so. `TaskError`'s own description said "distinct
|
||||
from the JSON-RPC Error" while typing its code as the integer that JSON-RPC uses.
|
||||
|
||||
Strings rather than grouped integer ranges, which was CORE's offered fallback:
|
||||
|
||||
* the mapping stays lossless without anyone maintaining a numbering scheme in two repos;
|
||||
* a log line or a `nc` session reads `"server_file_changed"` instead of `407`;
|
||||
* CORE cannot include a protocol header (the layering rule), so the two enums are related
|
||||
only by name — which makes the name the thing worth keeping identical, and a number the
|
||||
thing most likely to drift.
|
||||
|
||||
DAEMON owns the projection. Because the names are identical, that projection is a
|
||||
generated-looking switch with no judgement in it, and a new `vdm::Error` value that is not
|
||||
on the wire is a compile-time hole rather than a silent collapse to "internal".
|
||||
|
||||
`retryable` stays a per-occurrence boolean rather than a property of the code, because
|
||||
CORE's own table has `probe_failed` as "maybe". `cause` carries the underlying code for
|
||||
`max_retries_exhausted`, so a user is told what actually kept failing.
|
||||
|
||||
### 2. `TaskSummary.segments` is the **effective** count
|
||||
|
||||
It is the number of connections in use **right now**, after the per-host cap and after the
|
||||
demotion to 1 for a non-resumable source. A task the user asked 16 connections for may
|
||||
honestly report 1. The requested value stays in `DownloadSpec.segments` and is not echoed
|
||||
back. `TaskDetail.segmentDetail` always holds exactly this many entries.
|
||||
|
||||
Pinning this was the genuinely blocking half of CORE's B2: an unstated meaning is not a
|
||||
free field, it is a coin flip that becomes a major bump the moment either side guesses.
|
||||
|
||||
### 3. Segment ranges are **closed and inclusive**: `[startByte, endByte]`
|
||||
|
||||
`endByte` is the index of the last byte, not one past it. CORE asked for half-open
|
||||
`[start, end)`; PROTO chose inclusive and this ADR is the notice.
|
||||
|
||||
The reason is that these two fields are copied verbatim into `Range: bytes=<start>-<end>`,
|
||||
and RFC 9110 byte ranges are inclusive. Inclusive means zero arithmetic between the wire
|
||||
and the socket. Half-open would mean a `-1` at every boundary between the contract and
|
||||
every HTTP request the engine makes, which is precisely where off-by-ones live.
|
||||
|
||||
The contradiction CORE would have hit is also fixed: the old description encoded an empty
|
||||
segment as `endByte == startByte - 1`, which is `-1` for a segment at offset 0 — and every
|
||||
download's first segment starts at 0, so the schema's `minimum: 0` rejected it. **Empty
|
||||
ranges are no longer representable and are not needed.** `endByte >= startByte` always
|
||||
holds; a zero-length download carries an empty `segmentDetail`; a segment that donates its
|
||||
remainder to a steal keeps the bytes it already wrote.
|
||||
|
||||
`tests/conformance/check_contract.py` now asserts contiguity, coverage of exactly
|
||||
`[0, sizeBytes - 1]`, `downloadedBytes <= endByte - startByte + 1`, and that the entry
|
||||
count matches `TaskSummary.segments`. Flipping a fixture to half-open makes it fail.
|
||||
|
||||
Also settled, from B3: the field is spelled **`index`** everywhere including
|
||||
`event.task.progress` (there is no `i`), and the segment state enum is
|
||||
`pending | connecting | downloading | stalled | complete | failed` — `downloading`, as CORE
|
||||
asked and as `TaskState` already spells it, not `receiving`.
|
||||
|
||||
## Why this is not a major bump
|
||||
|
||||
1.0.0 has **not landed on `main`**. `main` still carries `1.0.0-draft`; the freeze lives on
|
||||
`lane/proto` and no lane has consumed it. These are corrections *to* 1.0.0 before it is
|
||||
published, not changes to a released contract. Bumping to 2.0.0 for a version nobody ever
|
||||
received would be ceremony, not safety.
|
||||
|
||||
The rule is unchanged and starts biting the moment this lands: after that, retyping
|
||||
`error.code` or re-pinning `segments` is major, with an ADR and a migration note.
|
||||
|
||||
## Consequences
|
||||
|
||||
* `TaskError` gains `cause`; `TaskSummary`, `DownloadSpec` and `download.update`'s patch
|
||||
now state which side of the requested/effective line they sit on.
|
||||
* The contract has 26 named types rather than 25.
|
||||
* CORE builds stage 6 against inclusive ranges. **This is the item most likely to be got
|
||||
wrong silently**, which is why it is in an ADR, in the schema description, in
|
||||
`contracts/README.md`, in a fixture assertion, and in a conformance check.
|
||||
|
||||
## Alternatives rejected
|
||||
|
||||
**Grouped integer ranges** (CORE's fallback). Works, but every value needs a number nobody
|
||||
can read, maintained in two places that cannot include each other's headers. The names are
|
||||
already identical; numbering them adds a translation step whose only purpose is to go
|
||||
wrong.
|
||||
|
||||
**Reusing `ErrorCode` for both.** This is what the draft accidentally did. It makes
|
||||
"the call failed" and "the download failed" indistinguishable at the type level, and there
|
||||
is no sensible JSON-RPC code for `checksum_mismatch`.
|
||||
|
||||
**Half-open ranges, as CORE asked.** Rejected for the HTTP reason above, but it was close,
|
||||
and it is the convention CORE would have implemented by default — hence the loud notice
|
||||
rather than a quiet schema edit.
|
||||
Reference in New Issue
Block a user