Keep setup usable when an optional tool installer fails - #3
Open
bmorelli25 wants to merge 1 commit into
Open
Conversation
A failing Vale or docs-builder installer aborted install before skills and host adapters were configured, so a sudo prompt that could not be answered left the command with nothing done. Both tools are optional, so report every failure, finish the rest of the setup, and exit non-zero. Status output could not distinguish a tool the user never asked for from one that failed to install. Item hints already existed but only reached the session-start hook, so print them beneath any row that is not current, and point a component that was never requested at its install option. Also make version reporting honest: --yes was parsed and discarded, version lookups silently reported unknown when the shared 60-per-hour GitHub limit was reached, and a tool that logged before printing its version was read as not installed. Co-authored-by: Cursor <cursoragent@cursor.com>
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.
Why
Running
elastic-docs-utils install --with-docs-toolson a machine where docs-builder installs to/usr/local/binhit a sudo prompt it could not answer. The command aborted immediately, so skills and host adapters were never configured, even though nothing was wrong with them.Investigating that also turned up three smaller reporting problems that made the failure hard to understand.
Changes
Optional tool failures no longer abandon the command.
installOptionalToolsnow returns every failure rather than the first, andinstallandupdatewarn, finish the rest of the work, then exit non-zero. Vale and docs-builder are explicitly opt-in, so their installers should not be able to prevent the setup those commands exist to perform.not installednow says what to do about it.updates.Itemalready carried aHint, but the only place it rendered was the session-start hook, gated onupdate available— never the state where a user needs it most. Hints now print beneath any non-current row instatusandcheck-updates, and a component that was never requested points at itsinstall --with-...option instead of an update command. This was the actual source of confusion:missingreads as a broken install rather than an opt-in tool.--yesdoes something. It was parsed and immediately discarded (_ = yes) while being advertised in--helpand the README. It now closes the installers' stdin so they cannot block on a prompt. It deliberately does not answer prompts, which stays--force's job, so existing configuration is left alone.Rate limiting is reported instead of hidden. Every lookup returned
""on any error, so an exhausted GitHub quota was indistinguishable from an undetectable version and surfaced as a bareunknown. Lookups now return errors, say when the limit was hit, and sendGITHUB_TOKENorGH_TOKENwhen set. The unauthenticated limit is 60 requests per hour per address, and each check spends five.Version probes are harder to fool.
binaryVersionused.Output(), so a tool that reports its version on stderr or logs before printing it read as not installed. It now parses combined output, prefers a line holding only a version over the first version-shaped string anywhere, and times out after 10s so a binary that starts a server cannot hang the command.Test plan
go test ./...,go vet ./..., andgofmt -l .are cleanHOMEwith a forced docs-builder failure: setup continues, 18 skills install, adapters sync, hints render, and the command exits 1 with the causeNotes for review
updatepassesassumeYes: falsebecause it has no--yesflag; its interactive behavior is unchanged.Item.Statevalues are untouched, so the cached JSON contract and the hook'supdate availablecheck still work.Made with Cursor