fix(hooks): map prompt-submit to Copilot's userPromptSubmitted - #3030
Daniel Meppiel (danielmeppiel) merged 4 commits into
Conversation
The copilot target renamed UserPromptSubmit and userPromptSubmit to userPromptSubmit, which is not a Copilot CLI event, so prompt hooks never ran. Copilot's camelCase event is userPromptSubmitted. Map all three spellings to it and correct the tests that asserted the old name. Claude-Session: https://claude.ai/code/session_01AqoPHorErGdiZcExmLWBzp
@microsoft-github-policy-service agree |
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
User-facing hook documentation must be synchronized with the new Copilot mapping.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
What changed in this PR
Fixes Copilot prompt hooks by mapping prompt-submit aliases to userPromptSubmitted.
Changes:
- Updates Copilot event normalization.
- Adds unit and integration regression coverage.
- Adds a changelog entry.
| File | Description |
|---|---|
src/apm_cli/integration/hook_integrator.py |
Maps prompt aliases to Copilot’s valid event. |
tests/unit/integration/test_hook_event_normalization.py |
Covers the three aliases. |
tests/integration/test_hook_integrator_copilot_casing_e2e.py |
Verifies generated Copilot keys. |
CHANGELOG.md |
Records the bug fix. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
Thank you for contributing this pull request and for filing #3031. APM starts with an issue, not an implementation (https://github.com/microsoft/apm/blob/main/CONTRIBUTING.md). #3031 is not labelled CODEOWNERS for this change are Daniel Meppiel (@danielmeppiel) and Sergio Sisternes (@sergio-sisternes-epam). Existing review requests are unchanged. This comment is advisory classification only. It is not merge approval and it is not scope acceptance. Generated by autopilot-pr-triage-worker. This comment is AI-generated and may contain errors. |
|
Sergio Sisternes (@sergio-sisternes-epam) thanks so much for the feedback! I wasn't entirely sure on the PR process since last issue I submitted I was asked to submit a PR so I went ahead and did the work for that, but someone else resolved the issue in the meantime. Will follow your required process moving forward. |
|
Sheila Shahpari (@sheilagithub), thank you for the fix and the documentation follow-up. I have approved the bounded Copilot prompt-event correction on #3031: #3031 (comment). I am the review contact. We can review this existing PR; there is no need to restart the contribution. I am linking that approval in the PR body and clearing the scope-related deferred label. The review will be launched separately, and scope approval is not merge approval. For background, #2111 is the separate discussion of broader hook semantics. I am keeping that reference here rather than in the PR scope section so #3031 is the sole nominated scope issue. Reverse aliases for other targets, other hook events, and broader translation redesign remain out of scope. |
APM Review Panel:
|
| Persona | B | R | N | Takeaway |
|---|---|---|---|---|
| Python Architect | 0 | 0 | 0 | Copilot alias correction stays in the canonical event map; other targets and integration structure are unchanged. |
| Devx Ux Expert | 0 | 0 | 0 | Prompt-submit mapping fix resolves a consumer-facing silent-failure bug; docs, tests, and changelog are clear and in sync. |
| Doc Writer | 0 | 0 | 0 | Both alias tables and the changelog accurately describe the Copilot-only fix and unchanged Claude limitation. The prior docs feedback is addressed. |
| Test Coverage Expert | 0 | 0 | 0 | Three aliases have unit coverage; real-file deployment catches the wrong Copilot key and retains the Claude control. Focused checks passed. |
B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.
Architecture
classDiagram
class BaseIntegrator
class HookIntegrator
class _MergeHookConfig
BaseIntegrator <|-- HookIntegrator
HookIntegrator ..> _MergeHookConfig : uses
flowchart LR
A[Package hook JSON] --> B[HookIntegrator]
B --> C[Copilot event map]
C --> D[userPromptSubmitted]
D --> E[Generated .github/hooks JSON]
Recommendation
The panel found no issues across architecture, DevX, documentation, and test coverage. The fix is a clean, bounded lookup-table correction with aligned docs, changelog, and regression tests. Sheila Shahpari (@sheilagithub)'s contribution is well-scoped and well-tested within the stated verification envelope. CODEOWNERS danielmeppiel and Sergio Sisternes (@sergio-sisternes-epam) can review with confidence, bearing in mind that full-suite CI, Windows, and Copilot runtime reproduction remain outside the panel's independent verification. No follow-ups are recommended.
Full per-persona findings
Python Architect
No findings.
Devx Ux Expert
No findings.
Doc Writer
No findings.
Test Coverage Expert
No findings.
This panel is advisory. It does not block merge. Re-apply the
panel-review label after addressing feedback to re-run.
Generated by autopilot-pr-review-worker. This comment is AI-generated and may contain errors.
4631342
into
microsoft:main
|
When are you planning to release this? Sergio Sisternes (@sergio-sisternes-epam)? |

Description
For the copilot target, apm renames
UserPromptSubmitanduserPromptSubmitto
userPromptSubmit. Copilot CLI has no event by that name, so prompt hooksinstalled for Copilot never run, and Copilot logs nothing.
Copilot's hooks reference
(https://docs.github.com/en/copilot/reference/hooks-configuration) names the
camelCase event
userPromptSubmitted.UserPromptSubmitis only itsVS Code-compatible PascalCase variant, which switches the payload to
snake_case. The copilot target writes camelCase, so
userPromptSubmittedisthe right name.
Changes:
hook_integrator.py:UserPromptSubmit,userPromptSubmitanduserPromptSubmittednow map touserPromptSubmittedfor copilot.test_hook_event_normalization.py: three alias cases.test_hook_integrator_copilot_casing_e2e.py: expectsuserPromptSubmittedand asserts
userPromptSubmitis no longer written. It previously assertedthe wrong name.
CHANGELOG.md: oneFixedentry.Reproduction (apm 0.31.0, Copilot CLI 1.0.86, Windows 11):
target: [copilot]and.apm/hooks/x.json:{"hooks":{"UserPromptSubmit":[{"hooks":[{"type":"command","command":"echo hit >> hook.log"}]}]}}target: [copilot], thenapm install..github/hooks/pkg-x.jsonholds"userPromptSubmit".copilotand send a prompt: nohook.logis written."userPromptSubmitted", restart and send a prompt:hook.logcontainshit.Issue and approved scope
Fixes #3031
Human scope-approval comment: approved scope and review contact.
The reverse direction is out of scope: a Copilot-authored
userPromptSubmittedhook installed for claude, codex or kiro still passesthrough unrenamed. I can open a separate issue for that if it's wanted.
Broader hook-semantics and translation redesign remain out of scope.
Background context is preserved in the maintainer handoff.
Type of change
Testing
main(4 failed)and pass with the fix. The 6 hook test files pass (516 tests).
tests/unitgives 443 failed and 44 errors. Those files also fail on unmodified
main, apart from a few auth/network tests that flip between runs.None of the failures is in a hook test. Relying on CI for the full suite.
above never fires. With this branch,
apm installwrites"userPromptSubmitted"to.github/hooks/pkg-x.json, and the firstprompt writes
hittohook.logwith no hand edits.Spec conformance (OpenAPM v0.1)