Skip to content

CLI-244 CLI-245 callback infrastructure#157

Merged
kirill-knize-sonarsource merged 6 commits intomasterfrom
task/kk/CLI-244-245-callback-infrastructure
Apr 16, 2026
Merged

CLI-244 CLI-245 callback infrastructure#157
kirill-knize-sonarsource merged 6 commits intomasterfrom
task/kk/CLI-244-245-callback-infrastructure

Conversation

@kirill-knize-sonarsource
Copy link
Copy Markdown
Member

No description provided.

@hashicorp-vault-sonar-prod
Copy link
Copy Markdown

hashicorp-vault-sonar-prod Bot commented Apr 8, 2026

CLI-244
CLI-245

@sonar-review-alpha
Copy link
Copy Markdown
Contributor

sonar-review-alpha Bot commented Apr 8, 2026

Summary

Summary

This PR implements a callback infrastructure for the SonarQube CLI that moves hook business logic from bash/PowerShell templates into TypeScript command handlers. Instead of embedding complex shell logic in hook scripts, scripts now delegate to centralized sonar hook <event> commands.

Five new hook handlers are introduced:

  • claude-pre-tool-use — Scans files for secrets before Claude reads them, blocking reads if secrets detected
  • claude-prompt-submit — Scans user prompts for secrets before sending, blocking submission if secrets found
  • claude-post-tool-use — Runs SonarCloud analysis on files after Claude edits/writes them
  • git-pre-commit — Scans staged files for secrets before commit
  • git-pre-push — Scans files in commits about to be pushed for secrets

Supporting infrastructure:

  • Refactored analyze/secrets.ts to export reusable secret scanning functions (runSecretsBinary, runSecretsBinaryOnText) with proper timeout handling
  • New hook/ module with stdin readers (readGitPushRefs, readStdinJson) and shared auth/binary resolution logic
  • Simplified hook templates that now just pipe stdin to TypeScript handlers
  • All handlers gracefully exit if auth or binary path unavailable (non-blocking)

Architecture improvements:

  • Centralized, testable business logic instead of scattered shell scripts
  • Consistent error handling and timeout semantics across all hooks
  • Better maintainability and debugging experience

What reviewers should know

Review Guide

Start with: src/cli/command-tree.ts to see how hooks are registered as a hidden command group, then review each handler in src/cli/commands/hook/ to understand the flow.

Key design decisions:

  • Handlers read JSON payloads from stdin (piped by hook scripts) and validate presence of required fields before acting
  • All handlers are non-blocking by design: if auth is missing, binary isn't installed, or stdin is unparseable, they exit silently (exit 0) rather than blocking user operations
  • Secrets scanner is wrapped with timeout handling and child process cleanup to prevent hanging
  • Git file enumeration in git-pre-push.ts handles three cases: new branches (no remote), existing branches (diff against remote), and fallback (diff against empty tree)

Testing coverage:

  • Comprehensive unit tests for stdin parsing, timeout handling, git command execution
  • Integration tests verify each hook end-to-end with realistic payloads
  • CLAUDE.md policy reinforces preference for e2e tests (see line 54)

Things to watch:

  • Timeout values are consistent: 30s for secrets scan, 5s for stdin read
  • Exit code 51 signals secrets found (defined in original secrets command, now shared constant)
  • Hook handlers inherit auth resolution logic from existing auth-resolver.ts
  • Binary path resolution is non-downloading (no automatic installation in hooks)
  • Windows and Unix hook templates now minimal; all logic is TypeScript-based

Integration points:

  • Hooks integrate with existing SonarQubeClient for SQAA analysis (agent-post-tool-use)
  • Uses existing spawnProcess utility for child process management
  • Leverages existing error types (InvalidOptionError, CommandFailedError)

  • Generate Walkthrough
  • Generate Diagram

🗣️ Give feedback

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch 3 times, most recently from da371c5 to 91be621 Compare April 8, 2026 17:54
sonar-review-alpha[bot]

This comment was marked as resolved.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 91be621 to e5bc9f8 Compare April 8, 2026 18:37
sonar-review-alpha[bot]

This comment was marked as outdated.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from e5bc9f8 to 6f2f053 Compare April 8, 2026 19:17
@kirill-knize-sonarsource kirill-knize-sonarsource changed the base branch from master to fix/kk/coverage-phantom-lines April 8, 2026 19:18
sonar-review-alpha[bot]

