Skip to content

Develop#47

Merged
mswdev merged 37 commits into
mainfrom
develop
May 28, 2026
Merged

Develop#47
mswdev merged 37 commits into
mainfrom
develop

Conversation

@mswdev
Copy link
Copy Markdown
Owner

@mswdev mswdev commented May 28, 2026

No description provided.

mswdev added 30 commits May 4, 2026 14:46
`gum choose --no-limit` exits 0 with empty stdout when the user presses
ENTER without first toggling items with SPACE. The single-select source
picker right before it accepts ENTER on its own, so muscle memory makes
this trivially easy to do — and the wizard previously returned 1 silently
in the targets case and printed only a bare "No types selected." in the
types case, leaving users to think the picker was broken.

Both empty-selection paths now print the same SPACE/ENTER hint to stderr
before returning 1. Adds two regression tests exercising the fallback
path (CKIPPER_NO_GUM=1) which mirrors the gum empty-stdout branch.
Two bugs surface together in `ckipper worktree list`:

1. Slow scan. `find` recursed unbounded under $CKIPPER_WORKTREES_DIR with
   only `*/node_modules/*` excluded, so the heavy build/cache trees
   (`dist`, `.next`, `target`, `__pycache__`, etc.) were walked in full.
   On a 6 GB / 380k-file worktrees tree the scan took ~700 ms. Pruning
   the known-heavy directory names brings it to ~30 ms (≈25× faster);
   bounding by depth was not viable because branches contain slashes
   (e.g. `feature/OGD-320-…`).

2. Spurious `branch='…'` lines under each bullet. `local branch` (no
   `=value`) inside the per-iteration loop body behaves like
   `typeset -p branch` once the variable already carries a value from a
   prior iteration. Hoist `branch` (and the other loop locals) above the
   `while` and assign without `local` inside, matching the existing
   precedent in lib/account/sync/preview.zsh.

Adds three regression tests:
- pruning skips `.git` files inside node_modules / dist / __pycache__
- branches with slashes still resolve (no depth bound)
- output never contains literal `branch=` lines
Two bugs surface together in `ckipper worktree list`:

1. Slow scan. `find` recursed unbounded under $CKIPPER_WORKTREES_DIR with
   only `*/node_modules/*` excluded, so the heavy build/cache trees
   (`dist`, `.next`, `target`, `__pycache__`, etc.) were walked in full.
   On a 6 GB / 380k-file worktrees tree the scan took ~700 ms. Pruning
   the known-heavy directory names brings it to ~30 ms (≈25× faster);
   bounding by depth was not viable because branches contain slashes
   (e.g. `feature/OGD-320-…`).

2. Spurious `branch='…'` lines under each bullet. `local branch` (no
   `=value`) inside the per-iteration loop body behaves like
   `typeset -p branch` once the variable already carries a value from a
   prior iteration. Hoist `branch` (and the other loop locals) above the
   `while` and assign without `local` inside, matching the existing
   precedent in lib/account/sync/preview.zsh.

Adds three regression tests:
- pruning skips `.git` files inside node_modules / dist / __pycache__
- branches with slashes still resolve (no depth bound)
- output never contains literal `branch=` lines
fix: sync picker hint + worktree list perf and output leak
…er spin

Bundled post-launch fixes uncovered by a real-user dry-run:

