agent-governance: stop the endorctl download from blocking session start - #21
Open
georgeap70 wants to merge 3 commits into
Open
agent-governance: stop the endorctl download from blocking session start#21georgeap70 wants to merge 3 commits into
georgeap70 wants to merge 3 commits into
Conversation
endorctl is a ~300 MB binary served uncompressed, and download_endorctl.sh fetched it inline in the session hook with no timeout of any kind. On a slow link that stalled agent startup for minutes; on a stalled connection it could hang indefinitely, since curl has no default transfer timeout and --retry 5 multiplied it. The binary is rebuilt roughly daily, so this was not a first-run-only cost. Split the bootstrap into a foreground decision and a detached background worker. The foreground now does two file tests and returns: steady state costs no network I/O and no binary spawn, down from ~0.8s plus an uncapped round trip. An available update is fetched in the background while the session audits with the binary already on disk. A machine with no endorctl yet skips that one audit (exit 0, so the appended audit call does not run against a missing binary) rather than blocking on the install. Also: - Bound every request. --connect-timeout/--max-time on metadata; --connect-timeout plus --speed-limit/--speed-time on the body. No --max-time on the body: off the critical path, a slow link should finish. - Resume across sessions instead of restarting from byte 0. curl -C - cannot be used here - it sends an open-ended "Range: bytes=A-", which this endpoint answers with a 200 and the whole body, so curl aborts with "server doesn't seem to support byte ranges". A closed bytes=A-B gets a proper 206, so probe the length with HEAD and request an explicit range. - Serialize with an mkdir lock, so concurrent agents perform one download rather than N x 300 MB competing for the same scarce bandwidth. - Pin the partial to the digest it is being built for, and discard a full-length partial that fails verification. Without that, a corrupt full-length partial would re-request a range past the end every session and never recover. - Throttle the version check to once every 24h (ENDORCTL_UPDATE_TTL_MINUTES), stamped on success only so failures retry. Windows (download_endorctl.ps1) is unchanged and still fetches inline; it needs a different detach primitive and is tracked as a follow-up in the design note. Adds tests/run-tests.sh, the repo's first test suite: 71 offline assertions driving the bootstrap under a throwaway HOME with a stubbed curl, an examples/ sync check that fails when generated output drifts from the scripts, and opt-in --network checks that pin the endpoint's range contract. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The bootstrap is embedded in every session hook and base64'd into the Windows form, so its commentary landed in every generated profile - the Claude SessionStart command had grown to 7.9 KB, mostly comments. Cut download_endorctl.sh's commentary to the few non-obvious points (148 -> 119 lines) and have render.sh drop whole-line comments and blank lines when inlining either bootstrap. SessionStart is now 3,796 bytes with no comment lines. The longer rationale moves into the design note, which also picks up the signal/trap and exit-0 reasoning that was previously only in the source. Windows artifacts change too: download_endorctl.ps1 is untouched, but its inlined copy is now comment-stripped like the POSIX one. Verified the stripped PowerShell is intact - no block comments, no backtick continuations, and the one '#' inside a string literal sits on a line that is itself a comment. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Keep the change to shipped scripts, docs and tests. The essential "why" that a maintainer needs is already in download_endorctl.sh as short comments - not using curl -C -, mtime-based lock staleness, exit 0 rather than exit 1, and signals routing through exit - so nothing load-bearing is lost. Also clears the three references that would otherwise dangle: the pointer in download_endorctl.sh's header, the one in render.sh's strip_src comment, and "design notes" in the README's repository layout. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Author
|
@codex review |
Author
|
it is almost impossible to follow what the script does now with all this extra logic. I added some tests, but this will require some real deploy and test to verify it works |
georgeap70
marked this pull request as ready for review
July 31, 2026 15:29
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.
Problem
endorctlis a ~300 MB binary served uncompressed (305,440,226 bytes), anddownload_endorctl.shfetched it inline in the session hook with no timeout of any kind — both curls used-fsSL --retry 5 --retry-connrefused --retry-all-errorswith no--connect-timeout,--max-time, or--speed-limit. curl has no default transfer timeout, so a slow-but-alive link or a captive portal could hang the hook indefinitely, and--retry 5multiplied it by up to 6.That's ~4 min at 10 Mbps and ~20 min at 2 Mbps, and since the binary is rebuilt roughly daily it wasn't a first-run-only cost — developers paid it on the first session of most days.
Four more defects behind the headline one:
exit 1, and the session hook is composed asbootstrap \n audit, so a network hiccup produced both no audit event and a hook error shown to the developer. The version check also ran every session with no "checked recently" stamp.Approach
Split the bootstrap into a foreground decision and a detached background worker.
Foreground now does two file tests and returns — steady state costs no network I/O and no binary spawn, down from ~0.8 s plus an uncapped round trip. An available update is fetched in the background while the session audits with the binary already on disk. A machine with no
endorctlyet skips that one audit (exit 0, so the appended audit call doesn't run against a missing binary) rather than blocking on the install.Background (
( trap '' HUP; … ) >/dev/null 2>&1 </dev/null &) does the check, download, verify, and atomic swap. The redirections are what release the hook's stdout pipe — without them the agent keeps waiting even after the parent exits.Also:
--connect-timeout 5 --max-time 30on metadata;--connect-timeout 10 --speed-limit 10240 --speed-time 60on the body. Deliberately no--max-timeon the body — off the critical path, a genuinely slow link should be allowed to finish.curl -C -cannot be used against this endpoint: it sends an open-endedRange: bytes=A-, which the server answers with a200and the entire body, so curl aborts with(33) HTTP server doesn't seem to support byte ranges. A closedbytes=A-Bgets a proper206, so the script probes total length withHEADand requests an explicit closed range, appending, with the offset recomputed per attempt.mkdirlock, so concurrent agents do one download instead of N × 300 MB. Staleness keyed on the partial's mtime (which curl advances continuously), not a fixed timeout that would kill a live download on a slow link.ENDORCTL_UPDATE_TTL_MINUTES), stamped on success only so failures retry.Scope
download_endorctl.ps1itself is untouched — but the Windows artifacts do change, because its inlined copy is now comment-stripped like the POSIX one.download_endorctl.ps1needs a different detach primitive —Start-Process powershell -EncodedCommand … -WindowStyle Hidden, re-encoding the updater from a here-string, sinceStart-Jobdies with its parent — plus a Range-header resume loop replacingInvoke-WebRequest -OutFile. Worth noting thatInvoke-WebRequest -TimeoutSecis not a whole-transfer timeout, so today's120does not actually bound a ~300 MB download. It's less acutely broken than POSIX was, since it at least has timeouts, so it's split out rather than blocking this change.Tests
Adds
agent-governance/tests/run-tests.sh— the repo's first test suite.The offline suite drives the bootstrap under a throwaway
HOMEwith a stubbedcurl, so branches that only happen on a bad network are reachable without waiting on a transfer: dead endpoint, half-finished download, corrupt download, two agents racing, signal mid-transfer. It also regenerates everyexamples/artifact and fails if the checked-in copy differs, and syntax-checks all 36 hook commands embedded across those artifacts to confirm they survive JSON/TOML escaping.--networkpins the behavior resume depends on. If the open-ended range ever starts returning206,curl -C -would work and the closed-range logic could be deleted.Verified beyond the suite:
curl -C -(7), no corruption recovery (6), no lock (5),exit 1instead ofexit 0(5), skipping digest verification (7).Review notes
render.shstrips whole-line comments and blank lines when inlining (strip_src), so they don't ship in every profile. The ClaudeSessionStartcommand is 3,796 bytes; it was 1.9 KB before this change and would have been 7.9 KB without the stripping.download_endorctl.sh: (1) the closed-range resume instead ofcurl -C -; (2) lock staleness judged by the partial's mtime rather than a fixed timeout, so a live slow download is never broken; (3)exit 0rather thanexit 1when there's no binary, so the hook stays successful and the appended audit call doesn't run; (4)INT/TERMrouting throughexitso the singleEXITtrap does cleanup — a signal handler that returns would resume the script and drop the lock mid-download..github/, so that felt like a separate call. The offline suite is designed to drop straight into a workflow.🤖 Generated with Claude Code