This comment was marked as outdated.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 6f2f053 to aa6bc64 Compare April 8, 2026 19:58
sonar-review-alpha[bot]

This comment was marked as outdated.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the fix/kk/coverage-phantom-lines branch from b2c895c to b8e4288 Compare April 8, 2026 20:26
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from aa6bc64 to 3a31a77 Compare April 8, 2026 20:30
sonar-review-alpha[bot]

This comment was marked as resolved.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 3a31a77 to c4b4665 Compare April 8, 2026 21:01
sonar-review-alpha[bot]

This comment was marked as outdated.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the fix/kk/coverage-phantom-lines branch from b8e4288 to 26b6e97 Compare April 8, 2026 23:00
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch 2 times, most recently from e44c9d3 to d117a3a Compare April 8, 2026 23:34
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the fix/kk/coverage-phantom-lines branch from 26b6e97 to 74d9271 Compare April 8, 2026 23:39
sonar-review-alpha[bot]

This comment was marked as outdated.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I love the direction this is going 😍

I left a few comments & suggestions, mainly about duplication of secrets scanning logic. I will test it now.

Comment thread src/cli/commands/hook/claude-pre-tool-use.ts Outdated
Comment thread src/cli/commands/hook/secrets-scan.ts Outdated
Comment thread src/cli/commands/hook/secrets-scan.ts Outdated
Comment thread src/cli/commands/integrate/claude/hook-templates.ts Outdated
Comment thread src/cli/commands/integrate/claude/hook-templates.ts Outdated
Comment thread src/cli/command-tree.ts Outdated
Comment thread src/cli/command-tree.ts Outdated
Comment thread src/cli/command-tree.ts Outdated
Comment thread tests/integration/specs/hook/hook-claude-pre-tool-use.test.ts
Comment thread tests/integration/specs/hook/hook.test.ts
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the fix/kk/coverage-phantom-lines branch from 74d9271 to 8e2ad74 Compare April 9, 2026 09:44
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from d117a3a to 4b1d963 Compare April 9, 2026 09:45
sonar-review-alpha[bot]

This comment was marked as resolved.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 4b1d963 to fc175e7 Compare April 9, 2026 09:53
sonar-review-alpha[bot]

This comment was marked as outdated.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the fix/kk/coverage-phantom-lines branch from 8e2ad74 to b9df45c Compare April 9, 2026 22:42
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 468e45c to 38f15b5 Compare April 9, 2026 23:09
sonar-review-alpha[bot]

This comment was marked as outdated.

Base automatically changed from fix/kk/coverage-phantom-lines to master April 10, 2026 09:09
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 00173a3 to 9191141 Compare April 10, 2026 12:53
sonar-review-alpha[bot]

This comment was marked as outdated.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 9191141 to 129cc50 Compare April 13, 2026 09:34
sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 557751a to 5ca64ea Compare April 14, 2026 15:24
sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

sonar-review-alpha[bot]

This comment was marked as outdated.

PR feedback: rename symbols, remove codex stub, fix hook naming, update tests

- Rename scanFiles → scanFilesForSecrets in secrets-scan.ts
- Rename callbackCommand → hookCommand in command-tree.ts
- Remove codex-pre-tool-use command (out of scope)
- Remove PR-specific comments from command-tree.ts
- Rename agent-prompt-submit → claude-prompt-submit
- Rename agent-post-tool-use → claude-post-tool-use
- Consolidate duplicate hook.test.ts tests, assert description shown
- Update README.md via gen:docs

Improve coverage: unit tests for readStdinJson / readRawStdin
@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch 2 times, most recently from 25d692d to 585206d Compare April 16, 2026 11:41
sonar-review-alpha[bot]

This comment was marked as resolved.

@kirill-knize-sonarsource kirill-knize-sonarsource force-pushed the task/kk/CLI-244-245-callback-infrastructure branch from 585206d to 92f5752 Compare April 16, 2026 11:53
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Contributor

@sonar-review-alpha sonar-review-alpha Bot left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

Two of the three previously flagged issues remain open — the stale README entries and the readRawStdin timeout duplication. The git-pre-commit auth guard duplication has been fixed.

🗣️ Give feedback

@kirill-knize-sonarsource kirill-knize-sonarsource merged commit 0b74480 into master Apr 16, 2026
13 checks passed
@kirill-knize-sonarsource kirill-knize-sonarsource deleted the task/kk/CLI-244-245-callback-infrastructure branch April 16, 2026 12:00
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.

2 participants