Polish --sandbox auto-kit output and tool auto-install logging#2878
Open
dgageot wants to merge 7 commits into
Open
Polish --sandbox auto-kit output and tool auto-install logging#2878dgageot wants to merge 7 commits into
dgageot wants to merge 7 commits into
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
aheritier
added
area/tools
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
aheritier
added
the
area/security
Add subtle ANSI styling to the auto-kit summary printed by --sandbox. fatih/color auto-disables when stdout isn't a TTY, so tests and piped output stay plain. - kit path: cyan - section headers (skills:, prompt files:, summary:): bold - skill / prompt-file names: bold - (from ...) and 'workspace mount' notes: faint - (redacted) markers and the secret count: yellow - counts in the summary line: bold
The kit dir lives under $HOME/.cache, so the printed header used to show the full host home path. Route it through pathx.ShortenHome like every other host path the summary prints.
stageSkills now consults every agent's SkillsConfig before copying any local skill into the kit: - no agent enables local skills -> ship nothing - at least one agent enables local skills without an include filter -> ship every local skill (wide case wins) - every local-enabled agent restricts skills via include -> ship only the union of those filters This matches what the in-sandbox runtime actually loads, so the kit no longer leaks unrelated host skills into agents that don't use them.
When the agent auto-downloads a missing MCP/LSP server, the only user-visible output used to be the raw 'go install' chatter, which gives no clue what's being fetched or why. Print a one-line stderr message naming the command and the resolved package@version right before the install kicks off. The line is styled subtly (bold command, faint package ref) and fatih/color auto-disables when stderr isn't a TTY, so logs and CI output stay plain.
fatih/color's global NoColor is computed from os.Stdout, so the
previous announceInstall comment ('auto-disables when stderr isn't a
TTY') was wrong in two ways: piping stderr to a log while keeping
stdout on the TTY would write escape codes into the log, and the more
common case — piping stdout while keeping stderr on the TTY — would
take the styled branch but emit plain text because SprintFunc consults
the (stdout-derived) NoColor flag.
Decide explicitly on stderr (honouring NO_COLOR and TERM=dumb), and
EnableColor() the local *color.Color values so styling actually
survives when the global flag has stripped it. Fall back to a plain
line when colour isn't safe.
Skills used to be discovered from $HOME alone, so a config-load failure was harmless for them. Now stageSkills consults the agent config and returns nothing when cfg is empty, so an unavailable config drops both skills and prompt files. Update the comment and the slog message to reflect the current behaviour.
The first log line in doInstall fired before the registry lookup, so debug logs showed 'Auto-installing missing command via aqua registry command=foo' with no clue which package would be downloaded. Drop that early line and emit a single, fully-attributed entry after resolution so the log names both the command and the resolved owner/repo@version before installing.
dgageot
force-pushed
the
sandbox-kit-polish
branch
from
c6f7333 to
64cabfb
Compare
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
3 participants
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.
The
--sandboxauto-kit output now reads better and carries more context when tools auto-install at runtime. These changes improve both the user-visible summary and the underlying tool-staging logic.Auto-kit summary readability. Subtle ANSI styling is now applied to the kit summary printed by
--sandbox(kit path in cyan, section headers and names in bold, faint notes for(from …)/workspace mount, yellow for(redacted)markers and the secret count). The styling gracefully degrades when stdout isn't a TTY — tests and piped output stay plain thanks to fatih/color's auto-detection.Path display. Every host path in the kit summary now goes through
pathx.ShortenHome, including the kit directory itself, so$HOMEis consistently collapsed to~for readability.Skill staging respects the agent config. The
stageSkillslogic now consults each agent'sSkillsConfiginstead of unconditionally shipping every local skill found. It ships nothing if no agent enables local skills, every local skill if any agent enables them without anincludefilter (the wide case wins), and otherwise only the union of agents'includefilters. Five new tests cover these scenarios.Auto-install logging mentions the package. When an MCP/LSP server auto-installs at runtime, both the debug log and a new user-visible stderr line now name the package as
command (owner/repo@version)before the download starts, providing context for thego installchatter that follows. The stderr banner respectsisattyon stderr directly and honoursNO_COLOR/TERM=dumb.All changes maintain backward compatibility. Linting and all 132 package tests pass.