The last contract gap blocking an M1 definition-of-done item: CORE's "401
handled" has no return path without it, and B2a's sibling F2 was accepted in
proto-answers-m1.md but never actually landed.
download.provideAuth {taskId, username, password, save?} -> {ok}, exactly as
proposed there. Privileged and Unix-socket-only: a credential-bearing method
must never be reachable from the browser, which is the other half of the
promise event.auth.required's own description already makes ("never back
through this event, never into a log"). It answers the challenge; it does not
itself resume the task -- the daemon retries with the credential attached and
the ordinary event.task.state reports the task leaving retry_wait, the same
as any other state change.
save only tells the daemon whether to persist the credential in the Secret
Service for next time, or use it for this attempt alone -- it never touches
SQLite or a log either way, in keeping with CLAUDE.md's secrets rule.
Three fixtures: the success path, -32010 for a task that no longer exists
(credentials submitted for it are simply discarded), and -32003 confirming
the extension has no path to this method under any transport.
mockd gets a real handler rather than falling through to the generic fixture
responder: it validates the taskId exists (so the -32010 fixture is
replayable) and actually transitions the task out of retry_wait.
Minor bump, 1.1.0 -> 1.2.0: additive method, no existing type touched.
Co-Authored-By: Claude Sonnet 5 <[email protected]>
Claude-Session: https://claude.ai/code/session_012fgjnqFCS5h5L7gZTZo3rV
8.1 KiB
PROTO → CORE — answers to core/docs/proto-requests-m1.md
Status: answered. Against contracts/ at 1.0.0 (lane/proto, not yet on main).
Raised by CORE at 1.0.0-draft; every freeze-blocker is resolved below.
Read docs/adr/0010-task-error-taxonomy-and-segment-ranges.md for the reasoning on
B1/B2/B3. This file is the index and the parts CORE has to act on.
Freeze-blockers — all three are in 1.0.0
B1 — a frozen wire enum for the task failure code · done, as a string enum
TaskError.code was a bare integer. It is now
TaskErrorCode: a string enum with your 27
failure values, mirrored by name and in your order, verified against
core/include/vdm/util/error.hpp mechanically rather than by eye. ok has no wire
spelling — a TaskError only exists when something failed.
You were right that this was blocker #1, and right about the diagnosis: the draft typed
code as the JSON-RPC integer while its own description said "distinct from the JSON-RPC
Error". Those are two code spaces. ErrorCode says why a call failed; TaskErrorCode
says why a download failed, and a download fails while every RPC succeeds.
Strings, not your grouped-integer fallback: the mapping is lossless with no numbering
scheme maintained in two repos that cannot include each other's headers, and a log line
reads "server_file_changed" instead of 407.
Two details you should design against:
retryablestays a per-occurrence boolean, not a property of the code — because your own table hasprobe_failedas "maybe". Emit it per failure.causeis new onTaskErrorand carries aTaskErrorCode. It exists formax_retries_exhausted, which your header says has acause: put the last underlyingErrorthere so the user is told what actually kept failing.
httpStatus is expected for the codes in TaskErrorCode's x-carriesHttpStatus
annotation, which matches the "carries httpStatus" column of your table.
B2 — the meaning of TaskSummary.segments · done, frozen as effective
"The EFFECTIVE connection count in use right now — not the number that was requested. What remains after the per-host connection cap and after the demotion to 1 for a non-resumable source."
The requested value stays in DownloadSpec.segments, which now says so on its own
description, as does download.update's patch.segments. TaskDetail.segmentDetail
carries exactly TaskSummary.segments entries, and conformance checks that.
B3 — Segment field names, and the range convention · done, but read this
(a) index vs i — settled as index, everywhere. There is no i spelling in the
contract; event.task.progress's per-segment entries use index too. Nothing to
reconcile, it was already consistent.
(c) the state enum — pending | connecting | downloading | stalled | complete | failed.
Spelled downloading as you asked, matching TaskState; the draft's receiving is
gone. pending is added for a range planned but not yet dialled — if the engine never
reports that, ignore it.
(b) the range convention — this is the one that will bite you if you skim.
Ranges are CLOSED and INCLUSIVE:
[startByte, endByte].
endByteis the index of the last byte, not one past it. The segment coversendByte - startByte + 1bytes.
You asked for half-open [start, end). PROTO chose inclusive and did not adopt your
convention — this notice is the point of this document, and it is deliberately before you
build stage 6.
The reason: these two fields are copied verbatim into Range: bytes=<start>-<end>, and
RFC 9110 byte ranges are inclusive. Inclusive means no arithmetic at all between the wire
and the socket. Half-open means a -1 at every boundary between the contract and every
HTTP request the engine makes — which is exactly where off-by-ones live.
Field names stayed startByte / endByte / downloadedBytes rather than your
start / end / completed, partly so that code written against the half-open spelling
does not silently compile against inclusive fields.
While fixing this we found a real contradiction in the draft: it encoded an empty segment
as endByte == startByte - 1, which is -1 at offset 0 — and every download's first
segment starts at 0, so the schema's own 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. If the engine has a state that genuinely needs
an empty range, say so now — that is a schema change, not something to encode around.
tests/conformance/check_contract.py enforces contiguity, coverage of exactly
[0, sizeBytes - 1], downloadedBytes <= endByte - startByte + 1, and the entry count.
A fixture flipped to half-open fails it.
Not gating the freeze — the follow-up queue
Agreed with your ranking: these are minor under rule 4 and land as small PRs to
contracts/ alone. They are not in 1.0.0. Ranked by when M1 needs them.
| # | Item | Verdict | Shape |
|---|---|---|---|
| B2a | readable effective buffer size | landed in 1.1.0 | TaskDetail.effectiveBufferBytes (placed on TaskDetail, not TaskSummary — bufferBytes itself was already TaskDetail-only, so the pair stays together). See docs/adr/0012-buffer-and-segment-budget.md, which also lands B4's bounds and the two new settings keys in the same PR. |
| F2 | credential return path for 401/407 | landed in 1.2.0 | download.provideAuth {taskId, username, password, save?} → {ok}, exactly as proposed: Unix socket only, privileged. It answers the challenge; it does not itself resume the task — the daemon retries with the credential attached and the usual event.task.state reports the task leaving retry_wait. |
| F1 | "needs user decision" carrier | the simple option | state: paused + event.notify is the intended carrier for M1: CORE reports server_file_changed, DAEMON pauses and notifies, GUI offers restart. A dedicated event.task.decision + download.decide is a real design with a state machine attached, and it should not be invented in a hurry — raise it again in M3 if the notify path proves too thin. A string comparison on error.code covers the engine side either way, which is now a TaskErrorCode comparison rather than a magic number. |
| F3 | checksum string format |
already frozen, differently | download.add {checksum} is not a string. It is a Checksum object: {algorithm: "md5"|"sha1"|"sha256"|"sha512", value: "<hex>"}, with value patterned ^[0-9a-fA-F]{32,128}$. Parse your "<algo>:<hex>" form at the CLI or GUI edge, not on the wire. Note sha512 is accepted by the contract even though the appendix lists MD5/SHA-256 — reject it in the engine if you do not implement it, rather than the contract forbidding it. |
Raise B2a and F2 as requests whenever you need them and PROTO will land them together; neither blocks anything you are building this week.
D1 — state-machine ownership
Your proposed split is right and PROTO does not dispute any of it: CORE owns
probing → connecting → downloading ⇄ paused → retry_wait → assembling → verifying → complete | failed plus cancelled from anywhere; DAEMON owns new, queued and
pause-for-schedule; paused is shared and both sides must be idempotent about it.
PROTO will not write that ADR alone. It is a three-way decision and the lane that owns
neither half writing it down is how a decision gets recorded that DAEMON never agreed to.
DAEMON should draft it, CORE and PROTO review. The contract's part is already frozen:
TaskState has the twelve values, and the wire does not encode who drove a transition.
One thing that is PROTO's and worth stating: event.task.state carries previousState,
so a client can render a transition without keeping its own state machine. Neither CORE nor
DAEMON should assume a client tracks lifecycle — clients render what they are told.