feat: sync system overhaul

[mswdev]

Summary

• Replace lib/account/sync.zsh (narrow --mcp/--settings/--all flag surface) with a polymorphic strategy-based sync system covering 10 syncable types.
• Interactive by default (gum pickers for source / targets / types); flag-driven for scripting (--include, --exclude, --dry-run, --yes, --force).
• Multi-destination (one source → many targets in a single invocation), summary + drill-down preview, timestamped backups, account sync undo for restore.
• Hard refusal when Claude is running on the destination (override with --force).
• Setup wizard offers initial sync after adding a 2nd-or-later account.
• Rename account sync-hooks → account redeploy-hooks to disambiguate install→all redeploy from the new peer-to-peer user-hook sync (--include hooks).

Architecture

Strategy pattern via declarative parallel arrays in lib/account/sync/registry.zsh (mirrors lib/config/schema.zsh). Engine in engine.zsh is type-agnostic — iterates the registry and dispatches to _ckipper_account_sync_<type>_{enumerate,compare,summary,diff,apply} functions per the strategy contract. Adding a new type is 3 array rows + 5 strategy functions; no engine edits.

lib/account/sync/
  registry.zsh          declarative type registry (10 types, 4 bundles)
  backup.zsh            backup dir/file copy, manifest, rollback, undo
  engine.zsh            type-agnostic main loop (refuse-claude, change-set, apply)
  preview.zsh           summary table + drill-down picker
  interactive.zsh       gum pickers for source / targets / types
  dispatcher.zsh        arg parsing + mode routing + integration
  strategies/
    structured.zsh      mcp + settings + prefs (JSON-key merges via jq)
    files_flat.zsh      claude-md, agents, commands, output-styles (sha256 compare)
    files_dir.zsh       skills (cp -a preserves symlinks)
    statusline.zsh      .statusLine + internal/external script detection
    hooks.zsh           user-only hooks (install allowlist filter) + paired settings

Safeguards

#	Safeguard	Implementation
1	Pre-write timestamped backups	Every destructive write copies the prior file/dir to <dst>/.ckipper-sync-backups/<UTC-ts>-from-<src>/
2	Per-invocation manifest	.ckipper-sync-manifest.json powers sync undo
3	Atomic per-file writes	mktemp + mv
4	JSON validation gate	jq -e . against tmpfile; refuses to commit invalid JSON
5	Hard refusal when Claude is running on dst	--force override required
6	Source ≠ destination guard	Identity check
7	--yes requires explicit intent	No "default yes"
8	Backup path always shown	Summary footer prints backup dir before apply prompt
9	Per-target atomic rollback	Failure mid-target restores from backup dir

Test plan

• make lint — green
• make test-unit — 438 bats + 6 pytest, all green
• make lint-merge-guards — green (_ckipper_account_sync_* pinned to lib/account/sync/)
• Manual: ckipper account sync (no args) launches the wizard
• Manual: ckipper account sync personal work --include mcp syncs servers and shows backup path
• Manual: ckipper account sync undo work restores from latest backup
• Manual: setup wizard offers initial sync after adding a 2nd account
• Manual: ckipper account redeploy-hooks still works as before

Notes for the reviewer

• Solo / pre-public license: no backwards-compat constraint. The old --mcp/--settings/--all flag surface is removed entirely (no aliases or shims).
• All new functions follow .claude/rules/: ≤25 lines, ≤2 nesting levels, ≤3 params (modulo the strategy-contract apply functions which take 4: src/dst/id/backup_dir — that's the engine's interface, intentional).
• Doc-headers on every public function per shell-conventions.md.
• Source-file caps respected: lib/account/ 5 files, lib/account/sync/ 6 files, lib/account/sync/strategies/ 5 files (cap is 10, test files don't count).
• Two zsh gotchas surfaced and noted in commit messages: local path=... corrupts $PATH (lowercase tied to array); local status=... fails (read-only alias for $?). Renamed to id/target and cmp_status/change_status respectively across strategy functions.

Plan / design references

• docs/plans/2026-05-02-sync-overhaul-design.md
• docs/plans/2026-05-02-sync-overhaul-implementation-plan.md

(Both gitignored per repo policy; included here as references for the reviewer.)

[mswdev]

Code review fixes pushed (7df8a43)

Addressed every Critical and Important issue from the code review.

Critical

• Statusline interpreter-prefix detection (statusline.zsh:58-70): walk every whitespace token of .statusLine.command, not just the first. Fixes the common bash <src>/script.sh form. Apply rewrite switched from anchored sub() to literal split + join for path-rewrite safety. New tests cover both detection and apply paths.
• Preview UX wired end-to-end (dispatcher.zsh:137-225, engine.zsh:188-204):
  • New _build_summaries calls each row's <type>_summary to populate the summaries TSV.
  • New 3-way prompt Apply / View changes / Abort; View changes enters the drill-down loop until Apply or Abort.
  • _count_changes now prints the trailing N changes (a new, b overwrite) line (per design §6.1).
• 10 strategies' _summary functions are no longer dead code.

Important

• Legacy command hint (ckipper.zsh:106): [sync-hooks] now points at account redeploy-hooks.
• Drill-down id/display threading (preview.zsh:140-152): items file is the source of truth for the original id; new _drill_down_resolve_id(items, type, display) recovers it. agents/foo.md no longer collapses to foo.md in the diff lookup.
• Per-target rollback safety (engine.zsh:159-186): manifest_append fires BEFORE the apply call. A strategy that backs up + writes + then errors mid-write now has a manifest entry, so rollback restores from the backup dir. New test simulates a crashing apply and verifies recovery.
• _core_* rename (24 files): every _core_* defined in lib/account/sync/ renamed to _ckipper_account_sync_* (55 functions). _core_* is reserved for lib/core/ per .claude/rules/shell-conventions.md. Two-pass sed: _core_account_sync_ → _ckipper_account_sync_, then _core_sync_ → _ckipper_account_sync_. The dynamic strategy-fn dispatch is unaffected (it builds names from the _ckipper_account_sync_<type>_<verb> template).
• New lint-merge-guards rule (Makefile:59): catches future _core_* function definitions outside lib/core/. Would have failed this PR's pre-fix state; now guards against the same drift recurring.

Verification

• make lint — green
• make test-unit — 444 bats + 6 pytest, all green (was 438; +6 from new CR-fix tests)
• make lint-merge-guards — green

Minor (not addressed in this push)

• Stale comment in _ckipper_account_help_for about a function that no longer exists — cosmetic
• _apply_one's 8 args — could be packed into a context array for clarity, but it's an internal bridge function
• manifest_init re-chmods the manifest on every append — redundant but harmless

These are noted for a follow-up if anything else lands on this branch.

[mswdev]

Code review

Found 5 issues:

1. _ckipper_account_sync_preview_prompt is called inside command substitution at dispatcher.zsh:153, but the "View changes" arm runs _ckipper_account_sync_drill_down_loop without redirecting stdout. The strategy _diff functions and the "(Press enter to return to picker)" lines all print to stdout, which gets captured into $action. After viewing a diff and picking "Apply", $action ends up as <diff text>...apply, the [[ "$action" == "apply" ]] check at dispatcher.zsh:205 fails, and _ckipper_account_sync_apply_target is never invoked — sync silently does nothing. Tests don't exercise this path.

Ckipper/lib/account/sync/dispatcher.zsh

Lines 150 to 158 in 7df8a43

	_ckipper_account_sync_show_preview "$target" "$dst_dir" "$changeset" "$summaries"
	local action="apply"
	if [[ "$_SYNC_DRY_RUN" != "true" && "$_SYNC_YES" != "true" ]]; then
	action=$(_ckipper_account_sync_preview_prompt "$src_dir" "$dst_dir" "$target" "$items")
	fi
	[[ "$_SYNC_DRY_RUN" == "true" ]] && action="dry-run"
	_ckipper_account_sync_finalize "$action" "$src_dir" "$dst_dir" \
	"$target" "$changeset" "$summaries" "$items"
	}

