feat(app,cli,tui): live install progress, detailed failure reporting, and safe cancellation#35
Merged
Merged
Conversation
…ing, and safe cancellation Replace the spinner-only install screen with an event-driven pipeline shared by all three install entry points (install, add, onboard): - internal/app emits typed lifecycle events (InstallPhase, InstallStatus, InstallProgressEvent) through an injected nil-safe callback; results carry provenance plus a classified InstallFailure (category, message, hint, expected/actual) derived from errs sentinels, never string parsing; Aggregate produces counters that always sum to the total. - internal/tui gains a shared progress component (terminal-count-driven bar, current skill/phase, wide and sub-60-column layouts) and a result screen (truthful summary, scrollable failure table, drill-down detail view); both wizards stream events over the existing channel adapter. - internal/cli rewrites the install renderer (headline, counter line, per-failure blocks with em-dash placeholders), extends the stable JSON document (status, summary, per-skill provenance/failure with omission rules), adds verbose-gated non-TTY progress lines, and suppresses the duplicate post-wizard summary while keeping exit codes intact. - Cancellation is safe end-to-end: SIGINT/SIGTERM cancel the run context, esc/ctrl+c cancels inside both wizards with visible feedback, the loop stops between skills, in-flight aborts classify as cancelled, remaining entries report not-attempted, completed work persists to a valid lockfile, and every path exits 130. All untrusted text is sanitized on every surface; behavior is pinned by unit, TUI, golden (plain + JSON), cancellation, and security tests, plus an import-boundary test keeping rendering out of the domain layer.
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
Replaces the spinner-only install screen ("⏳ Fetching, verifying, and linking…" followed by the contradictory "Installed 0 skill(s), 3 failed") with a structured, event-driven progress and result pipeline shared by all three install entry points (
gskill install,gskill add,gskill onboard).What changed
Domain layer (
internal/app)InstallPhase(resolving → complete),InstallStatus(incl.skipped,cancelled,not-attempted),InstallProgressEventemitted strictly sequentially through an injected nil-safe callback (zero cost unobserved).InstallFailure— category derived from the typederrssentinels viaerrors.Is(never string parsing), remediation hint from the error chain, expected/actual hashes carried as data on integrity mismatches.Aggregateproduces summary counters that sum to the total by construction; cancellation dominates outcome classification.TUI (
internal/tui)CLI (
internal/cli)—placeholders for unknowns; presentation vocabulary shared with the TUI so the surfaces cannot drift.--json: additively extended stable document (top-levelstatus, completesummarycounters, per-skill provenance andfailureobjects with omission rules); nothing emitted until the single final document.-v; zero cursor-control sequences.Cancellation, end to end
SIGINT/SIGTERMnow cancel the run context (cmd/gskill/main.go), so direct CLI runs stop gracefully: the loop halts between skills, in-flight ctx-aware aborts classify ascancelled, remaining entries reportnot-attempted, completed work persists to a valid lockfile, and every path exits 130.Safety
All untrusted text (skill names, sources, error messages) is sanitized on every render surface; JSON escapes control bytes. An import-boundary test keeps rendering dependencies out of the domain layer.
Testing
./scripts/verify.shexits 0: race tests, coverage 76.5% (up from 75.1% baseline), golangci-lint clean, govulncheck, gitleaks.Spec:
specs/014-install-progress-report/(internal).