Repository navigation
Document what building droidsight needs, and attest the release archives - #22
Merged
Merged
Conversation
openh264-sys2 builds Cisco's H.264 decoder from C++ and assembles its hot paths with NASM, which no platform preinstalls. CI installs it as the first step of every job that compiles, and the release workflow does the same -- and those two comments were the only mention of it anywhere in the repository. So both compiling routes failed on a clean machine. `cargo build --locked --release --bin droidsight`, which the README offered as the alternative to npx, and `cargo install droidsight`, which is what someone arriving from crates.io will type. Neither failure names this crate: the error comes from a transitive dependency the reader has no reason to have heard of, which is the hardest possible shape for a first-contact error to take. The README also documented exactly one way to install this server. The crate has been on crates.io and the server in the MCP Registry since 1.0.0, and it mentioned neither. Both are now listed alongside npx, with badges, and the npx route is marked as the one that needs no build tools at all.
Every diagnostic in cli.js was a console.error followed immediately by process.exit. Writes to stderr are only synchronous when stderr is a TTY or a regular file; to a pipe on POSIX they are queued, and process.exit does not drain the queue. An MCP client always gives this process a pipe for stderr. So the messages explaining a missing platform package, an --omit=optional install, or a binary that cannot be executed were at risk of being dropped in precisely the situation they were written for -- while looking correct in every terminal anyone would test them in. They now go through one helper that writes with fs.writeSync, which bypasses the queue. A non-blocking pipe can refuse the write with EAGAIN, which means retry rather than failed, so that case loops; EPIPE means the reader is already gone and there is nobody left to tell, so that one stops. Anything else falls back to console.error, on the grounds that reporting through the lossy path beats not reporting at all. The unsupported-platform message was also pointing at `cargo install --git` a moment before the crate existed on crates.io, and said nothing about NASM. It now names the published crate and the prerequisite, so it does not send someone to the failure the README was just taught to prevent. Verified with fourteen checks against the real file through fully piped stdio: message content on all three failure paths, argument forwarding, and exit-code passthrough. One caveat worth recording -- stderr-to-a-pipe is already synchronous on Windows, so a local run proves this did not regress rather than proving the POSIX case it exists for.
The workflow said it plainly: the npm packages carry provenance attestations, but the standalone archives are unsigned and unattested, and a published hash is the only integrity check someone downloading a tarball directly can perform. That hash is computed and published by this same workflow, so it can say a file has not changed since it was hashed and nothing at all about what produced it. attest-build-provenance binds each archive to this workflow, this commit, and this repository, in a store the release page cannot rewrite, checkable with `gh attestation verify <archive> --repo edgecasehuman/droidsight`. It runs before anything is published, like the npm preflight checks above it, so a failure ends the run while the tag is still re-runnable. The release notes were `--generate-notes`, which produces a list of commit subjects. That describes the work rather than what it means for someone deciding whether to upgrade, and it is the one thing a release page has to carry. Notes now come from a CHANGELOG section matching the version being built, with the checksums appended. A tag whose version has no section fails the run rather than publishing an empty page. The changelog itself is reconstructed from the tags and the commits, not from memory: 1.0.0 as released, and the three post-tag registry and provenance fixes under Unreleased. Verified by executing the workflow's own run: script -- read out of the YAML rather than retyped -- against the real changelog, for a version that exists, the oldest version, which has no successor heading to stop at, and a version that does not exist, which must fail.
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
1 participant
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.
What this changes
Three things a user meets before they ever reach a device: the build fails on a
clean machine for an undocumented reason, the launcher's error messages can be
lost in the one environment it runs in, and the release archives are unattested.
openh264-sys2assembles Cisco's decoder with it and no platform preinstallsit. CI installs it as the first step of every compiling job, and those
workflow comments were its only mention anywhere. So
cargo buildandcargo install droidsightboth failed on a clean machine, inside atransitive dependency, where the error names nothing the reader recognizes.
The README also documented exactly one install route; the crate has been on
crates.io and the server in the MCP Registry since 1.0.0 and it mentioned
neither.
fs.writeSync. Every messagewas a
console.errorfollowed immediately byprocess.exit, and writes tostderr are only synchronous for a TTY or a regular file — to a pipe on POSIX
they are queued, and
process.exitdoes not drain the queue. An MCP clientalways gives this process a pipe, so those messages were at risk of being
dropped in precisely the situation they exist for. EAGAIN retries, EPIPE
stops, anything else falls back to the lossy path.
from a new CHANGELOG.md instead of
--generate-notes. The published hash iscomputed and published by the same workflow, so it can say a file has not
changed since it was hashed and nothing about what produced it.
Authority and security
The attestation step adds
attestations: writeto the publish job, alongsidethe
id-token: writethat npm trusted publishing already required. No newsecret: both are short-lived OIDC tokens minted per run.
No change to tool schemas, environment variables, path confinement, subprocess
construction, device selection, output limits, or protocol lifecycle.
Checks
cargo fmt --all -- --checkcargo clippy --locked --all-targets -- -D warningscargo test --locked -- --test-threads=1— 114 passingCargo.lockis included, if dependencies changed — no dependency changeBeyond the Rust gates: both
npm/check-*-consistency.mjspass, the launcherwas exercised by fourteen checks through fully piped stdio (message content on
all three failure paths, argument forwarding, exit-code passthrough), and the
release workflow's own note-extraction script was executed — read out of the
YAML rather than retyped — against the real changelog, including the case where
a version has no section and the release must fail.
Two things stay unproven here and are worth saying out loud. The attestation
step only runs on a tag, so it has never executed. And stderr-to-a-pipe is
already synchronous on Windows, so a local run of the launcher proves the fix
did not regress rather than proving the POSIX case it exists for.
Device testing
Not exercised against a device. Nothing here touches the device path.