fix(setup,sync): 1.0 polish round 2 — layout, completion handoff, cancel audit#44
Merged
fix(setup,sync): 1.0 polish round 2 — layout, completion handoff, cancel audit#44
Conversation
…cel audit
Bundled fixes for issues uncovered when the user actually walked through
`ckipper setup` end-to-end. Three parallel agents investigated cancel-
propagation across the codebase, layout redesign, and wizard
completeness; this commit synthesizes their findings.
Detected configuration layout (Issue 1)
Replaced the alternating wide-row + indented-description rendering with
a card-style stack: each setting renders as `<key padded> <value>
<source>` on one line, with the schema description in dim color on the
next, separated by blank lines. Removes the column-overflow problem
(long values used to push the source marker out of alignment) and
gives the eye a clear stopping point per setting. Empty values now
render as `(empty)` instead of blank space.
Wizard completion handoff (Issue 2)
After a 5-minute docker build the old "Setup complete" header was
buried in scrollback and the wizard exited silently. Now:
- `_ckipper_setup_offer_image_build` records ok/failed/skipped
and `_ckipper_setup_print_completion_summary` renders a coloured
banner so a failed build is impossible to miss.
- The summary lists `ckipper worktree rebuild-image` and `ckipper
account sync` alongside the existing get-started commands so users
can find them later without re-running the full wizard.
- A "Press ENTER to finish setup" prompt anchors the screen so users
know setup is over (skipped on non-TTY for CI).
Per-account aliases auto-source (Issue 1b)
`install.sh` appends the per-account aliases source line to ~/.zshrc,
but a setup-only re-run never offered to. Added
`_ckipper_setup_offer_aliases_source` with an idempotency check so
re-runs don't duplicate the line. Without this, `claude-<account>`
launchers silently didn't work for users who only ran `ckipper setup`.
Sync-after-2nd-add (Issue 3)
Verified by trace: `_ckipper_setup_offer_initial_sync` correctly
counts accounts AFTER the add and fires when count >= 2. No code
change needed; the existing test covers this path.
Rebuild-image discoverability (Issue 4a)
`ckipper worktree rebuild-image` is the dedicated CLI; now mentioned
in the completion summary's Maintenance block. Help text updated to
list every wizard step including this one.
Cancel propagation audit (Issue 5)
Agent 1 found two unguarded sites in the sync interactive fallback
(CKIPPER_NO_GUM=1 path):
- `_ckipper_account_sync_pick_targets_fallback`
- `_ckipper_account_sync_pick_types` (read fallback branch)
Both now `|| return $?` after `_core_prompt_input` to propagate cancel.
Other sites flagged by the audit (gum --no-limit pickers) are already
protected by the dispatcher's empty-array length check + SPACE/ENTER
hint added in PR #41.
Tests
- 520/520 shell tests pass (+10 over previous baseline).
- New regressions: image build status (ok/failed/skipped), summary
mentions rebuild-image + sync + failed-build banner, aliases-source
skip/append/decline, sync interactive fallback cancel propagation.
- Lint clean.
Round 3 polish in response to user feedback that the previous card layout "still looks bad" and that the completion screen "doesn't match the format as the rest of the setup with the color and stuff." Detected configuration Renders through `gum table -p` with a rounded border tinted to gum's prompt-accent pink (212), so the block visually belongs to the same wizard as the Yes/No prompt below it. Auto-sizes columns to longest value; over-long values (e.g. 50+ char filesystem paths) are truncated with `…` to keep the table inside narrow terminals. Descriptions intentionally drop out of the summary — they appear as labels on the pick-keys-to-customize picker (added in PR #43), where they matter most. A dim-text tip below the table points users there. Falls back to a plain key/value/source list under CKIPPER_NO_GUM (tests, CI, hosts without gum). Same data, no border. Loop locals hoisted out of the body to dodge zsh's `local var` echo-on-redeclare quirk. Completion screen Wrapped in `gum style --border rounded --padding "1 2"` with a colored build-status row (✓ green / ✗ red / ○ dim) and bold section headers inside. Plain fallback path preserved for non-gum environments. Two sections — Getting started, Maintenance — each list 3-4 commands so users can find `ckipper worktree rebuild-image` and `ckipper account sync` without re-running the wizard. Tests - 521/521 pass. - Replaced the "renders descriptions inline" regression with two new pinning the new contract: descriptions DON'T appear in the summary body, and the summary points users at the picker for them.
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.
Summary
Round 2 of post-launch polish, addressing things found in a real user dry-run of
ckipper setup. Three parallel investigation agents covered: prompt cancel audit, detected-config layout redesign, wizard completeness review.Issue 1 — detected configuration layout
Replaced alternating wide-row + indented description rendering with a card-style stack. Each setting becomes two lines + a blank-line separator, in dim color for the description. Long values no longer push source markers out of alignment; empty values render as
(empty).Issue 2 — wizard completion handoff
5 minutes of docker build output buried the "Setup complete" message; the wizard then exited silently. Now:
_ckipper_setup_offer_image_buildrecordsok/failed/skippedand a colored banner surfaces it on the completion screen.ckipper worktree rebuild-image,ckipper account sync,ckipper config list,ckipper doctoralongside the existing launch commands.Issue 1b — per-account aliases auto-source
install.shwrites the source line; ackipper setupre-run never did. Without it,claude-<account>launchers silently don't work. Added_ckipper_setup_offer_aliases_sourcewith idempotency (skips if already sourced).Issue 3 — sync-after-2nd-add
Verified by trace:
_ckipper_setup_offer_initial_syncalready counts accounts AFTER the add and fires when count ≥ 2. No code change.Issue 4 — docker rebuild CLI
The dedicated command is
ckipper worktree rebuild-image— now mentioned in the completion summary and the wizard help so users can find it without re-runningsetup.Issue 5 — cancel propagation audit
Agent #1 audited every prompt site. Two unguarded fallback paths (
CKIPPER_NO_GUM=1):_ckipper_account_sync_pick_targets_fallback_ckipper_account_sync_pick_types(read-fallback branch)Both now propagate
_core_prompt_input's non-zero rc on cancel. Gum-path multi-select pickers are already protected by the dispatcher's empty-array length check + SPACE/ENTER hint from PR #41.Test plan
make test-unit— 520/520 shell tests pass (+10 new regressions)make lint-shell lint-zsh lint-fmt lint-merge-guards— cleanok/failed/skippedrebuild-image+account sync+failed-banner on failure