Skip to content

CLI-815 Install Claude Code CAG hook#569

Draft
michael-jabbour-sonarsource wants to merge 15 commits into
masterfrom
mj/CLI-815_CAG_PostToolUse_hook
Draft

CLI-815 Install Claude Code CAG hook#569
michael-jabbour-sonarsource wants to merge 15 commits into
masterfrom
mj/CLI-815_CAG_PostToolUse_hook

Conversation

@michael-jabbour-sonarsource

@michael-jabbour-sonarsource michael-jabbour-sonarsource commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

Depends on SonarSource/sonar-context-augmentation#325

Forward PreToolUse, PostToolUse and PostToolUseFailure events to context augmentation, with patterns that match the following tools: Bash, PowerShell, Monitor and Read.


Summary by Gitar

  • New feature:
    • Implemented PostToolUse and PostToolUseFailure hooks for Claude Code.
  • Robustness:
    • Added idempotency checks to settings.json patching and ensured coexistence with existing SQAA hooks.
  • Tests:
    • Added integration tests covering hook installation, idempotency, and SQAA hook coexistence.
  • Documentation:
    • Updated CLAUDE.md to detail the new Claude Code hook implementation and lifecycle.

This will update automatically on new commits.

@michael-jabbour-sonarsource

Copy link
Copy Markdown
Contributor Author

This change is part of the following stack:

Change managed by git-spice.

@netlify

netlify Bot commented Jul 10, 2026

Copy link
Copy Markdown

Deploy Preview for sonarqube-cli canceled.

Name Link
🔨 Latest commit 3b50ca1
🔍 Latest deploy log https://app.netlify.com/projects/sonarqube-cli/deploys/6a60c7a71bd3ea000825154d

@hashicorp-vault-sonar-prod

hashicorp-vault-sonar-prod Bot commented Jul 10, 2026

Copy link
Copy Markdown

CLI-815

Comment thread src/cli/commands/integrate/claude/declaration.ts Outdated
Comment thread src/cli/commands/integrate/_common/features/context-augmentation-feature.ts Outdated
@gitar-bot

gitar-bot Bot commented Jul 13, 2026

Copy link
Copy Markdown

Confirmed — PowerShell and Monitor are documented Claude Code tool identifiers. Thanks for the clarification, no change needed here.

Avoid leaving Claude PostToolUse hook resources active when context augmentation setup aborts before the feature is recorded.
Make rollback of extra context augmentation resources best-effort so cleanup errors do not mask the original tool integrate failure.

Also, make log a warning if remove fails.
Install Claude PostToolUse and PostToolUseFailure CAG hooks through sonar context __hook Claude so CAG can dispatch on the hook event carried by the payload.
@michael-jabbour-sonarsource
michael-jabbour-sonarsource force-pushed the mj/CLI-815_CAG_PostToolUse_hook branch from 01768c9 to 9171f36 Compare July 13, 2026 17:22
When Claude CAG was already recorded, a retry could skip hook resources as already applied but still rerun tool integrate. If that operation failed, the rollback removed the existing hook script/settings while state continued to report the feature as installed.

Pass the owning integration id into the CAG feature and only roll back extra resources when the feature was not already recorded for the same integration, scope, and target root. First-install failures still clean up partial hook resources. Add regression coverage for a failing retry preserving the existing Claude CAG hooks.
Comment on lines 746 to +747
expectSkillFile(harness, CLAUDE_SKILL_PATH, false);
expectClaudeCagHookInstalled(harness);

@michael-jabbour-sonarsource michael-jabbour-sonarsource Jul 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While working on this, I realized that a failing call to runToolIntegrateCommand would currently keep the skill file and the hooks installed. I think this is a declarative framework issue, not only a Context Augmentation issue for the following reasons, so I decided to keep the fix out of scope for this PR.

Today IntegrationInstaller.applyFeatureWithUniqueDependencies() applies resources first, then operations, and the feature is only recorded in state after both phases complete. If an operation fails, any resources already written by that feature remain on disk, and the feature is not recorded (as shown by this test). On a later sonar integrate run, the framework treats the feature as not installed, so deselecting it only declines installation; it does not call removeFeature(). I think that makes the problem even worse, because it leaves the user with no CLI-driven cleanup path for those partial resources.