Ckipper/lib/account/sync/preview.zsh

Lines 88 to 103 in 7df8a43

	# Args: $1 — src dir; $2 — dst dir; $3 — src name; $4 — dst name; $5 — items file.
	# Returns: 0 always.
	_ckipper_account_sync_drill_down_loop() {
	local src_dir="$1" dst_dir="$2" src_name="$3" dst_name="$4" items_file="$5"
	[[ ! -s "$items_file" ]] && { echo "No overwrites to drill into."; return 0; }
	while true; do
	local choice
	choice=$(_ckipper_account_sync_drill_down_pick "$items_file") || return 0
	[[ "$choice" == "Back" || -z "$choice" ]] && return 0
	_ckipper_account_sync_drill_down_show "$choice" "$items_file" \
	"$src_dir" "$dst_dir" "$src_name" "$dst_name"
	echo ""
	echo "(Press enter to return to picker)"
	local _ack; read -r _ack
	done
	}

2. 3-parameter cap violation — systemic across the new sync subsystem (.claude/rules/code-style.md says "MAXIMUM 3 parameters per method. Beyond that, introduce a parameter object or rethink the design." — listed as a hard limit, not guideline). Worst offenders: _ckipper_account_sync_apply_one (8 params), _ckipper_account_sync_finalize (7), _ckipper_account_sync_drill_down_show (6), _ckipper_account_sync_drill_down_loop (5). The PR's own minor list acknowledged apply_one, but the rule has no "internal bridge" exception.

Ckipper/lib/account/sync/engine.zsh

Lines 168 to 181 in 7df8a43

	# Args: $1 — type; $2 — src_dir; $3 — dst_dir; $4 — src_name; $5 — dst_name;
	# $6 — id; $7 — change status; $8 — backup_dir.
	# Returns: 0 on success; non-zero on apply failure.
	_ckipper_account_sync_apply_one() {
	local type="$1" src_dir="$2" dst_dir="$3" src_name="$4" dst_name="$5"
	local id="$6" change_status="$7" backup_dir="$8"
	local apply_fn; apply_fn=$(_ckipper_account_sync_strategy_fn "$type" apply)
	local arg_a="$src_dir" arg_b="$dst_dir"
	[[ "$type" == "prefs" ]] && { arg_a="$src_name"; arg_b="$dst_name"; }
	local op="overwrite"; [[ "$change_status" == "new" ]] && op="create"
	local rel; rel=$(_ckipper_account_sync_manifest_rel "$type" "$id")
	_ckipper_account_sync_manifest_append "$backup_dir" "$rel" "$op" "$type" "$id"
	"$apply_fn" "$arg_a" "$arg_b" "$id" "$backup_dir"
	}

3. Nesting cap violation in _ckipper_account_sync_parse_args (.claude/rules/code-style.md says "MAXIMUM 2 levels of control flow nesting per method. If you need a third level, extract a method."). Structure: while (L1) → case (L2) → if/else inside the *) arm (L3).

Ckipper/lib/account/sync/dispatcher.zsh

Lines 36 to 55 in 7df8a43

	_ckipper_account_sync_parse_args() {
	_ckipper_account_sync_reset_args
	while [[ $# -gt 0 ]]; do
	case "$1" in
	--include) _SYNC_INCLUDE="$2"; shift 2 ;;
	--exclude) _SYNC_EXCLUDE="$2"; shift 2 ;;
	--dry-run) _SYNC_DRY_RUN="true"; shift ;;
	--yes) _SYNC_YES="true"; shift ;;
	--force) _SYNC_FORCE="true"; shift ;;
	-h|--help) _ckipper_account_sync_help_text; return 2 ;;
	--*) echo "Unknown flag: $1" >&2; return 1 ;;
	*)
	if [[ -z "$_SYNC_FROM" ]]; then _SYNC_FROM="$1"
	else _SYNC_TARGETS+=("$1"); fi
	shift
	;;
	esac
	done
	return 0
	}

4. CHANGELOG.md falsely claims lib/account/sync.zsh is "intentionally untouched — full sync overhaul ships in a separate future PR." This appears in the [Unreleased] section that documents this PR; the file was actually deleted (commit ea61ee6) and replaced with the entire lib/account/sync/ tree. Future readers will hunt for an untouched file that no longer exists.

Ckipper/CHANGELOG.md

Lines 44 to 47 in 7df8a43

	### Notes
	- `lib/account/sync.zsh` is intentionally untouched — full sync overhaul ships in a separate future PR.

5. _ckipper_account_sync_undo_dispatch writes Unknown flag: $1 to stderr at line 258 but the doc-header lacks the required Errors (stderr): line. .claude/rules/shell-conventions.md says: doc-headers must include Errors (stderr): "<exact message>" — <when> for any function that writes to stderr (omit only if it never does). The sibling _ckipper_account_sync_parse_args doc-header gets this right.

Ckipper/lib/account/sync/dispatcher.zsh

Lines 244 to 260 in 7df8a43

	# Undo subcommand dispatcher.
	#
	# Args: $1 — account name; flags: --pick | --list | --force.
	# Returns: 0 on success; 1 on user-visible failure.
	_ckipper_account_sync_undo_dispatch() {
	local account="$1"; shift 2>/dev/null
	[[ -z "$account" ]] && { echo "Usage: ckipper account sync undo <account>" >&2; return 1; }
	local dst_dir; dst_dir=$(_core_account_dir "$account") || return 1
	local mode="latest"
	while [[ $# -gt 0 ]]; do
	case "$1" in
	--pick) mode="pick"; shift ;;
	--list) mode="list"; shift ;;
	--force) _SYNC_FORCE="true"; shift ;;
	*) echo "Unknown flag: $1" >&2; return 1 ;;
	esac
	done

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[mswdev]

Code review

Found 16 issues. Verified by 5 parallel reviewers + a validator agent against .claude/CLAUDE.md and .claude/rules/*.

Bugs (correctness / data-loss risk)

1. mcp_compare and settings_compare return "overwrite" instead of "new" when the destination file is absent. jq … 2>/dev/null on a missing file yields an empty string, so the [[ "$d" == "null" ]] guard never fires and the function falls through to echo "overwrite". The wrong op is recorded in the manifest, so a later undo looks for a backup that was never made and the new file is left in place — a brand-new account is the common case that hits this.

Ckipper/lib/account/sync/strategies/structured.zsh

Lines 62 to 72 in 0c3724f

	# Returns: 0; prints "new" | "overwrite" | "unchanged".
	_ckipper_account_sync_mcp_compare() {
	local src="$1" dst="$2" name="$3"
	local s d
	s=$(jq -c --arg n "$name" '.mcpServers[$n] // null' "$src/.claude.json" 2>/dev/null)
	d=$(jq -c --arg n "$name" '.mcpServers[$n] // null' "$dst/.claude.json" 2>/dev/null)
	if [[ "$d" == "null" ]]; then echo "new"; return 0; fi
	if [[ "$s" == "$d" ]]; then echo "unchanged"; return 0; fi
	echo "overwrite"
	}

Ckipper/lib/account/sync/strategies/structured.zsh

Lines 148 to 159 in 0c3724f

	# Returns: 0; prints "new" | "overwrite" | "unchanged".
	_ckipper_account_sync_settings_compare() {
	local src="$1" dst="$2" id="$3"
	local jq_path; jq_path=$(_ckipper_account_sync_settings_jq_path "$id")
	local s d
	s=$(jq -c "$jq_path // null" "$src/settings.json" 2>/dev/null)
	d=$(jq -c "$jq_path // null" "$dst/settings.json" 2>/dev/null)
	if [[ "$d" == "null" ]]; then echo "new"; return 0; fi
	if [[ "$s" == "$d" ]]; then echo "unchanged"; return 0; fi
	echo "overwrite"
	}

2. hooks_filter_src uses jq gsub($src; $dst) to rewrite hook command paths. jq's gsub treats its first argument as a regex, so . in ~/.claude-personal matches any char. The sibling statusline_copy_and_rewrite deliberately uses split($src) | join($dst) for this exact reason and has a comment explaining why — the hooks path missed it.

Ckipper/lib/account/sync/strategies/hooks.zsh

Lines 143 to 161 in 0c3724f

	# Returns: 0; prints filtered hooks JSON object (may be empty {}).
	_ckipper_account_sync_hooks_filter_src() {
	local src_settings="$1" script_basename="$2" src="$3" dst="$4"
	jq --arg sb "$script_basename" --arg src "$src" --arg dst "$dst" '
	(.hooks // {})
	| to_entries
	| map({
	key: .key,
	value: (
	.value
	| map(.hooks |= map(select(.command | tostring | contains("/" + $sb))))
	| map(select(.hooks | length > 0))
	| map(.hooks |= map(.command |= gsub($src; $dst)))
	)
	})
	| map(select(.value | length > 0))
	| from_entries
	' "$src_settings"
	}

Ckipper/lib/account/sync/strategies/statusline.zsh

Lines 131 to 137 in 0c3724f

	cp -a "$internal" "$dst/$rel" || return 1
	# Literal split+join (NOT sub/gsub) — paths often contain regex
	# metacharacters (`.`, `-`) and the interpreter-prefix form means
	# the `$src` substring is not necessarily at position 0.
	echo "$obj" | jq --arg src "$src" --arg dst "$dst" \
	'.command = (.command | split($src) | join($dst))'
	}

3. statusline_copy_and_rewrite writes both $dst/settings.json AND the internal script at $dst/$rel, but manifest_rel only emits one manifest entry (settings.json). The script file is backed up but no manifest row is appended for it. On a mid-write crash, rollback restores settings.json and leaves the new script orphaned — exactly the failure mode apply_one's comment warns against.

Ckipper/lib/account/sync/strategies/statusline.zsh

Lines 124 to 137 in 0c3724f

	# $4 — backup_dir; $5 — statusline JSON object.
	# Returns: 0 on success (prints rewritten JSON); 1 on cp failure.
	_ckipper_account_sync_statusline_copy_and_rewrite() {
	local src="$1" dst="$2" internal="$3" backup_dir="$4" obj="$5"
	local rel="${internal#$src/}"
	_ckipper_account_sync_backup_file "$backup_dir" "$dst/$rel" "$rel" || return 1
	mkdir -p "$dst/${rel:h}"
	cp -a "$internal" "$dst/$rel" || return 1
	# Literal split+join (NOT sub/gsub) — paths often contain regex
	# metacharacters (`.`, `-`) and the interpreter-prefix form means
	# the `$src` substring is not necessarily at position 0.
	echo "$obj" | jq --arg src "$src" --arg dst "$dst" \
	'.command = (.command | split($src) | join($dst))'
	}

Ckipper/lib/account/sync/engine.zsh

Lines 177 to 209 in 0c3724f

	# Manifest is appended BEFORE the apply call, not after. If the apply
	# crashes mid-write (backed-up the file, started writing, errored), the
	# manifest still contains the entry so rollback can restore from the
	# backup dir. Without this, mid-write failures leave the destination
	# half-written with no manifest record (rollback would skip the file).
	#
	# Args: $1 — type; $2 — id; $3 — change status.
	# Returns: 0 on success; non-zero on apply failure.
	_ckipper_account_sync_apply_one() {
	local type="$1" id="$2" change_status="$3"
	local apply_fn; apply_fn=$(_ckipper_account_sync_strategy_fn "$type" apply)
	local arg_a="${_SYNC_CTX[src_dir]}" arg_b="${_SYNC_CTX[dst_dir]}"
	[[ "$type" == "prefs" ]] && { arg_a="${_SYNC_CTX[src_name]}"; arg_b="${_SYNC_CTX[dst_name]}"; }
	local op="overwrite"; [[ "$change_status" == "new" ]] && op="create"
	local rel; rel=$(_ckipper_account_sync_manifest_rel "$type" "$id")
	_ckipper_account_sync_manifest_append "${_SYNC_CTX[backup_dir]}" "$rel" "$op" "$type" "$id"
	"$apply_fn" "$arg_a" "$arg_b" "$id" "${_SYNC_CTX[backup_dir]}"
	}
	# Compute the manifest's path field for a given (type, id). The relpath
	# is what _ckipper_account_sync_rollback_one operates on.
	#
	# Args: $1 — type; $2 — id.
	# Returns: 0; prints relpath.
	_ckipper_account_sync_manifest_rel() {
	local type="$1" id="$2"
	case "$type" in
	mcp) echo ".claude.json" ;;
	settings|statusline) echo "settings.json" ;;
	prefs) echo "accounts.json" ;;
	*) echo "$id" ;;
	esac
	}

4. settings and statusline both map to settings.json in manifest_rel. backup_file is non-idempotent — the second call overwrites the original-state backup with the post-first-write state. A failed apply rolls back to the partial state, not the original.

Ckipper/lib/account/sync/engine.zsh

Lines 200 to 209 in 0c3724f

	# Returns: 0; prints relpath.
	_ckipper_account_sync_manifest_rel() {
	local type="$1" id="$2"
	case "$type" in
	mcp) echo ".claude.json" ;;
	settings|statusline) echo "settings.json" ;;
	prefs) echo "accounts.json" ;;
	*) echo "$id" ;;
	esac
	}

Ckipper/lib/account/sync/backup.zsh

Lines 44 to 59 in 0c3724f

	# Copy a single file or directory from the destination into the backup
	# dir at the given relative path. No-op when the source path does not
	# exist (i.e. operation is "create" — there's nothing to back up).
	#
	# Args: $1 — backup_dir; $2 — absolute source path; $3 — relative destination path.
	# Returns: 0 on success or no-op; 1 if cp fails.
	_ckipper_account_sync_backup_file() {
	local backup_dir="$1" src="$2" rel="$3"
	[[ ! -e "$src" ]] && return 0
	local dst="$backup_dir/$rel"
	mkdir -p "${dst:h}"
	cp -a "$src" "$dst" || return 1
	[[ -f "$dst" ]] && chmod "$_CKIPPER_SYNC_BACKUP_FILE_PERMS" "$dst"
	return 0
	}

5. assert_dst_idle runs unconditionally in run_one_target before the --dry-run branch is consulted. --dry-run aborts with "Refusing to sync: Claude is running…" even though no writes will happen. The pre-rewrite lib/account/sync.zsh skipped this guard for dry-run.

Ckipper/lib/account/sync/dispatcher.zsh

Lines 156 to 182 in 0c3724f

	# Returns: 0 on success; 1 on failure.
	_ckipper_account_sync_run_one_target() {
	local target="$1"; shift
	local src_dir dst_dir
	src_dir=$(_core_account_dir "$_SYNC_FROM")
	dst_dir=$(_core_account_dir "$target")
	_ckipper_account_sync_assert_dst_idle "$dst_dir" "$_SYNC_FORCE" || return 1
	local changeset summaries items
	changeset=$(mktemp); summaries=$(mktemp); items=$(mktemp)
	_SYNC_CTX=(
	src_dir "$src_dir" dst_dir "$dst_dir"
	src_name "$_SYNC_FROM" dst_name "$target"
	changeset "$changeset" summaries "$summaries" items "$items"
	)
	_ckipper_account_sync_build_change_set "$src_dir" "$dst_dir" \
	"$_SYNC_FROM" "$target" "$@" > "$changeset"
	_ckipper_account_sync_build_summaries "$src_dir" "$dst_dir" \
	"$_SYNC_FROM" "$target" < "$changeset" > "$summaries"
	_ckipper_account_sync_drill_down_items < "$changeset" > "$items"
	_ckipper_account_sync_show_preview "$target" "$dst_dir" "$changeset" "$summaries"
	local action="apply"
	if [[ "$_SYNC_DRY_RUN" != "true" && "$_SYNC_YES" != "true" ]]; then
	action=$(_ckipper_account_sync_preview_prompt)
	fi
	[[ "$_SYNC_DRY_RUN" == "true" ]] && action="dry-run"
	_ckipper_account_sync_finalize "$action"
	}

6. account sync-hooks is documented as "hidden but callable" in the dispatcher doc-header and as "still callable" in CHANGELOG.md:43, but the case statement only matches redeploy-hooks — sync-hooks falls to the *) _ckipper_account_unknown arm. CHANGELOG.md:20 also contradicts line 43 in the same Unreleased section ("Renamed" vs "still callable").

Ckipper/lib/account/dispatcher.zsh

Lines 15 to 43 in 0c3724f

	# Dispatch an `account` subcommand.
	#
	# Args:
	# $1 — subcommand name (add, list, default, remove, rename, sync,
	# sync-hooks [hidden but callable], help, -h, --help, or empty)
	# $2..$N — arguments forwarded to the subcommand handler
	#
	# Returns: 0 on success; 1 on unknown subcommand.
	#
	# Errors (stderr):
	# "Unknown command: '<cmd>'. Did you mean: '<match>'? ..."
	_ckipper_account_dispatch() {
	local cmd="$1"
	shift 2>/dev/null
	case "$cmd" in
	add|list|default|remove|rename|redeploy-hooks)
	if [[ "$1" == "--help" || "$1" == "-h" ]]; then
	_ckipper_account_help_for "$cmd"
	return 0
	fi
	"_ckipper_account_${cmd//-/_}" "$@"
	;;
	sync)
	_ckipper_account_sync_dispatch "$@"
	;;
	""|help|-h|--help) _ckipper_account_help ;;
	*) _ckipper_account_unknown "$cmd"; return 1 ;;
	esac
	}

Ckipper/CHANGELOG.md

Lines 19 to 44 in 0c3724f

	- **New:** Setup wizard offers initial sync after adding a 2nd-or-later account.
	- **Renamed:** `ckipper account sync-hooks` → `ckipper account redeploy-hooks`. The new name reflects that it deploys the ckipper-managed safety hooks from the install dir to every account; it is NOT peer-to-peer sync.
	- **Removed:** Old flag surface (`--mcp [names]`, `--settings <keys>`, `--all`). The new `--include` / `--exclude` model with bundles supersedes these.
	### Added
	- `ckipper setup` — interactive wizard for configuring Ckipper. Re-runnable.
	- `ckipper config get / set / unset / list / edit` — view and modify settings.
	- `ckipper run <project> <branch>` — top-level shortcut for `ckipper worktree run`.
	- Bare `ck` (no args) — interactive launcher menu.
	- `ckipper doctor --fix` — gum-driven repairs, including the former `repair-plugins` flow.
	- Per-account preferences: `always_docker`, `always_firewall`, `ssh_forward`, populating flag defaults for `ck run`/`ck wt run`.
	- New global config keys: `CKIPPER_DEFAULT_BRANCH`, `CKIPPER_DEP_INSTALL_CMD`, `CKIPPER_NOTIFY_BELL`, `CKIPPER_ALIASES_AUTO_SOURCE`.
	- New flags: `--no-docker`, `--no-firewall`, `--ssh-forward`, `--no-ssh-forward` (override per-account preferences inline).
	- Auto-detection of `origin/HEAD` for the worktree base branch.
	- Restyled output across `ck account list`, `ck worktree list`, `ck doctor`, and all `--help` text via shared `lib/core/style.zsh`.
	- `ck doctor` validates `accounts.json` v2 preferences shape and `ckipper-config.zsh` keys against the schema.
	### Changed
	- `accounts.json` schema bumped v1 → v2; auto-migrates on first command after upgrade. Backup written to `accounts.json.v1.bak.<timestamp>`.
	- `install.sh` now ends by auto-invoking `ckipper setup` in interactive shells.
	- `gum` is a hard prereq (added to `install.sh` prereq check and `make bootstrap`).
	### Removed
	- `ckipper account repair-plugins` (folded into `ckipper doctor --fix`).
	- `ckipper account sync-hooks` from public help (still callable; auto-runs after install/setup).

7. manifest_append leaks $tmp in the backup dir on jq failure. The > "$tmp" && mv && chmod chain short-circuits and never rm -fs the orphaned tmp file. (Sibling json_atomic_write in structured.zsh does clean up on failure — same shape, divergent handling.)

Ckipper/lib/account/sync/backup.zsh

Lines 80 to 90 in 0c3724f

	# $4 — type id; $5 — items (comma-separated, optional).
	# Returns: 0 on success; 1 on jq failure.
	_ckipper_account_sync_manifest_append() {
	local backup_dir="$1" rel="$2" op="$3" type="$4" items="${5:-}"
	local manifest="$backup_dir/$_CKIPPER_SYNC_MANIFEST_FILE"
	local tmp; tmp=$(mktemp "$manifest.XXXXXX")
	jq --arg p "$rel" --arg o "$op" --arg t "$type" --arg i "$items" \
	'.files += [{path: $p, operation: $o, type: $t, items: ($i | split(",") | map(select(length > 0)))}]' \
	"$manifest" > "$tmp" && mv "$tmp" "$manifest" \
	&& chmod "$_CKIPPER_SYNC_BACKUP_FILE_PERMS" "$manifest"
	}

CLAUDE.md compliance — 3-param cap

rules/code-style.md says: "MAXIMUM 3 parameters per method. Beyond that, introduce a parameter object or rethink the design." The _SYNC_CTX map was introduced in the round-2 fix specifically to address this; eight call sites still violate it (one was added by the fix commit itself):

8. _ckipper_account_sync_walk_type — 5 params (type, src_dir, dst_dir, src_name, dst_name)

Ckipper/lib/account/sync/engine.zsh

Lines 117 to 130 in 0c3724f

	# Args: $1 — type; $2 — src_dir; $3 — dst_dir; $4 — src_name; $5 — dst_name.
	# Returns: 0 always; prints rows.
	_ckipper_account_sync_walk_type() {
	local type="$1" src_dir="$2" dst_dir="$3" src_name="$4" dst_name="$5"
	local enumerate_fn compare_fn
	enumerate_fn=$(_ckipper_account_sync_strategy_fn "$type" enumerate)
	compare_fn=$(_ckipper_account_sync_strategy_fn "$type" compare)
	local arg_a="$src_dir" arg_b="$dst_dir"
	[[ "$type" == "prefs" ]] && { arg_a="$src_name"; arg_b="$dst_name"; }
	local id display change_status
	while IFS=$'\t' read -r id display; do
	[[ -z "$id" ]] && continue
	change_status=$("$compare_fn" "$arg_a" "$arg_b" "$id")
	echo "$type"$'\t'"$id"$'\t'"$display"$'\t'"$change_status"

9. _ckipper_account_sync_manifest_append — 5 params; _ckipper_account_sync_rollback_one — 4 params

Ckipper/lib/account/sync/backup.zsh

Lines 80 to 90 in 0c3724f

	# $4 — type id; $5 — items (comma-separated, optional).
	# Returns: 0 on success; 1 on jq failure.
	_ckipper_account_sync_manifest_append() {
	local backup_dir="$1" rel="$2" op="$3" type="$4" items="${5:-}"
	local manifest="$backup_dir/$_CKIPPER_SYNC_MANIFEST_FILE"
	local tmp; tmp=$(mktemp "$manifest.XXXXXX")
	jq --arg p "$rel" --arg o "$op" --arg t "$type" --arg i "$items" \
	'.files += [{path: $p, operation: $o, type: $t, items: ($i | split(",") | map(select(length > 0)))}]' \
	"$manifest" > "$tmp" && mv "$tmp" "$manifest" \
	&& chmod "$_CKIPPER_SYNC_BACKUP_FILE_PERMS" "$manifest"
	}

Ckipper/lib/account/sync/backup.zsh

Lines 133 to 145 in 0c3724f

	# Returns: 0 on success; 1 on rm/mv failure.
	# Errors (stderr): "rollback failed: <rel> — <reason>"
	_ckipper_account_sync_rollback_one() {
	local backup_dir="$1" dst_dir="$2" op="$3" rel="$4"
	local live="$dst_dir/$rel"
	if [[ "$op" == "create" ]]; then
	rm -rf "$live" 2>/dev/null || {
	echo "rollback failed: $rel — could not remove created file" >&2
	return 1
	}
	return 0
	fi
	local backup_path="$backup_dir/$rel"

10. _ckipper_account_sync_statusline_copy_and_rewrite — 5 params; _ckipper_account_sync_hooks_filter_src — 4 params

Ckipper/lib/account/sync/strategies/statusline.zsh

Lines 124 to 137 in 0c3724f

	# $4 — backup_dir; $5 — statusline JSON object.
	# Returns: 0 on success (prints rewritten JSON); 1 on cp failure.
	_ckipper_account_sync_statusline_copy_and_rewrite() {
	local src="$1" dst="$2" internal="$3" backup_dir="$4" obj="$5"
	local rel="${internal#$src/}"
	_ckipper_account_sync_backup_file "$backup_dir" "$dst/$rel" "$rel" || return 1
	mkdir -p "$dst/${rel:h}"
	cp -a "$internal" "$dst/$rel" || return 1
	# Literal split+join (NOT sub/gsub) — paths often contain regex
	# metacharacters (`.`, `-`) and the interpreter-prefix form means
	# the `$src` substring is not necessarily at position 0.
	echo "$obj" | jq --arg src "$src" --arg dst "$dst" \
	'.command = (.command | split($src) | join($dst))'
	}

Ckipper/lib/account/sync/strategies/hooks.zsh

Lines 143 to 161 in 0c3724f

	# Returns: 0; prints filtered hooks JSON object (may be empty {}).
	_ckipper_account_sync_hooks_filter_src() {
	local src_settings="$1" script_basename="$2" src="$3" dst="$4"
	jq --arg sb "$script_basename" --arg src "$src" --arg dst "$dst" '
	(.hooks // {})
	| to_entries
	| map({
	key: .key,
	value: (
	.value
	| map(.hooks |= map(select(.command | tostring | contains("/" + $sb))))
	| map(select(.hooks | length > 0))
	| map(.hooks |= map(.command |= gsub($src; $dst)))
	)
	})
	| map(select(.value | length > 0))
	| from_entries
	' "$src_settings"
	}

11. _ckipper_account_sync_show_preview — 4 params (added by the round-2 cap-fix commit 0c3724f; all 4 args are already in _SYNC_CTX at the call site); _ckipper_account_sync_render_summary — 4 params

Ckipper/lib/account/sync/dispatcher.zsh

Lines 186 to 194 in 0c3724f

	# Args: $1 — target; $2 — dst_dir; $3 — changeset file; $4 — summaries file.
	# Returns: 0 always.
	_ckipper_account_sync_show_preview() {
	local target="$1" dst_dir="$2" changeset="$3" summaries="$4"
	local backup_path
	backup_path=$(_ckipper_account_sync_backup_dir_path "$dst_dir" "$_SYNC_FROM")
	_ckipper_account_sync_render_summary "$_SYNC_FROM" "$target" \
	"$backup_path" "$summaries" < "$changeset"
	local counts; counts=$(_ckipper_account_sync_count_changes < "$changeset")

Ckipper/lib/account/sync/preview.zsh

Lines 44 to 60 in 0c3724f

	# Args: $1 — src name; $2 — dst name; $3 — backup_dir path; $4 — summaries file.
	# Returns: 0 always.
	_ckipper_account_sync_render_summary() {
	local src_name="$1" dst_name="$2" backup_dir="$3" summaries="$4"
	echo ""
	echo "Sync $src_name → $dst_name"
	_ckipper_account_sync_print_divider
	local current_type=""
	local type id display change_status
	while IFS=$'\t' read -r type id display change_status; do
	[[ -z "$type" ]] && continue
	if [[ "$type" != "$current_type" ]]; then
	echo " ${_CKIPPER_SYNC_TYPE_LABEL[$type]:-$type}"
	current_type="$type"
	fi
	local summary=""
	if [[ -f "$summaries" ]]; then

12. The strategy contract documents apply <src_dir> <dst_dir> <id> <backup_dir> (4 params), but the generic files_flat_apply / files_dir_apply helpers actually take 5 (with a prepended type). The contract doc and impl disagree.

Ckipper/lib/account/sync/engine.zsh

Lines 29 to 40 in 0c3724f

	# JSON values use side-by-side `jq` pretty-print. May be empty for
	# new items (drill-down skips status==new in the preview UI).
	#
	# <type>_apply <src_dir> <dst_dir> <id> <backup_dir>
	# Perform the merge. MUST call _ckipper_account_sync_backup_file
	# before any destructive write. Returns 0 on success; non-zero on
	# failure (engine then triggers per-target rollback).
	#
	# All five functions take their arguments in the same order so the engine
	# can call them through _ckipper_account_sync_strategy_fn uniformly.
	# Per-target context shared by apply_target → apply_one and the preview/

CLAUDE.md compliance — other

13. Magic number 26 in printf '%-26s' — rules/code-style.md says "NO MAGIC NUMBERS EVER — ALWAYS EXTRACT TO A NAMED CONSTANT." Sibling defines _CKIPPER_SYNC_DIVIDER_WIDTH=45.

Ckipper/lib/account/sync/preview.zsh

Lines 30 to 38 in 0c3724f

	local cmp_status="$1" display="$2" summary="$3"
	case "$cmp_status" in
	new) printf ' %s %-26s (%s)\n' "$_CKIPPER_SYNC_BADGE_NEW" "$display" "${summary:-new}" ;;
	overwrite) printf ' %s %-26s (%s)\n' "$_CKIPPER_SYNC_BADGE_OVERWRITE" "$display" "${summary:-overwrite}" ;;
	unchanged) ;;
	esac
	}
	# Render the summary table. Reads a change-set on stdin (TSV rows), groups

14. ~20 wrapper functions in files_flat.zsh and files_dir.zsh lack doc-headers. rules/shell-conventions.md says "Document every public function with a comment block immediately above its definition with these labelled sections (Args, Returns, Errors)."

https://github.com/mswdev/Ckipper/blob/0c3724f8de86c42961465200c295d2620d73aa72/lib/account/sync/strategies/files_flat.zsh#L106-L134

Test gaps

rules/testing.md says "If logic makes a decision, it gets a test." No direct unit tests cover:

15. resolve_types (3-way: interactive picker / default-to-all / explicit --include); manifest_rel (4-arm case); undo_run (3-way: latest/pick/list); hooks_merge_settings and hooks_filter_src (only the end-to-end hooks_apply is tested).

Ckipper/lib/account/sync/dispatcher.zsh

Lines 142 to 154 in 0c3724f

	#
	# Returns: 0; prints type ids one per line.
	_ckipper_account_sync_resolve_types() {
	if [[ -z "$_SYNC_INCLUDE" && "$_SYNC_YES" != "true" && "$_SYNC_DRY_RUN" != "true" ]]; then
	_ckipper_account_sync_pick_types
	return 0
	fi
	[[ -z "$_SYNC_INCLUDE" ]] && _SYNC_INCLUDE="all"
	_ckipper_account_sync_resolve_includes "$_SYNC_INCLUDE" "$_SYNC_EXCLUDE"
	}
	# One-target slice: build change set, render preview, prompt, apply.
	#

Ckipper/lib/account/sync/engine.zsh

Lines 195 to 209 in 0c3724f

	# Compute the manifest's path field for a given (type, id). The relpath
	# is what _ckipper_account_sync_rollback_one operates on.
	#
	# Args: $1 — type; $2 — id.
	# Returns: 0; prints relpath.
	_ckipper_account_sync_manifest_rel() {
	local type="$1" id="$2"
	case "$type" in
	mcp) echo ".claude.json" ;;
	settings|statusline) echo "settings.json" ;;
	prefs) echo "accounts.json" ;;
	*) echo "$id" ;;
	esac
	}

UX / doc accuracy

16. Zero-change runs still show the Apply / View changes / Abort prompt — run_one_target has no early-return when count_changes is 0. The pre-rewrite sync printed "nothing to sync" and returned. Separately, the comment at lib/account/dispatcher.zsh:80-81 says _ckipper_account_help_text_sync is "kept only as a fallback" — that function does not exist anywhere in the tree.

Ckipper/lib/account/sync/dispatcher.zsh

Lines 173 to 182 in 0c3724f

	"$_SYNC_FROM" "$target" < "$changeset" > "$summaries"
	_ckipper_account_sync_drill_down_items < "$changeset" > "$items"
	_ckipper_account_sync_show_preview "$target" "$dst_dir" "$changeset" "$summaries"
	local action="apply"
	if [[ "$_SYNC_DRY_RUN" != "true" && "$_SYNC_YES" != "true" ]]; then
	action=$(_ckipper_account_sync_preview_prompt)
	fi
	[[ "$_SYNC_DRY_RUN" == "true" ]] && action="dry-run"
	_ckipper_account_sync_finalize "$action"
	}

Ckipper/lib/account/dispatcher.zsh

Lines 77 to 95 in 0c3724f

	# Per-subcommand help text router. Each arm prints a focused usage block.
	#
	# Note: the `sync` arm dispatches to the new sync subsystem's help via
	# parse_args before reaching this router, so `_ckipper_account_help_text_sync`
	# is no longer used here — kept only as a fallback.
	#
	# Args: $1 — subcommand name.
	# Returns: 0 always.
	_ckipper_account_help_for() {
	case "$1" in
	add) _ckipper_account_help_text_add ;;
	list) _ckipper_account_help_text_list ;;
	default) _ckipper_account_help_text_default ;;
	remove) _ckipper_account_help_text_remove ;;
	rename) _ckipper_account_help_text_rename ;;
	sync) _ckipper_account_sync_help_text ;;
	redeploy-hooks) _ckipper_account_help_text_redeploy_hooks ;;
	esac
	}

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}

[mswdev]

Fourth-round code review

Spawned a 5-agent review pass focused on functional bugs that would prevent the sync system from working. Six confirmed defects (validated against the source, then fixed in commit 56f2837):

1. apply_one op-derivation bug — destroys data on sync undo. change_status="new" fires when a sub-key is absent (e.g. one new MCP server) even if the file already exists with unrelated content. The manifest recorded op=create, and rollback's rm -rf $live deleted the entire .claude.json. Fix: derive op from file existence at apply time.

   Ckipper/lib/account/sync/engine.zsh

   Lines 185 to 207 in 56f2837

   	# backup dir. Without this, mid-write failures leave the destination
   	# half-written with no manifest record (rollback would skip the file).
   	#
   	# `op` is derived from whether the live file exists at apply time, NOT
   	# from change_status. change_status="new" can fire when a sub-key is
   	# absent from a file that already exists (e.g. adding one MCP server to a
   	# .claude.json that already has others); recording op=create there would
   	# make rollback rm-rf the whole file, destroying unrelated data.
   	#
   	# Args: $1 — type; $2 — id.
   	# Returns: 0 on success; non-zero on apply failure.
   	_ckipper_account_sync_apply_one() {
   	local type="$1" id="$2"
   	local apply_fn; apply_fn=$(_ckipper_account_sync_strategy_fn "$type" apply)
   	local arg_a="${_SYNC_CTX[src_dir]}" arg_b="${_SYNC_CTX[dst_dir]}"
   	[[ "$type" == "prefs" ]] && { arg_a="${_SYNC_CTX[src_name]}"; arg_b="${_SYNC_CTX[dst_name]}"; }
   	local rel; rel=$(_ckipper_account_sync_manifest_rel "$type" "$id")
   	local live; live=$(_ckipper_account_sync_live_path "$type" "${_SYNC_CTX[dst_dir]}" "$rel")
   	local op="overwrite"; [[ ! -e "$live" ]] && op="create"
   	_ckipper_account_sync_manifest_append "${_SYNC_CTX[backup_dir]}" "$rel" "$op" "$type" "$id"
   	"$apply_fn" "$arg_a" "$arg_b" "$id" "${_SYNC_CTX[backup_dir]}"
   	}

2. prefs rollback restores to wrong path — registry undo silently broken. prefs_apply backs up $CKIPPER_REGISTRY, but rollback_one resolved live as $dst_dir/accounts.json (the account dir, not the registry). Stray files appeared in the account dir; the registry was never restored. Fix: new live_path helper routes prefs to $CKIPPER_REGISTRY; rollback reads the type field from the manifest.

   Ckipper/lib/account/sync/backup.zsh

   Lines 21 to 34 in 56f2837

   	# Compute the LIVE absolute path for a manifest entry. Most types live
   	# under <dst_dir>/<rel>; prefs operates on $CKIPPER_REGISTRY (which is
   	# usually outside the destination account dir). Used by both apply_one
   	# (op-derivation in engine.zsh) and rollback_one so the two agree on
   	# what file is being touched.
   	#
   	# Args: $1 — type (may be empty for legacy callers); $2 — dst_dir;
   	# $3 — relpath from the manifest.
   	# Returns: 0; prints absolute path.
   	_ckipper_account_sync_live_path() {
   	local type="$1" dst_dir="$2" rel="$3"
   	case "$type" in
   	prefs) echo "$CKIPPER_REGISTRY" ;;
   	*) echo "$dst_dir/$rel" ;;

3. assert_dst_idle was dead code on macOS. pgrep -lx claude only emits <PID> claude — no env, no argv. The grep -F "$dst_dir" filter could never match. macOS doesn't expose foreign processes' CLAUDE_CONFIG_DIR, so dst-specific filtering isn't achievable; reverted to "refuse on any running Claude CLI."

   Ckipper/lib/account/sync/engine.zsh

   Lines 67 to 84 in 56f2837

   	# develop did the same; --force is the documented escape hatch.
   	#
   	# Args: $1 — destination dir (kept in the signature for forward
   	# compatibility once we have a dst-specific signal); $2 — force flag
   	# ("true" | "false").
   	# Returns: 0 if safe to proceed; 1 if any Claude CLI is running (unless force).
   	# Errors (stderr): multiline message identifying the running process(es).
   	_ckipper_account_sync_assert_dst_idle() {
   	local dst_dir="$1" force="$2"
   	[[ "$force" == "true" ]] && return 0
   	local procs; procs=$(_core_running_claude_processes 2>/dev/null)
   	[[ -z "$procs" ]] && return 0
   	{
   	echo "Refusing to sync: a Claude CLI process is running."
   	echo "$procs" | sed 's/^/ /'
   	echo ""
   	echo "Quit running Claude (or pass --force to override; risk of file races)."
   	} >&2

4. _SYNC_FORCE leaks from sync into sync undo. undo_dispatch read the module-level _SYNC_FORCE directly. After sync ... --force (which left _SYNC_FORCE=true), a subsequent sync undo (without --force) silently bypassed the running-Claude refusal because parse_args only resets the flag on the sync path. Fix: local force var in undo_dispatch.

   Ckipper/lib/account/sync/dispatcher.zsh

   Lines 292 to 312 in 56f2837

   	_ckipper_account_sync_undo_dispatch() {
   	local account="$1"; shift 2>/dev/null
   	[[ -z "$account" ]] && { echo "Usage: ckipper account sync undo <account>" >&2; return 1; }
   	local dst_dir; dst_dir=$(_core_account_dir "$account") || return 1
   	local mode="latest" force="false"
   	while [[ $# -gt 0 ]]; do
   	case "$1" in
   	--pick) mode="pick"; shift ;;
   	--list) mode="list"; shift ;;
   	--force) force="true"; shift ;;
   	*) echo "Unknown flag: $1" >&2; return 1 ;;
   	esac
   	done
   	_ckipper_account_sync_assert_dst_idle "$dst_dir" "$force" || return 1
   	_ckipper_account_sync_undo_run "$account" "$dst_dir" "$mode"
   	}
   	# Run the chosen undo mode (latest / pick / list).
   	#
   	# Args: $1 — account name; $2 — dst_dir; $3 — mode (latest|pick|list).
   	# Returns: 0 on success; 1 on failure or no backups.

5. statusline_compare missing empty-string guard. mcp_compare and settings_compare guard [[ -z "$d" || "$d" == "null" ]] for the dst-file-missing case; statusline only checks "null". When dst's settings.json was absent, compare returned "overwrite" instead of "new".

   Ckipper/lib/account/sync/strategies/statusline.zsh

   Lines 37 to 47 in 56f2837

   	_ckipper_account_sync_statusline_compare() {
   	local src="$1" dst="$2"
   	local s d
   	s=$(jq -c '.statusLine // null' "$src/settings.json" 2>/dev/null)
   	d=$(jq -c '.statusLine // null' "$dst/settings.json" 2>/dev/null)
   	# Empty `d` means the destination file is missing entirely (jq exited
   	# non-zero); see the equivalent guard in `_ckipper_account_sync_mcp_compare`.
   	if [[ -z "$d" || "$d" == "null" ]]; then echo "new"; return 0; fi
   	if [[ "$s" == "$d" ]]; then echo "unchanged"; return 0; fi
   	echo "overwrite"
   	}

6. hooks_apply mutates settings.json but doesn't record it in the manifest. manifest_rel returns the script path; the paired .hooks block mutation was untracked. Rollback after a hook sync left phantom .hooks entries pointing at deleted scripts. Fix: append an explicit settings.json manifest entry; multi-hook duplicates are harmless because rollback is idempotent.

   Ckipper/lib/account/sync/strategies/hooks.zsh

   Lines 93 to 121 in 56f2837

   	# Apply: copy script + write paired settings.hooks entry with rewritten paths.
   	#
   	# Records the settings.json mutation in the manifest in addition to the
   	# script file. The engine's apply_one only records the script entry (its
   	# manifest_rel returns "<id>" for hooks), but hooks_apply ALSO mutates
   	# settings.json — without an explicit manifest entry, rollback wouldn't
   	# restore settings.json and would leave dangling .hooks entries pointing
   	# at scripts that have just been deleted/restored.
   	#
   	# Multiple hooks in one sync each append a settings.json entry; rollback
   	# is idempotent so duplicate entries are harmless (each restore from the
   	# same backup yields the same pre-sync state).
   	#
   	# Args: $1 — src; $2 — dst; $3 — relpath; $4 — backup_dir.
   	# Returns: 0 on success; non-zero on cp/jq/write failure.
   	_ckipper_account_sync_hooks_apply() {
   	local src="$1" dst="$2" rel="$3" backup_dir="$4"
   	_ckipper_account_sync_backup_file "$backup_dir" "$dst/$rel" "$rel" || return 1
   	local settings_op="overwrite"; [[ -e "$dst/settings.json" ]] || settings_op="create"
   	_ckipper_account_sync_backup_file "$backup_dir" "$dst/settings.json" "settings.json" || return 1
   	_ckipper_account_sync_manifest_append "$backup_dir" "settings.json" "$settings_op" hooks "$rel"
   	mkdir -p "$dst/${rel:h}"
   	cp -a "$src/$rel" "$dst/$rel" || return 1
   	chmod +x "$dst/$rel" 2>/dev/null
   	_ckipper_account_sync_hooks_merge_settings "$src" "$dst" "$rel"
   	}
   	# Merge a single user-hook's paired settings.json entries from src into dst,
   	# rewriting absolute paths from src dir → dst dir. Operates by:

Each fix has a regression test that fails on develop and passes after the change. Test count: 455 → 467 (+12). All green; make lint (shell + zsh + fmt + merge-guards) passes.

🤖 Generated with Claude Code

_{- If this review was useful, please react with 👍. Otherwise, react with 👎.}

[mswdev]

Code review

Found 3 issues:

1. The settings sync strategy enumerates statusLine.* paths but only excludes .hooks, and settings_apply copies values verbatim with no path rewrite. Running ckipper account sync --include settings (or --include claude-config, which contains settings but not statusline) plants the source's absolute statusLine.command into the destination's settings.json, silently breaking the destination's statusline. The dedicated statusline strategy handles this rewrite, so the bug is masked under --include all/customizations but fires for the documented narrower includes.

Ckipper/lib/account/sync/strategies/structured.zsh

Lines 135 to 141 in 8adf336

	| . as $p
	| select(($root | getpath($p)) | type != "object")
	| ($p | map(tostring) | join(".")) as $k
	| select($k | startswith("hooks") | not)
	| "\($k)\t\($k)"
	' "$file" 2>/dev/null
	}

2. Magic number 26 (printf column width) hardcoded in two printf lines (.claude/rules/code-style.md says "NO MAGIC NUMBERS EVER — ALWAYS EXTRACT TO A NAMED CONSTANT"). The same file already extracts _CKIPPER_SYNC_DIVIDER_WIDTH=45 two lines above, so the missing extraction is visible in-context.

Ckipper/lib/account/sync/preview.zsh

Lines 29 to 36 in 8adf336

	_ckipper_account_sync_render_row() {
	local cmp_status="$1" display="$2" summary="$3"
	case "$cmp_status" in
	new) printf ' %s %-26s (%s)\n' "$_CKIPPER_SYNC_BADGE_NEW" "$display" "${summary:-new}" ;;
	overwrite) printf ' %s %-26s (%s)\n' "$_CKIPPER_SYNC_BADGE_OVERWRITE" "$display" "${summary:-overwrite}" ;;
	unchanged) ;;
	esac
	}

3. The sync-hooks → redeploy-hooks rename missed user-facing strings outside the diff: install.sh:124 is printed on every fresh install, and lib/account/doctor.zsh:416 directs the user to the legacy name when hooks/ is missing. Both now route through the legacy-rename hint and won't execute, so users who follow the on-screen instruction get an error instead of the action.

Ckipper/install.sh

Lines 121 to 127 in 8adf336

	# 6. Deploy settings-template.json (consumed by ckipper account add / sync-hooks per-account)
	echo "Copying settings-template.json to $CKIPPER_DIR/..."
	cp "$REPO_DIR/templates/settings-template.json" "$CKIPPER_DIR/settings-template.json"
	echo " Settings template deployed. ckipper account sync-hooks applies it per-account."
	# 7. Add or update source line in .zshrc
	# Pre-merge installs sourced w-function.zsh from ~/.claude/docker/ or

🤖 Generated with Claude Code

_{- If this code review was useful, please react with 👍. Otherwise, react with 👎.}