feat: surface installed hook actions during apm install (closes #316)#1700
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds install-time transparency for integrated hook primitives, surfacing per-event hook action summaries during apm install and (in verbose mode) emitting the rewritten hook JSON content that will be deployed/merged. It also introduces unit tests that lock in the security contract that the displayed hook payloads reflect the post-path-rewrite (on-disk / executed) content.
Changes:
- Add hook display metadata generation in
HookIntegrator(flatten hook entries + summarize commands + capture rewritten JSON). - Thread
display_payloadsthroughIntegrationResultand install output aggregation to emit per-hook-file action summaries (and verbose JSON detail). - Add unit tests to ensure
display_payloads.rendered_jsonmatches the rewritten content written to disk / merged into config.
Show a summary per file
| File | Description |
|---|---|
tests/unit/test_install_hook_transparency.py |
Adds unit tests for hook entry flattening, command summarization, and the “display matches rewritten/on-disk content” security invariant. |
src/apm_cli/integration/hook_integrator.py |
Builds per-hook-file display payloads from rewritten hook data and returns them via integration results. |
src/apm_cli/integration/base_integrator.py |
Extends IntegrationResult with a display_payloads field to carry hook transparency metadata. |
src/apm_cli/install/services.py |
Aggregates and emits hook transparency output during install, including verbose-mode rewritten JSON printing. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 1
Comment on lines
+425
to
+437
| command = "" | ||
| for key in ("command", "bash", "powershell"): | ||
| value = entry.get(key) | ||
| if isinstance(value, str) and value.strip(): | ||
| command = value.strip() | ||
| break | ||
| if not command: | ||
| return "runs hook command" | ||
| for token in command.split(): | ||
| cleaned = token.strip("\"'") | ||
| if "/" in cleaned or cleaned.startswith("."): | ||
| return f"runs {cleaned}" | ||
| return f"runs {command}" |
Collaborator
Author
Shepherd-driver completion advisoryPR #1700 supersedes #409 (@harshitlarl). Authorship preserved in git history. Convergence summaryRebase (origin/main rebase of feat/hook-install-transparency-316):
Architecture adaptation (main refactored install.py -> services.py + dispatch table):
Security invariant satisfied: display_payloads uses the 'rewritten' dict Folded items
Deferred items
CI evidenceAll checks green on final commit (1b595b4):
Lint evidenceruff check + format: silent | pylint R0801: 10.00/10 | auth-signals: clean Tests12 new hook transparency tests pass (including security assertion: display_payloads == on-disk content) Copilot reviewCopilot left 1 review summary on #409 (no inline comments accessible). Classified: reviewed and no actionable inline items. Mergeabilitymergeable: MERGEABLE | mergeStateStatus: BLOCKED (awaiting review approval -- branch protection) |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…yloads - Rebase feat/hook-install-transparency-316 onto current main - Adapt hook transparency from old monolithic install.py to refactored services.py + dispatch table architecture - Add display_payloads field to IntegrationResult (base_integrator.py) so all integrator results carry the field; HookIntegrator populates it - Add _iter_hook_entries, _summarize_command, _build_display_payload to HookIntegrator; _build_display_payload takes the post-path-rewrite 'rewritten' dict so display_payloads faithfully reflects what is actually written to disk and executed (security requirement from #316) - Extract _log_hook_display_payloads helper in services.py to keep integrate_package_primitives below C901 complexity threshold - Rewrite test suite to assert display_payloads == on-disk content (the security-critical invariant the panel prior raised) Co-authored-by: harshitlarl <zipdemonharshits012@gmail.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Fold panel advisory: _log_hook_display_payloads used bare dict key access (_act['event'], _act['summary']) which would throw KeyError on a malformed payload. Switch to .get() with fallback '?' to align with the defensive style used on adjacent lines. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The hook transparency summary built display payloads from the pre-finalization per-file `rewritten` dict, which diverged from what is actually written and executed: for Claude (schema-strict) the rendered JSON carried `_apm_source` keys stripped before the disk write, and for Gemini it showed the pre-transform Copilot schema instead of the bash->command / timeoutSec->timeout(ms) rewrite that lands on disk. Additionally `_iter_hook_entries` and `_summarize_command` only inspected 3 of the 6 HOOK_COMMAND_KEYS, so hooks using `windows`/`linux`/`osx` keys deployed with zero transparency output -- the precise failure the feature was built to prevent. `_summarize_command` could also emit multi-line summaries when a command contained a newline, breaking log formatting and enabling log-spoofing. Defer display-payload construction in the merged-target path until the JSON config is finalized (Gemini transform applied, schema-strict `_apm_source` stripped) and build from the same entry objects written to disk; iterate the full HOOK_COMMAND_KEYS tuple; and collapse internal whitespace so summaries are always single-line. addresses CEO follow-ups (Claude/Gemini rendered_json divergence, 3-of-6 HOOK_COMMAND_KEYS blindspot) and Copilot inline on src/apm_cli/integration/hook_integrator.py:437 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Add regression traps for the four folded fixes: OS-specific hook keys (windows/linux/osx) each surface an action; command summaries collapse embedded newlines/whitespace to a single line; Claude rendered_json omits the `_apm_source` key stripped from disk; and Gemini rendered_json reflects the bash->command / timeoutSec->timeout(ms) transform actually written to settings.json. Also cover the user-visible `_log_hook_display_payloads` install logger, which had no direct test. Each trap was proven by the mutation-break gate: with the production guards reverted, all four divergence tests fail. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Record the install-time hook action transparency feature under Unreleased > Added, crediting the original community author. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
force-pushed
the
feat/hook-install-transparency-316-rebased
branch
from
387386b to
20f56df
Compare
Collaborator
Author
apm-review-panel advisory (shepherd-driver terminal pass)The panel's three blocking findings and the test gap were all folded into this PR; CI is green on the rebased head. Recommending the maintainer take this through required review. Folded in this run
Copilot signals reviewed
Regression-trap evidence (mutation-break gate)
Lint contract
CIAll required checks green on Conflict resolutionRebased onto current main (
Pushed with Mergeability status
Convergence1 outer iteration; 1 Copilot round. |
sergio-sisternes-epam
pushed a commit
that referenced
this pull request
Jun 9, 2026
…arency) Sync main commits #1700, #1676, #1694, #1710. The #1700 feature (surface installed hook actions during install) wove display-payload tracking through code my refactor extracted into sibling helpers; resolve two conflicts by keeping the extracted structure and porting #1700 semantics: - services.py: accept the extracted _log_per_kind_results() call; move _log_hook_display_payloads into services_integrate.py (avoids circular import) and re-export it from services.py; emit hook summaries inside _log_per_kind_results. - hook_integrator._integrate_merged_hooks: accept extracted _merge_hook_file_entries / _write_merged_config; thread per-file display data out via a new optional capture_entries kwarg on _merge_hook_file_entries; build display payloads after _write_merged_config finalizes (post _apm_source strip). - Keep hook_integrator.py <=800 by relocating _iter_hook_entries / _summarize_command / _build_display_payload to hook_transforms.py and _parse_hook_json to hook_merge.py as thin delegators. Shadow gate green: ruff, ruff format, pylint R0801 10.00/10 (EXIT=0), auth-signals, import smoke, 3178 tests pass. All files within 800-line guard (hook_integrator.py 799, hook_merge.py 766, hook_transforms.py 601, services.py 690, services_integrate.py 307). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Merged
sergio-sisternes-epam
pushed a commit
that referenced
this pull request
Jun 9, 2026
…e integrity) Sync with main through v0.19.0 (#1715, #1714, #1718, #1719, #1717). Conflicts resolved: - CHANGELOG.md: main cut the v0.19.0 release, moving prior Unreleased entries into a dated section. Took main's restructure; relocated the #1681 "Tightened Stage 2 code-complexity thresholds" entry to the new ## [Unreleased] section (released sections stay immutable per Keep a Changelog). Took main's #1702/#1710 close-ref for the lockfile entry. - src/apm_cli/install/services.py: #1718 added per-file skill-bundle integrity recording inline at a site my refactor had extracted into _log_skill_result. Kept the new _skill_bundle_file_entries helper in services.py (sibling of _deployed_path_entry); dropped main's duplicate inline _log_hook_display_payloads (already relocated to services_integrate.py and re-exported in my #1700 port); accepted the _log_skill_result delegator at the call site. - src/apm_cli/install/services_integrate.py: ported #1718's deployed.extend(_skill_bundle_file_entries(tp, ...)) into _log_skill_result's deployed-path loop (deployed aliases result["deployed_files"]), with a lazy import of the helper. Shadow gate: no markers, all src <=800 lines, ruff check + format clean, pylint R0801 EXIT=0, auth-signals clean; 11144 tests pass across the install/integration suites plus the #1718, #1700, and #1709 targeted sets. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
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
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
This PR supersedes #409 by @harshitlarl (rebased to resolve conflicts with current main).
Authorship preserved: harshitlarl's original commit (8783a21) is in the git history as author.
What this PR does
Security contract
_build_display_payload takes the post-path-rewrite 'rewritten' dict so
display_payloads faithfully reflects what is written to disk and executed.
Closes #316
Co-authored-by: harshitlarl zipdemonharshits012@gmail.com