Note that this problem doesn't seem to impact only "Context Augmentation", but a couple of other cases as well:

  • Context Augmentation can leave generated skill resources if context-augmentation-tool-integrate fails. After this PR, we are leaving hooks as well.
  • Native Git global hooks can leave the generated hook file if configure-global-hooks-path fails.
  • Pre-commit framework integration can leave the .pre-commit-config.yaml patch if activate-hook fails.

A reasonable framework fix that I could think about would be to make a single feature application transactional around operation failures (i.e. rollback these resources in case of an operation fail):

  • The framework snapshots target file content before applying each resource and restore it if a later operation fails before rethrowing the error, or:
  • model partial installs in state so the CLI can offer cleanup on the next run.

Would any of these ideas make sense? Any thoughts are appreciated.

Comment thread src/commands/integrate/claude/hook-templates.ts Outdated
The hook is not about posttool only anymore
@gitar-bot

gitar-bot Bot commented Jul 22, 2026

Copy link
Copy Markdown
Code Review ✅ Approved 3 resolved / 3 findings

Implements Claude Code CAG hooks for lifecycle event tracking, while resolving template naming inconsistencies and improving error handling during rollback. No issues found.

✅ 3 resolved
Edge Case: Rollback aborts and masks original error if remove() throws

📄 src/cli/commands/integrate/_common/features/context-augmentation-feature.ts:87-92
In the new rollback loop, if any resource.remove(context) rejects (e.g. readJson throws CommandFailedError on invalid JSON in the target settings file, or an I/O error occurs), the loop stops immediately: the remaining extraResources are left un-rolled-back, and the removal error propagates instead of the original tool-integration error. This both leaves a partial install behind and hides the real failure cause from the user.

Consider wrapping each removal in a try/catch so every resource is attempted, and always re-throw the original error:

} catch (error) {
  for (const resource of [...extraResources].reverse()) {
    try {
      await resource.remove(context);
    } catch { /* best-effort rollback */ }
  }
  throw error;
}
Quality: CAG hook template named "PostTool" but used for Pre/PostFailure too

📄 src/commands/integrate/claude/hook-templates.ts:46-52 📄 src/commands/integrate/claude/declaration.ts:246-260
The getContextAugmentationPostToolTemplateUnix/Windows helpers are wired into PreToolUse, PostToolUse and PostToolUseFailure hook entries alike (declaration.ts:246-272), and the script simply delegates to sonar context __hook Claude for all three events. The PostTool name is misleading since the same launcher serves all three event types; consider renaming to something event-neutral (e.g. getContextAugmentationHookTemplate*) to avoid implying it is PostToolUse-specific. Purely a readability nit, no functional impact.

Quality: Verify PostToolUse matcher tool names (PowerShell/Monitor)

📄 src/cli/commands/integrate/claude/declaration.ts:72 📄 src/cli/commands/integrate/claude/declaration.ts:252
The new Context Augmentation PostToolUse hook uses matcher Bash|PowerShell|Monitor|Read (CONTEXT_POSTTOOL_MATCHER in declaration.ts). Other Claude hooks in this file match well-known Claude Code tool names (Read, Edit|Write). PowerShell and Monitor are not standard Claude Code tool names, so if they don't correspond to real tools the hook simply won't fire for those matcher segments. This won't cause an error, but it may silently reduce coverage of the hook. Please confirm these tool names are the intended/valid Claude Code tool identifiers. If they are placeholders or aspirational, consider trimming the matcher to the tools that actually exist so the configuration is not misleading.

Options

Auto-apply is off → Gitar will not commit updates to this branch.
Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Auto-apply Compact
gitar auto-apply:on         
gitar display:verbose         

Was this helpful? React with 👍 / 👎 | Gitar

@michael-jabbour-sonarsource michael-jabbour-sonarsource changed the title CLI-815 Install Claude Code CAG PostToolUse hook CLI-815 Install Claude Code CAG hook Jul 22, 2026
@sonarqubecloud

Copy link
Copy Markdown

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