feat: surface installed hook actions during apm install#409
feat: surface installed hook actions during apm install#409harshitlarl wants to merge 3 commits intomicrosoft:mainfrom
Conversation
|
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Pull request overview
Adds install-time transparency for hook packages so developers can see what hook actions will run and (in verbose mode) inspect the deployed hook JSON content, addressing supply-chain visibility concerns from #316.
Changes:
- Emit per-event hook action summaries during
apm installfor integrated hooks. - In
--verbosemode, print the fully rewritten hook JSON that will be deployed/merged. - Add focused unit tests covering the new hook transparency output.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/apm_cli/integration/hook_integrator.py |
Adds hook action flattening/summarization and returns CLI display payloads alongside integration results. |
src/apm_cli/commands/install.py |
Logs hook action summaries and verbose hook JSON content during install. |
tests/unit/test_install_hook_transparency.py |
New unit tests validating the new install-time hook transparency output (normal + verbose). |
| return { | ||
| "target_label": target_label, | ||
| "output_path": output_path, | ||
| "source_hook_file": source_hook_file.name, | ||
| "actions": actions, | ||
| "rendered_json": json.dumps(rewritten, indent=2, sort_keys=True), | ||
| } |
There was a problem hiding this comment.
rendered_json is precomputed for every integrated hook via json.dumps(...), even when apm install is not in --verbose mode. This adds avoidable work/memory on the hot install path. Consider deferring JSON rendering until verbose logging is actually requested (e.g., store the rewritten dict, or store a callable/None and render in the logger code only when needed).
| "output_path": output_path, | ||
| "source_hook_file": source_hook_file.name, | ||
| "actions": actions, | ||
| "rendered_json": json.dumps(rewritten, indent=2, sort_keys=True), |
There was a problem hiding this comment.
The verbose display payload uses json.dumps(..., sort_keys=True), but the deployed JSON files are written with json.dump(..., indent=2) (no sort_keys). This means --verbose will show a key order that can differ from what was actually deployed, which undermines the goal of letting developers review the exact hook content. Align the verbose rendering with the on-disk serialization (or reuse the same serialization helper for both).
| "rendered_json": json.dumps(rewritten, indent=2, sort_keys=True), | |
| "rendered_json": json.dumps(rewritten, indent=2), |
| display_payloads.append( | ||
| self._build_display_payload( | ||
| ".github/hooks/", | ||
| target_filename, |
There was a problem hiding this comment.
For VSCode/GitHub hooks, the display payload sets output_path to target_filename only. This makes the verbose line render as hooks.json -> <filename> without the .github/hooks/ prefix, unlike the Claude/Cursor cases that include the full target path. Consider storing the full relative destination path (e.g. .github/hooks/<filename>) or using target_label when formatting the destination so the output is unambiguous.
| target_filename, | |
| rel_path, |
| if logger.verbose: | ||
| logger.verbose_detail( | ||
| f" Hook JSON ({source_name} -> {payload['output_path']}):" | ||
| ) | ||
| for line in payload["rendered_json"].splitlines(): | ||
| logger.verbose_detail(f" {line}") |
There was a problem hiding this comment.
The new verbose hook transparency (Hook JSON ... + full rewritten JSON) changes what --verbose emits during apm install, but the CLI docs currently describe verbose mode only in terms of file paths + diagnostic details. Please update the Starlight docs (e.g. docs/src/content/docs/reference/cli-commands.md under apm install) to mention that verbose mode also prints the full rewritten hook JSON so users can review deployed hook content.
Summary
Testing
Closes #316