Populate download cancellation output - #119
Merged
pfeiferj merged 1 commit intoAug 9, 2026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mapdExtendedOut.downloadProgress.cancelledfield.DownloadProgress.Canceledstate through the generated serializer.Motivation
The download owner already records cancellation in
settings.DownloadProgress.Canceled, and main transfers progress snapshots intoExtendedState. The publicMapdDownloadProgressschema definescancelled, its generated accessor already exists, and the output documentation and CLI use that field.ExtendedState.setDownloadProgresscopies the active state, totals, locations, and per-location details but never copies cancellation. Even whenExtendedStateholds a canceled terminal snapshot, the serialized field remains at its default false value.Implementation
The serializer now calls
SetCancelledwiths.DownloadProgress.Canceledbeside the existingSetActivecall.The internal American spelling and public British spelling remain unchanged; this is the missing mapping between their existing contracts.
Behavior
68813e05active=true,cancelled=falseactive=false,cancelled=falseactive=false,cancelled=trueValidation
An external Linux oracle constructed the real extended-output struct, invoked
ExtendedState.setDownloadProgress, marshaled and unmarshaled the Cap'n Proto message, and read the result through generated accessors.68813e05failed because a terminal snapshot withCanceled=trueserializedcancelled=false.go test ./...,go test -race ./...,go vet ./..., andgo build ./...passed on Linux/amd64 with Go 1.25.1.make buildpath successfully for exact commit5688b97.Compatibility
cancelledare unaffected. Consumers that read it receive the state already held byExtendedState.extended_state.go, so this serializer correction does not duplicate or conflict with that lifecycle change.Audit follow-up — this line has no observable effect until #107 lands
An independent audit confirmed the added line is correct and sets the right field from the right source, but found that the Behavior table describes the serializer boundary rather than what a fork actually receives on current
main.Canceled=trueappears only in the terminal progress snapshot, which is delivered through a non-blocking send into a capacity-1 channel polled once per 50 ms — and that terminal snapshot is virtually always dropped. Measured on currentmain: 0 of 30 runs publishedcancelled=true; what is actually published isactive=true, cancelled=false. The table holds given such a snapshot, and that was reproduced, but the snapshot does not arrive.PR #107 is what makes this work: its
publishProgress()evicts the stale capacity-1 entry before re-sending, so the terminal canceled snapshot actually reachesExtendedState. The file-level independence claim is accurate — #107 does not touchextended_state.go— but the dependency is behavioral, so land #107 first or alongside this one; by itself, this publishes a field that stays false. Composition with #106 and with #107 was verified by trial merge, build, and test.