Post-sync-overhaul cohesion cleanup + Docker-mode README clarifications#38
Merged
Post-sync-overhaul cohesion cleanup + Docker-mode README clarifications#38
Conversation
…cker
Two fixes in lib/account/account-management.zsh:
1. Account validation/usage messages ("Account 'foo' is already
registered.", "Usage: ckipper account remove <name>", etc.) printed
to stdout. Other modules (lib/config, lib/worktree, doctor.zsh)
already use stderr, so `ckipper account remove foo 2>/dev/null`
silenced success but leaked the failure path. Routed every error
path through `>&2`.
2. The Keychain entry picker used by `ckipper account add --adopt`
hand-rolled `nl + read -r` instead of using _core_prompt_choose.
Switched to _core_prompt_choose with an explicit "(skip)" sentinel
so gum/no-gum routing matches every other interactive picker.
Two consistency cleanups left over from the sync overhaul:
1. Three sites bypassed _core_prompt_*: lib/account/sync/interactive.zsh
used raw `read -r "var?label"` for two free-form prompts, and
lib/setup/prompts.zsh:98 used raw `read -r` for a y/N. All three now
route through _core_prompt_input / _core_prompt_confirm. Same
behavior in no-gum mode; future helper-layer changes propagate
automatically.
2. Module-level globals in lib/account/sync/{dispatcher,engine,preview}
used the bare _SYNC_* prefix. Every other module uses _CKIPPER_*
per .claude/rules/shell-conventions.md. Renamed the eight
dispatcher state vars (_SYNC_FROM, _SYNC_TARGETS, _SYNC_INCLUDE,
_SYNC_EXCLUDE, _SYNC_DRY_RUN, _SYNC_YES, _SYNC_FORCE, _SYNC_CTX)
plus the unprefixed ALIASES_FILE_PERMS readonly in aliases.zsh.
Tests updated. Verified zero leftover _SYNC_* references via grep.
Three small additions that close gaps the README implies are covered: - ckipper doctor now checks (a) gum on PATH (FAIL — required for the setup/sync wizards), (b) ~/.zsh/completions/_ckipper exists and matches CKIPPER_COMPLETION_VERSION (WARN — stale completions only produce outdated tab suggestions), and (c) aliases.zsh parses with `zsh -n` (FAIL — a broken aliases file disables every per-account launcher in the user's shell). Also renamed MIN_HOOK_FILES to _CKIPPER_DOCTOR_MIN_HOOK_FILES while in doctor.zsh. - Tab completion now offers schema keys after `ck config get/set/unset`, sourced live from _CKIPPER_SCHEMA_TYPE. Bumped CKIPPER_COMPLETION_VERSION 7 → 8 so existing installs regenerate the cached completion file on next shell start. - The setup-wizard final message now lists all three Claude-launching paths (`ckipper run`, `claude-<account>`, bare `<account>`) and the `ck` interactive menu, plus a hint to run `ckipper doctor`. Previously it only mentioned `ckipper run`.
- Renamed slogan from "Multi-account Claude Code manager with Docker
isolation, per-account preferences, and worktree-aware launchers." to
"A lightweight Claude Code account, worktree, and Docker manager."
Shorter, scans cleaner.
- Core commands table now lists `sync` and `redeploy-hooks` under
account-namespace subcommands. Both are fully implemented and
documented later in the README, but the at-a-glance command index
was missing them.
- Renamed "Known limitations" → "Known limitations (Docker mode only)"
and "Multi-account caveats" → "Multi-account caveats (host and
Docker)" with intro lines that explicitly tag scope. The previous
headings left it ambiguous whether each limitation applied only to
the dockerized launch path or to ckipper in general.
- Added two Docker-mode limitations confirmed via research:
- Claude in Chrome MCP — the browser extension can't bridge the
host/container boundary (anthropics/claude-code#25506).
- Custom system-sound hooks — afplay/say/osascript don't exist in
the Linux container; built-in notify-bell.sh sidesteps via the
terminal bell (\a) character.
mswdev
force-pushed
the
feature/cohesion-and-doc-cleanup
branch
from
0a8e2b6 to
0199490
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
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
Closes the punch list from the post-sync-overhaul cohesion audit (research kicked off after PR #37 merged). Four logical commits, each independently revertable:
fix(account)— stderr + keychain picker. Account validation/usage messages printed to stdout, breaking2>/dev/nullpipelines. Also replaced the hand-rollednl + read -rkeychain picker in--adoptwith_core_prompt_chooseso gum/no-gum routing matches every other interactive picker.refactor— prompt helpers + constant prefixes. Three sites (sync interactive fallbacks, setup pick-keys fallback) bypassed_core_prompt_*; standardized them. Eight_SYNC_*module globals + two unprefixed readonlys (MIN_HOOK_FILES,ALIASES_FILE_PERMS) renamed to the_CKIPPER_*convention fromshell-conventions.md.feat— doctor + completion + wizard.ckipper doctornow checksgumon PATH, completion-file freshness, andaliases.zshparses cleanly viazsh -n. Tab completion now offers schema keys afterck config get/set/unset(bumped completion version 7→8). Setup-wizard final message now lists all three Claude-launching paths (ck run,claude-<account>, bare<account>) and theckinteractive menu, plus ackipper doctorhint.docs(readme)— slogan + table + Docker-mode clarifications. Slogan replaced. Core-commands table now lists the previously-omittedsyncandredeploy-hookssubcommands. Renamed Known limitations → Known limitations (Docker mode only) and Multi-account caveats → Multi-account caveats (host and Docker) so each issue's scope is unambiguous. Added two Docker-mode limitations the audit identified: Claude in Chrome MCP (browser extension can't bridge the container boundary, anthropics/claude-code#25506) and custom system-sound hooks (afplay/say/osascriptdon't exist in the Linux container).Test plan
make lint-shell lint-zsh lint-fmt lint-merge-guards— cleanbats --recursive .— 467 / 467 passingpython3 -m pytest— 6 / 6 passingckipper account remove nonexistent 2>/dev/null— should be silent (was leaking errors to stdout pre-fix)ck config get <TAB>— should now offer the 10 schema keysckipper doctor— should now report ongum, completion file, andaliases.zshparseckipper setup, complete flow, verify final message lists all three Claude-launching pathsScope notes
read -r "var?label"which writes the prompt to stderr correctly. The behavioral change after switching to_core_prompt_inputis that the prompt now displays the bracketed empty default (e.g.Enter comma-separated targets []:). Centralizing on the helper trades a minor visual change for consistency and future-proofing.find -maxdepth 3scan inside the completion function (slow on large~/Developertrees). Caching with mtime checks adds complexity for marginal benefit on typical setups; skipping per KISS.