setup customize picker (Issue 1, 2)
  - Header now reads "Pick keys to customize (SPACE to mark, ENTER to confirm)"
    so the multi-select contract is obvious — same hazard as PR #41's sync
    picker. Users were habitually pressing ENTER on the first row (single-
    select muscle memory) and silently advancing with no overrides.
  - Detected-configuration table now carries a DESCRIPTION column so users
    can read what each key does without entering the picker. Polished the
    schema descriptions so bool keys state what `true` means explicitly
    (the user's reported confusion: "what is aliases_auto_source").

setup sync between existing accounts (Issue 3)
  - Cross-account sync was offered only after the wizard added a NEW
    account. Re-running `ckipper setup` on an established multi-account
    install never surfaced the sync feature. Added a top-level offer
    that fires when accounts >= 2.

docker image build via setup (Issue 4)
  - `_core_prompt_spin` wraps `gum spin -- "$@"`, which execs argv as a
    binary. Passing a shell function failed with "executable file not
    found in $PATH". Drop the spinner — the build streams its own
    progress over ~5 min, which the user wants to see anyway.

`_core_prompt_input` cancel propagation (Issue 5a)
  - `gum input` exits non-zero with empty stdout on Esc / Ctrl-C, but the
    helper substituted the default in both the cancel and empty-submit
    cases. The launcher branch prompt then created a worktree on
    `feature/dev` even when the user pressed Ctrl-X to back out.
  - Now propagates rc: cancel → return non-zero, no stdout. Empty submit
    → rc=0, default echoed (existing contract preserved).
  - Updated setup/dispatcher.zsh callers (account-name, customize-loop)
    to skip on cancellation rather than commit empty values.

launcher project autodetect depth (Issue 5b)
  - Maxdepth was 3, but the existing comment claimed support for the
    `<projects_dir>/<group>/<org>/<repo>` layout — `.git` sits one level
    deeper than the repo root, so depth 4 is needed. The user had a
    real project (~/Developer/AFF/happyhippo/hippo-vmail) hidden by
    the off-by-one. ~5 ms cost on the author's tree.

Tests
  - 510/510 shell tests pass, including the new regressions:
    * _core_prompt_input returns non-zero with no stdout on EOF
    * _ckipper_setup_offer_existing_sync skips < 2 accounts
    * _ckipper_setup_offer_existing_sync invokes / skips sync_dispatch
fix(worktree): speed up worktree list and stop leaking branch=… lines
Three follow-ups uncovered by self-review:

1. Detected-config DESCRIPTION column was unrenderable (Critical)
   The 4th column added in this PR overflowed the fixed-width table
   (`%-22s` does not truncate; descriptions run 90+ chars), making the
   table effectively unreadable. Render descriptions on the line below
   each row instead — keeps the three-column alignment intact and gives
   the description unlimited room. Picker labels also enriched with
   `key — description` so the user sees what each setting does while
   choosing what to customize, with awk extracting the bare key on the
   way out.

2. `lib/config/set.zsh` missed `_core_prompt_input` cancel audit
   (Important). With the new rc-propagating contract, an Esc/Ctrl-C
   during `ckipper config set <key>` (no value arg) would silently blank
   the key. Added `if ! value=$(...); then return 1` guard and a
   regression test that pipes EOF and asserts the writer is bypassed.

3. `_core_prompt_spin` was orphaned (Important). The only caller (setup)
   was removed in this PR. Per CLAUDE.md "no half-finished
   implementations" — the function and its two tests are gone. Its
   `gum spin -- $@` design was incompatible with shell functions, which
   is the predominant pattern in this codebase, so re-using it later
   would require redesigning it anyway.

Tests: 510/510 pass (net unchanged: -2 spin tests, +1 description-row,
+1 config-set cancel). Lint clean.
fix: 1.0 polish — setup wizard UX, prompt cancel, project depth, docker spin
…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.
fix(setup,sync): 1.0 polish round 2 — layout, completion handoff, cancel audit
Add _core_registry_update_at / _init_at / _check_version_at /
_migrate_v1_to_v2_at variants that take an explicit registry file
path; existing zero-arg wrappers delegate with $CKIPPER_REGISTRY as
default. Lock paths and tmpfiles derive from the file path so multiple
registries (accounts.json, desktop.json) do not contend on a shared
lock.

Prep for the desktop multi-instance feature, which needs its own
registry file.
$CKIPPER_DIR/desktop.json at version 1. Used by the new lib/desktop/
feature in subsequent commits.
Adds the routing skeleton for the new `ckipper desktop` subcommand
namespace. Subcommand handlers are stubbed and will be implemented in
subsequent commits.

- New feature dir lib/desktop/ with dispatcher.zsh + help.zsh
- New top-level command 'desktop' with short alias 'dt'
- macOS-only guard at the dispatcher entry
- Per-subcommand --help / -h is short-circuited to focused help text
The previous 'ckipper desktop add' / 'claude://' assertions also appeared
in the overview help, so a silent fall-through from per-subcommand help
to the overview would not have failed the test. Match on phrases that
only exist in the per-subcommand help block ('Prerequisite:' for add,
'Why this exists:' for login) so the routing is actually under test.
Generates a minimal macOS application bundle at the given path with:
- Info.plist (CFBundleExecutable, Identifier, Name, IconFile when icon copied)
- Contents/MacOS/launcher (zsh script execing /Applications/Claude.app
  with --user-data-dir baked in at generation time, not path-walked)
- Contents/Resources/AppIcon.icns (best-effort copy from system app)
- Launch Services indexing via lsregister -f (best-effort)

Display name is title-cased (Claude-Work.app) while the canonical name
and bundle identifier suffix stay lowercase. lsregister path is the
full system path (not in $PATH) — overridable via _CKIPPER_TEST_LSREGISTER
for tests. Source Claude.app path overridable via _CKIPPER_TEST_CLAUDE_APP.
The display name was derived solely from the canonical name; compute it
inside _write_plist via _title_case rather than threading it through
the orchestrator. Brings every function back to the project's 3-param
limit (.claude/rules/code-style.md). No behavior change; 7/7 tests
still pass.
Registers a new Claude Desktop instance: creates ~/.claude-desktop-<name>/,
writes a Claude-<Name>.app bundle to ~/Applications/, and records the
entry in ~/.ckipper/desktop.json. Validates the name regex, refuses on
duplicates, requires /Applications/Claude.app to be installed, and
refuses if the bundle path already exists. Prints a deep-link routing
tip when this brings the instance count to two or more.

Also demotes _CKIPPER_DESKTOP_SYSTEM_APP in bundle.zsh to honor an
env-supplied override (was a plain assignment that overwrote the
inherited value on source) so tests and the new Claude.app-presence
assertion can point it at a fake bundle.
Prints registered Desktop instances in a column layout: name, data dir,
bundle path, registered_at, running/stopped status. Status comes from
pgrep against the --user-data-dir cmdline argument so list reflects the
same probe that desktop remove/rename will use to refuse on live
instances.
Unregisters a Desktop instance from the registry, then interactively
prompts to delete the user-data dir (default N — preserves user data:
chats, settings, OAuth tokens) and the .app bundle (regeneratable via
desktop add). Refuses if a Claude Desktop process is currently running
against that user-data-dir.

The not-running probe is _ckipper_desktop_assert_not_running. Task 9
will likely consolidate or extend it (e.g., richer process info); for
now it lives next to its only callers (remove and the upcoming rename).
Renames a Desktop instance in place: moves the data dir, regenerates
the .app bundle under the new name, and atomically swaps the registry
entry. Refuses if the instance is running or if the destination name
is already taken, validates the new name against the lowercase regex,
and rolls back the directory move + bundle regeneration if the
registry update fails.

Helper splits (rename_validate / rename_perform_fs /
rename_swap_registry / rename_rollback_fs) keep every function inside
the 25-line / 2-nesting / 3-param caps from .claude/rules/code-style.md.
Two small cleanups surfaced by the Task 5-8 self-review:

- Extract the literal "2" in _ckipper_desktop_add_announce into a named
  constant (_CKIPPER_DESKTOP_DEEP_LINK_TIP_THRESHOLD) — the project's
  NO_MAGIC_NUMBERS rule has no exceptions, and the threshold has a
  domain meaning worth a doc comment.
- Replace the brittle '[Nn]othing\ to\ do' regex in the rename
  identical-names test with a quoted literal substring match. Bash
  regex's '\ ' is interpretation-dependent; double-quoted strings on
  the RHS of =~ are guaranteed literal in bash 3.2+ (bats's floor on
  macOS).
The helper was placed in instance-management.zsh during Tasks 7-8 as a
temporary measure. Its proper home is lib/desktop/launcher.zsh
alongside the launch / login functions that share the same pgrep
semantics. Callers (remove, rename) are unchanged — function name is
the same. Adds dedicated bats tests for the helper. Establishes
launcher.zsh as the new launcher module with module-level timing
constants used by Task 10's login dance.

Also lifts _install_fake_claude_app from instance-management_test.bats
into tests/lib/test-helper.bash so the new launcher_test.bats (and
future test files) can share it.
Quits ALL running Claude Desktop processes via SIGTERM (with SIGKILL
fallback after a 5s timeout), then opens only the target wrapper bundle.
Works around the macOS claude:// deep-link auth-callback routing
gotcha: with multiple instances running, the OAuth callback lands in
whichever Claude app was most recently active. Quitting everything
first guarantees the callback has only one place to land.

Timing constants are typeset -g (not readonly) so tests can shrink the
timeout for fast feedback. Removes the Task 10 stub from
dispatcher.zsh.
Opens the registered Desktop instance via open -n -a on its wrapper
bundle. Unlike desktop login, this does NOT quit other running
Claude instances — use it when you know auth flows aren't in play.

Removes the Task 11 stub from dispatcher.zsh, which now contains
zero "not yet implemented" stubs; all six desktop subcommands
(add, list, remove, rename, login, launch) are live.
New lib/desktop/doctor.zsh runs after the account/tooling doctor:
checks /Applications/Claude.app presence (FAIL only if instances exist),
desktop.json schema, per-instance data_dir + .app bundle existence,
plus an Info.plist parse via plutil when available. Warns when 2+
instances are registered (the claude:// deep-link reminder).

Module is feature-isolated: uses lib/core/* helpers only, with its own
fail/warn counters — does not touch the account-namespace doctor
helpers. Top-level dispatcher composes the exit codes via || rc=1.
Adds the merge-guard for the new desktop namespace and extends every
existing feature-isolation guard's target list to include lib/desktop/
so sibling features (account, worktree, config) can't reach into it
and desktop can't reach back. Orchestration dirs (launcher, setup, run)
remain exempt per the dispatcher-exception pattern in shell-conventions.md.

Updates shell-conventions.md's prefix inventory + guard list.
Bumps CKIPPER_COMPLETION_VERSION 8 → 9 so installed shells regenerate.
Adds 'desktop' / 'dt' to the top-level command list and a new
desktop_subs array. Instance-name arg3 completion reads keys from
~/.ckipper/desktop.json.
Adds 'Launch a Desktop instance', 'List Desktop instances', and 'Add a
Desktop instance' to the launcher menu. 'Launch' uses a new helper
that prompts the user to pick from registered instances; the other
two delegate directly to _ckipper_desktop_dispatch.

Updates menu test fixture for the new option count.
Single line added to the 'Getting started:' section of both the gum
and plain post-setup summary cards. Discovery-only; the wizard does
not walk through Desktop setup (CLI and Desktop are configured
independently per the design).
mswdev added 7 commits May 27, 2026 17:18
New 'Claude Desktop instances' section in README between 'Multiple
accounts' and 'Sync state between accounts'. Covers add/list/launch/
rename/remove, the claude:// deep-link gotcha + login workaround, on-
disk layout, and doctor coverage. Reinforces that CLI accounts and
Desktop instances are independently configured.

CHANGELOG entry under the Unreleased section listing the new
namespace, the login dance, doctor integration, the desktop.json
registry, the registry.zsh refactor that supports it, and the
completion version bump.
Applied review findings (in-feature dedup + small leaks). Deferred
cross-feature extractions to lib/core/ (doctor-render, path-tildify,
name-validator) out of scope for this PR.

- lib/core/registry.zsh: parametrized _at form's auto-migration message
  uses ${registry_file:t} instead of the hard-coded 'accounts.json'
  string (would have lied for future non-accounts migrations). Doc
  headers updated to match.
- lib/desktop/doctor.zsh: deleted _ckipper_desktop_doctor_instance_count
  (byte-identical duplicate of _ckipper_desktop_instance_count in
  instance-management.zsh — same feature dir, no isolation rule).
  Removed duplicate _CKIPPER_DESKTOP_DOCTOR_DEEP_LINK_THRESHOLD and
  consume the existing _CKIPPER_DESKTOP_DEEP_LINK_TIP_THRESHOLD from
  instance-management.zsh.
- lib/desktop/launcher.zsh: _ckipper_desktop_lookup_bundle collapsed
  from 2 jq invocations to 1 via jq's 'error()' on missing-key — on
  the launch / login hot path.
- lib/desktop/bundle.zsh: extracted _CKIPPER_DESKTOP_BUNDLE_VERSION
  constant; CFBundleShortVersionString + CFBundleVersion both reference
  it (last magic string in the file).
…sion

The .app bundle's Contents/MacOS/launcher used double-quoted strings
for --user-data-dir and the system app path, so any $VAR / backtick
/ "$(…)" in the baked-in paths would be re-expanded at runtime when
the wrapper bundle launches.

Currently unreachable — instance names are regex-validated to
^[a-z0-9_-]+$ and the path is $HOME/.claude-desktop-<name>, so
neither $ nor backticks can appear. Defense-in-depth fix per PR #45
code review: switch to single-quoted output and escape any embedded
single quotes via the standard '\\''-replacement idiom. Test
updated to assert the single-quote form.

No behavior change in any supported scenario.
feat: Claude Desktop multi-instance support
…mmands

The previous Desktop section only existed deep in the README, so the
top-of-page narrative (tagline, Inspired-by attribution, Problem,
Solution, core commands table) gave no signal that Desktop management
was a thing. Five surgical edits:

- Tagline: add 'Claude Desktop multi-instance setups' to the feature list
- Inspired-by: credit Philipp Stracker's gist, which inspired the .app
  wrapper + --user-data-dir approach
- The Problem: second paragraph naming the single-instance-by-default
  pain point that motivated ckipper desktop
- The Solution: second example block (ck desktop add work) parallel to
  ck run, showing the wrapper-bundle outcome
- Core commands table: new row for ck desktop with the dt alias and
  anchor to the deeper section
Consolidates findings from a 3-agent README review (structure +
consistency + onboarding). Tier-1 changes that all three agreed on,
applied as one cohesive edit.

Top-of-page additions:
- One-line 'who is this for' between tagline and credits
- Quick Start gains a first-commands-by-feature mini-list (ck run,
  ckipper account add, ck desktop add) so new readers see Desktop is
  a peer feature before scrolling 100 lines
- Prerequisites adds /Applications/Claude.app, qualified 'only if
  you'll use ck desktop instances' (currently a runtime trap for
  Desktop-curious readers)

Desktop section restructure:
- Moved to after 'Sync state between accounts' so the accounts+sync
  pair stays contiguous (structure-agent finding)
- Opens with a value-first sentence ('Run a personal Claude Desktop
  in one window and a work Claude Desktop in another...') instead of
  leading with the --user-data-dir mechanism
- New 'Accounts vs Desktop instances' comparison table — explicit
  about what does NOT cross over (no shared auth, no shared MCP,
  sync only on the CLI side, no Desktop default)
- Deep-link gotcha promoted from buried blockquote to its own H3
  '### Don't run /login with two instances open' (mirrors Multiple
  Accounts' '### Don't run the same account in two sessions' shape)
- 'Use an instance' split into 'Use' + 'List, rename, remove' to
  mirror the Multiple Accounts shape
- 'Diagnostics' H3 renamed to 'Diagnose' for parallelism

Trivial polish:
- Sync section code-fence language tags: sh -> bash (matches every
  other fence in the file)

Deferred to a follow-up PR (touches more sections, higher risk):
- Diagnostics consolidation (3 doctor mentions in different parents)
- Multi-account caveats -> Upstream caveats + Desktop fold-in
- OAuth-race deep-dive de-duplication
docs(readme): surface Desktop multi-instance in intro + Solution
@mswdev mswdev merged commit b09b161 into main May 28, 2026
2 checks passed
@mswdev mswdev deleted the develop branch May 28, 2026 20:34
@mswdev mswdev restored the develop branch May 28, 2026 20:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant