Add: --install-hook for auto-reapply after CC updates#654
Open
LeonFedotov wants to merge 5 commits intoPiebald-AI:mainfrom
Open
Add: --install-hook for auto-reapply after CC updates#654LeonFedotov wants to merge 5 commits intoPiebald-AI:mainfrom
LeonFedotov wants to merge 5 commits intoPiebald-AI:mainfrom
Conversation
Adds three new CLI options:
--install-hook Install a SessionStart hook in ~/.claude/settings.json
that runs `tweakcc --apply --quiet` on every CC session start
--remove-hook Remove the auto-reapply hook
--quiet Suppress output in --apply mode (for hook usage)
This addresses the long-standing issue of CC auto-updates wiping patches,
requiring users to manually re-run tweakcc after every update.
The hook runs `tweakcc --apply --quiet` which:
- Detects the current CC installation
- Reapplies all saved customizations
- Suppresses output so it doesn't pollute session startup
Relates to: Piebald-AI#101
Contributor
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CLI flags Changes
Sequence Diagram(s)sequenceDiagram
participant User as "User (CLI)"
participant CLI as "tweakcc CLI"
participant FS as "Filesystem (~/.claude/settings.json)"
participant Claude as "Claude Hooks (SessionStart)"
User->>CLI: invoke with --install-hook / --remove-hook / --apply [-q]
alt install/remove hook flow
CLI->>FS: read settings.json
FS-->>CLI: return JSON
CLI->>Claude: inspect/create `SessionStart` hooks entry
CLI->>FS: write updated settings.json (add/remove `tweakcc --apply [--quiet]`)
FS-->>CLI: write result
CLI-->>User: exit (messages suppressed if -q)
else apply (non-interactive) flow
CLI->>CLI: run handleApplyMode(quiet)
alt quiet = false
CLI-->>User: print patch results and statuses
else quiet = true
CLI-->>User: minimal output (suppress logs/patch printing)
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.tsx`:
- Around line 370-374: The code assumes settings.hooks and
settings.hooks.SessionStart are arrays and casts them directly to sessionStart,
which can throw when the shape is different; update the initialization of
hooks/sessionStart (symbols: hooks, sessionStart, settings.hooks, SessionStart)
to first check types using Array.isArray and guard the inner items' shape
(ensure each item is an object with optional matcher and hooks arrays) and
fallback to [] when invalid, and likewise guard any subsequent array ops (e.g.,
findIndex, some) by validating the target is an array before calling those
methods so malformed user settings cannot cause runtime errors.
- Around line 363-368: The try/catch around reading/parsing settings
(fsSync.readFileSync(settingsPath) and JSON.parse) currently treats parse errors
the same as a missing file and can overwrite user settings; change the catch to
distinguish a missing file (ENOENT) from a JSON parse error: if the file is
missing, initialize settings to an empty object; if JSON.parse throws, log the
parse error and leave the existing settings variable untouched (do not replace
it with a new minimal object or write out a new settings.json). Apply the same
fix to the other read/parse block mentioned (the block around lines 405-410) so
parse failures never overwrite existing settings.
- Around line 236-240: The CLI currently treats --install-hook and --remove-hook
together as install mode; update the handling around
options.installHook/options.removeHook so when both are truthy it fails fast:
detect the conflict, log or throw a clear error (or call process.exit with
non-zero) indicating the flags cannot be used together, and only call await
handleHookMode(!!options.installHook) when exactly one of options.installHook or
options.removeHook is set; reference the existing options.installHook,
options.removeHook, and handleHookMode to locate and modify the logic.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+363
to
+368
| try { | ||
| const raw = fsSync.readFileSync(settingsPath, 'utf8'); | ||
| settings = JSON.parse(raw) as Record<string, unknown>; | ||
| } catch { | ||
| // File doesn't exist or isn't valid JSON — start fresh | ||
| } |
Contributor
There was a problem hiding this comment.
Do not overwrite Claude settings when JSON parsing fails.
At Line 366-Line 368, invalid JSON is treated the same as a missing file. In install mode, this can replace an existing settings.json with a new minimal object and drop unrelated user settings.
Suggested fix
- } catch {
- // File doesn't exist or isn't valid JSON — start fresh
+ } catch (error) {
+ const err = error as NodeJS.ErrnoException;
+ if (err.code !== 'ENOENT') {
+ console.error(chalk.red(`Error reading ${settingsPath}: ${err.message}`));
+ process.exit(1);
+ }
}Also applies to: 405-410
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.tsx` around lines 363 - 368, The try/catch around reading/parsing
settings (fsSync.readFileSync(settingsPath) and JSON.parse) currently treats
parse errors the same as a missing file and can overwrite user settings; change
the catch to distinguish a missing file (ENOENT) from a JSON parse error: if the
file is missing, initialize settings to an empty object; if JSON.parse throws,
log the parse error and leave the existing settings variable untouched (do not
replace it with a new minimal object or write out a new settings.json). Apply
the same fix to the other read/parse block mentioned (the block around lines
405-410) so parse failures never overwrite existing settings.
Author
Note: native bytecode buildsThe |
…lags - Reject --install-hook + --remove-hook used together - Distinguish missing file from JSON parse error - Validate hooks/SessionStart are arrays before accessing Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.tsx`:
- Around line 371-392: The code currently treats any fs read error as "file
missing" and accepts any JSON (including null/array), so update the settings
loading around settingsPath and settings: when fsSync.readFileSync throws,
inspect the error.code and for ENOENT allow fresh start but for EACCES/EPERM and
other errors log a descriptive error via console.error (include error.message)
and exit; when parsing JSON, validate that JSON.parse(raw) returns a non-null
object that is not an array before assigning to settings—if the parsed value is
null, not an object, or an array, log an error describing that settings.json
must be an object and exit—then compute hooks from the validated settings object
(use the existing hooks detection logic but ensure settings is an object).
- Around line 401-458: The current search uses includes(hookMarker) and treats a
whole SessionStart entry as managed, which can misidentify unrelated hooks and
lead to deleting unrelated hooks; update the logic that inspects
hooks.SessionStart to find the specific hook by exact command match (compare
hook.command === hookCommand or, if you prefer, a stronger marker check combined
with the same matcher) and capture both the sessionStart index and the specific
hook index; when installing, only skip if that exact hook already exists in any
sessionStart entry; when removing, remove only that specific hook from the found
sessionStart entry (sessionStart[foundIdx].hooks.splice(hookIdx, 1)) and only
delete hooks.SessionStart if its hooks array becomes empty; update references
around hookMarker/hookCommand, sessionStart, existingIdx, and where you
currently splice the entire sessionStart to implement this safer per-hook
add/remove behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.tsx`:
- Around line 394-396: The catch block swallowing all errors after the
fs.readFileSync call must distinguish "file not found" from other errors; update
the catch to inspect the thrown error (e.g., NodeJS.ErrnoException.code) and
only treat ENOENT as “start fresh”, while rethrowing or surfacing any other
codes (EACCES/EPERM/etc.) so the process does not silently overwrite unreadable
files; locate the try/catch around readFileSync in src/index.tsx and modify that
catch accordingly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
8 tasks
Distinguish ENOENT (start fresh), SyntaxError (invalid JSON), non-object JSON, and permission errors (EACCES/EPERM) as separate cases instead of nested try/catch blocks. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
src/index.tsx (1)
402-420:⚠️ Potential issue | 🟠 MajorOnly match well-formed managed hook entries.
Array.isArray(rawSessionStart)only validates the outer container.nullitems or entries with non-arrayhookswill still blow up inentry.hooks?.some(...)/findIndex(...), and the current predicate also treats the same command under a non-emptymatcheras the managed hook. That means--install-hookcan report "already installed" against a user-owned entry and--remove-hookcan delete it later. Please guard each entry before calling array methods and requirematcher === '',type === 'command', and the exact command when locating the managed hook.Also applies to: 466-476
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.tsx` around lines 402 - 420, The current lookup for the managed hook (using rawSessionStart/sessionStart and existingIdx) doesn't validate each entry or hook shape; update the logic that derives sessionStart and the findIndex predicate to only consider well-formed managed hooks by: ensuring each sessionStart entry is an object (not null), has matcher === '' (exact empty string), has a hooks property that's an array, and within that array only treat hooks with type === 'command' and command === hookCommand as matches; adjust the existingIdx computation (and the analogous code around lines 466-476) to both guard against null/non-array values before calling .some/.findIndex and to require the exact matcher/type/command constraints when locating the managed hook.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/index.tsx`:
- Around line 443-448: Wrap the synchronous directory creation and file write
operations into a single guarded helper (e.g., ensureDirAndWriteSync or
safeWriteSettings) that takes settingsPath and contents, calls
fsSync.mkdirSync(path.dirname(settingsPath), { recursive: true }) and
fsSync.writeFileSync(settingsPath, ...), and catches any exceptions
(EACCES/EPERM/ENOSPC/etc.); on error, call the CLI logger/console.error with a
clear message including the path and error.message and exit with non-zero status
instead of letting the exception bubble. Replace the two existing write sites
that use fsSync.mkdirSync/fsSync.writeFileSync (the block around settingsPath
and the similar block at the other location) to call this helper so both
--install-hook / --remove-hook paths use the guarded writer.
- Around line 236-247: Reject combined hook and other top-level modes: instead
of short-circuiting, check that when options.installHook or options.removeHook
is set no other top-level action flags (e.g. options.apply, options.patches,
options.listPatches or any other mutually exclusive CLI mode flags used
elsewhere) are also present; if they are, print an error (similar to the
existing message) and exit non‑zero. Only call await
handleHookMode(!!options.installHook) and return when install/removeHook is the
sole top-level mode; otherwise fail fast. Use the existing symbols
options.installHook, options.removeHook and handleHookMode to locate and
implement the guard.
---
Duplicate comments:
In `@src/index.tsx`:
- Around line 402-420: The current lookup for the managed hook (using
rawSessionStart/sessionStart and existingIdx) doesn't validate each entry or
hook shape; update the logic that derives sessionStart and the findIndex
predicate to only consider well-formed managed hooks by: ensuring each
sessionStart entry is an object (not null), has matcher === '' (exact empty
string), has a hooks property that's an array, and within that array only treat
hooks with type === 'command' and command === hookCommand as matches; adjust the
existingIdx computation (and the analogous code around lines 466-476) to both
guard against null/non-array values before calling .some/.findIndex and to
require the exact matcher/type/command constraints when locating the managed
hook.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
Comment on lines
+236
to
+247
| // Handle --install-hook / --remove-hook flags | ||
| if (options.installHook && options.removeHook) { | ||
| console.error( | ||
| chalk.red( | ||
| 'Error: Cannot use --install-hook and --remove-hook together.' | ||
| ) | ||
| ); | ||
| process.exit(1); | ||
| } | ||
| if (options.installHook || options.removeHook) { | ||
| await handleHookMode(!!options.installHook); | ||
| return; |
Contributor
There was a problem hiding this comment.
Reject mixed command modes instead of silently picking one.
This branch short-circuits the rest of the default action, so --remove-hook --apply / --install-hook --patches ... return success after only managing the hook, while --install-hook --list-patches never reaches this block because list mode returned earlier. Please make hook management mutually exclusive with the other top-level modes instead of ignoring part of the CLI input.
💡 Proposed guard
if (options.installHook && options.removeHook) {
console.error(
chalk.red(
'Error: Cannot use --install-hook and --remove-hook together.'
)
);
process.exit(1);
}
+ if (
+ (options.installHook || options.removeHook) &&
+ (options.apply ||
+ options.restore ||
+ options.revert ||
+ options.listPatches ||
+ options.listSystemPrompts !== undefined ||
+ options.configUrl ||
+ options.patches)
+ ) {
+ console.error(
+ chalk.red(
+ 'Error: --install-hook/--remove-hook cannot be combined with other command modes.'
+ )
+ );
+ process.exit(1);
+ }
if (options.installHook || options.removeHook) {
await handleHookMode(!!options.installHook);
return;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.tsx` around lines 236 - 247, Reject combined hook and other
top-level modes: instead of short-circuiting, check that when
options.installHook or options.removeHook is set no other top-level action flags
(e.g. options.apply, options.patches, options.listPatches or any other mutually
exclusive CLI mode flags used elsewhere) are also present; if they are, print an
error (similar to the existing message) and exit non‑zero. Only call await
handleHookMode(!!options.installHook) and return when install/removeHook is the
sole top-level mode; otherwise fail fast. Use the existing symbols
options.installHook, options.removeHook and handleHookMode to locate and
implement the guard.
Comment on lines
+443
to
+448
| fsSync.mkdirSync(path.dirname(settingsPath), { recursive: true }); | ||
| fsSync.writeFileSync( | ||
| settingsPath, | ||
| JSON.stringify(settings, null, 2) + '\n', | ||
| 'utf8' | ||
| ); |
Contributor
There was a problem hiding this comment.
Handle write-side failures before exiting.
mkdirSync / writeFileSync can still fail on EACCES, EPERM, disk-full, or a read-only file. Right now that turns --install-hook / --remove-hook into an uncaught exception path. Please funnel both write sites through one guarded helper so the CLI prints a clear write error instead of a stack trace.
💡 Proposed helper
+function writeClaudeSettings(
+ settingsPath: string,
+ settings: Record<string, unknown>
+): void {
+ try {
+ fsSync.mkdirSync(path.dirname(settingsPath), { recursive: true });
+ fsSync.writeFileSync(
+ settingsPath,
+ JSON.stringify(settings, null, 2) + '\n',
+ 'utf8'
+ );
+ } catch (error) {
+ console.error(
+ chalk.red(
+ `Error writing ${settingsPath}: ${error instanceof Error ? error.message : String(error)}`
+ )
+ );
+ process.exit(1);
+ }
+}
+
...
- fsSync.mkdirSync(path.dirname(settingsPath), { recursive: true });
- fsSync.writeFileSync(
- settingsPath,
- JSON.stringify(settings, null, 2) + '\n',
- 'utf8'
- );
+ writeClaudeSettings(settingsPath, settings);
...
- fsSync.writeFileSync(
- settingsPath,
- JSON.stringify(settings, null, 2) + '\n',
- 'utf8'
- );
+ writeClaudeSettings(settingsPath, settings);Also applies to: 489-492
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/index.tsx` around lines 443 - 448, Wrap the synchronous directory
creation and file write operations into a single guarded helper (e.g.,
ensureDirAndWriteSync or safeWriteSettings) that takes settingsPath and
contents, calls fsSync.mkdirSync(path.dirname(settingsPath), { recursive: true
}) and fsSync.writeFileSync(settingsPath, ...), and catches any exceptions
(EACCES/EPERM/ENOSPC/etc.); on error, call the CLI logger/console.error with a
clear message including the path and error.message and exit with non-zero status
instead of letting the exception bubble. Replace the two existing write sites
that use fsSync.mkdirSync/fsSync.writeFileSync (the block around settingsPath
and the similar block at the other location) to call this helper so both
--install-hook / --remove-hook paths use the guarded writer.
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
Adds CLI options to install a Claude Code SessionStart hook that automatically reapplies tweakcc patches after CC auto-updates. Addresses #101.
--install-hook— installs hook in~/.claude/settings.json--remove-hook— removes the hook--quiet— suppresses output in--applymode (for hook usage)Motivation
Claude Code auto-updates replace the binary, wiping all tweakcc patches. Users must manually re-run
tweakcc --applyafter every update. This has been reported as the biggest pain point (#101) and the maintainer has expressed interest in solving it.The solution leverages Claude Code's own SessionStart hook system — a shell command that runs at the start of every CC session. The hook detects the installation and reapplies patches silently.
How it works
# One-time setup tweakcc --install-hookThis writes to
~/.claude/settings.json:{ "hooks": { "SessionStart": [{ "matcher": "", "hooks": [{ "type": "command", "command": "tweakcc --apply --quiet", "timeout": 30 }] }] } }On every CC session start:
tweakcc --apply --quiet--quietsuppresses all output so session startup isn't polluted# To remove tweakcc --remove-hookChanges
src/index.tsx--install-hook,--remove-hook,--quietoptions +handleHookMode()Commits
--install-hook,--remove-hook,--quietflags withhandleHookMode()--install-hook --remove-hook, validatehooks/SessionStartstructure withArray.isArrayguardscommand ===match instead ofincludes(), per-hook splice on remove instead of deleting the whole entryEdge cases handled
null,[]): Exits with descriptive error--install-hookis idempotent — reports "already installed"--remove-hookwhen no hook exists — reports "not found"--applyis deterministic (same config → same patches), so races are benigntweakcc(notnpx tweakcc) to avoid download latency. Requires global install (npm i -g tweakcc).Testing
pnpm lintpassespnpm run testpasses (221 total, 0 failures)pnpm prettier --check srcpassesRelates to: #101
Summary by CodeRabbit