Skip to content

test(hooks): align cursor smoke expectations with native flat handlers - #3198

Merged
Daniel Meppiel (danielmeppiel) merged 4 commits into
microsoft:mainfrom
MohammedAlkindi:fix/cursor-flat-hooks-smoke
Oct 19, 2026
Merged

Daniel Meppiel (danielmeppiel) merged 4 commits into
microsoft:mainfrom
MohammedAlkindi:fix/cursor-flat-hooks-smoke

Conversation

@MohammedAlkindi

@MohammedAlkindi Mohammed Alkindi (MohammedAlkindi) commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Description

Retain the useful Cursor test assertions after #3251 fixed the basic smoke failures on main. This tests-only delta uses the native preToolUse event in the audit fixture, checks the exact incomplete pointer and inventory, and verifies that reinstall preserves the entire inserted user entry, not just its command.

The two affected functions also carry the existing lifecycle_smoke marker so the PR-time lifecycle selector executes all nine parameterized cases.

The existing native flat-handler fixture (including matcher: Shell), other targets, and dated release changelog remain unchanged. Original commits by Mohammed Alkindi (@MohammedAlkindi) are preserved; main 65b3b9f4172feaae5be9118e9a8b672f7a527481 is integrated.

Trade-off: this strengthens regression detection without changing product hook rendering or audit behavior. Documentation impact is no_change from the docs-index L0 tests-only path rule. No runtime control flow changed, so no architecture diagram is needed.

Issue and approved scope

Issue: #3259

Human scope-approval comment: #3259 (comment)

Does this PR complete the issue, or what remains? The two approved test files cover the residual assertions, and all observed technical workflows succeeded at the published head. The subsequent daily scheduled smoke run remains ordinary post-landing observation; this PR does not claim that a future scheduled run has passed. The original bot-filed smoke report is historical context, not an additional scope request.

Type of change

  • Bug fix
  • New feature
  • Documentation
  • Maintenance / refactor

Testing

  • Tested locally
  • All existing tests pass
  • Added tests for new functionality (if applicable)

Validation on c1c64fe6c5bfccf8c60f3833a04b2ba97c28b00c:

Command Result
APM_BINARY_PATH="$PWD/.venv/bin/apm" uv run --extra dev pytest -q tests/integration/test_hook_root_source_drift_e2e.py tests/integration/test_audit_primitive_contract.py --basetemp=.pytest-artifacts/final-head -p no:cacheprovider 102 passed in 99.14s; installed Python CLI, not a frozen binary
APM_E2E_TESTS=1 APM_BINARY_PATH="$PWD/.venv/bin/apm" uv run --extra dev pytest -q -p no:cacheprovider --basetemp=.pytest-artifacts/ci-selection -n 4 --dist loadgroup -m 'lifecycle_smoke and not lifecycle_merge_group' tests/integration/test_hook_root_source_drift_e2e.py tests/integration/test_audit_primitive_contract.py 9 passed in 38.32s locally; these functions selected zero cases before the two markers
uv run --extra dev ruff check src/ tests/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py Passed
uv run --extra dev ruff format --check src/ tests/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py Passed; 1958 files formatted
uv run --extra dev python -m pylint --disable=all --enable=R0801 --min-similarity-lines=10 --fail-on=R0801 src/apm_cli/ scripts/lint_architecture_boundaries.py scripts/architecture_linter/ scripts/windows_native_symlink_probe_entry.py Passed
bash scripts/lint-auth-signals.sh Passed
bash scripts/lint-architecture-boundaries.sh Passed
uv run --frozen python scripts/check_test_assertions.py Passed
uv run --frozen python scripts/check_exact_test_duplicates.py Passed

The YAML I/O, 2100-line limit, and portable-path guards also passed using equivalent Python regex checks because macOS grep lacks -P.

Four isolated source-copy mutations failed as expected: removing the flat-handler rejection; changing the diagnostic pointer; adding a spurious complete inventory entry; dropping the preserved user handler's type. Production files were not modified. The full repository test suite was not run locally.

Actual technical workflows succeeded at the published head: CI (attempt 2), CodeQL, NOTICE, and Merge Gate. CI includes successful Windows Git Discovery on Python 3.10 and 3.11. Its Lifecycle Smoke job executed the required marker selector: 276 passed, 1 skipped in 328.27s. The remote log reports aggregate results; the nine individual selected node IDs were verified locally.

Fresh-main compatibility was checked separately after main advanced to 4d35e74f1801b1ed2424569f55c927da0d18eccd. git -c merge.union.driver='git merge-file -p %A %O %B' merge-tree --write-tree c1c64fe6c5bfccf8c60f3833a04b2ba97c28b00c 4d35e74f1801b1ed2424569f55c927da0d18eccd passed without conflicts. The prospective tree preserves current-main source and changelog and the exact published test changes. Running python -m pytest -q tests/integration/test_hook_root_source_drift_e2e.py tests/integration/test_audit_primitive_contract.py tests/unit/integration/test_hook_native_metadata.py tests/integration/test_hook_nested_metadata_install.py --basetemp=../tests -p no:cacheprovider against that tree, with its src on PYTHONPATH and the existing installed CLI, passed 114 tests in 76.29s. This is local prospective-merge evidence, not CI on an unpublished commit.

To reproduce locally, create .pytest-artifacts/runtime, export TMPDIR="$PWD/.pytest-artifacts/runtime", and run the pytest command above after installing the development environment with uv.

Spec conformance (OpenAPM v0.1)

If this PR changes behaviour that an OpenAPM v0.1 req-XXX covers,
confirm the three-step ritual in the
development guide:

  • Spec edit: docs/src/content/docs/specs/openapm-v0.1.md updated
    (new/changed <a id="req-XXX"></a> anchor + prose + Appendix C
    row).
  • Manifest edit: docs/src/content/docs/specs/manifests/openapm-v0.1.requirements.yml
    updated.
  • Test edit: a @pytest.mark.req("req-XXX") test under
    tests/spec_conformance/ added or extended.
  • CONFORMANCE.{md,json} regenerated via
    uv run --extra dev python -m tests.spec_conformance.gen_statement
    and committed.
  • N/A -- this PR does not change OpenAPM-observable behaviour.

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

microsoft#3149 renders Cursor hooks under camelCase native events with flat
handlers, but two integration tests still expected the old shape and
have kept the scheduled smoke red since:

- the root source drift test read the ownership sidecar under
  PreToolUse instead of preToolUse, and seeded a nested user hook that
  Cursor's strict validator now rejects on reinstall;
- the audit contract test expected a nested group in .cursor/hooks.json
  to be silently not-applicable, while audit now reports it as
  incomplete coverage because Cursor does not load nested handlers.

Refs microsoft#3166

Copilot AI left a comment

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.

🟡 Changes recommended

The updated tests can miss user-hook mutation and still use a non-native Cursor event in the audit fixture.

2 open findings
What changed in this PR

Updates Cursor integration tests to match native camel-case events and flat handlers.

Changes:

  • Uses preToolUse and flat user hooks for Cursor drift tests.
  • Updates audit expectations for nested Cursor handlers.
File Description
tests/​integration/​test_hook_root_source_drift_e2e.py Aligns Cursor drift fixtures with flat handlers.
tests/​integration/​test_audit_primitive_contract.py Expects nested Cursor handlers to be incomplete.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread tests/integration/test_audit_primitive_contract.py Outdated
Comment thread tests/integration/test_hook_root_source_drift_e2e.py Outdated

Copilot AI commented Oct 9, 2026

Copy link
Copy Markdown
Contributor

One or more custom setup steps configured for this repository failed during this Copilot code review run:

Sync dependencies with uv

Setup steps run before each review. If the review above is missing context, or no review was posted at all, the failing step above may be the cause. See the workflow run for failure details, fix your setup steps configuration, and re-request a review.

Note

You can configure setup steps for Copilot code review separately from Copilot cloud agent with a copilot-code-review.yml file. Read the docs for details.

@sergio-sisternes-epam Sergio Sisternes (sergio-sisternes-epam) added the panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. label Oct 16, 2026
@github-actions

Copy link
Copy Markdown

APM Review Panel: ship_with_followups

Tests-only: Cursor hook expectations aligned with #3149 flat preToolUse handlers -- clean panel, ship after rebase

panel-mode=lean; personas=test-coverage-expert,apm-ceo

cc Mohammed Alkindi (@MohammedAlkindi) @danielmeppiel @sergio-sisternes-epam -- a fresh advisory pass is ready for your review.

Single-panelist lean review with no findings and no dissent. The test-coverage-expert confirmed the two Cursor integration tests assert the flat preToolUse handler shape and the incomplete diagnostic for nested Cursor groups introduced by #3149. No src/ changes, so no behavioral regression risk. The PR reports mergeable_state dirty, so a rebase onto main is needed. Both prior Copilot inline threads (event name per target; exact user_entry comparison) are outdated and addressed by the current head. Scope was approved on #3259.

Panel summary

Persona B R N Takeaway
Test Coverage Expert 0 0 0 Aligns cursor expectations with #3149 flat handlers; no coverage gaps.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Top 1 follow-ups

  1. [Apm Ceo] Rebase onto main to resolve conflicts and confirm CI green on both [cursor] cases -- mergeable_state is dirty; CI cannot pass on the current head until conflicts are resolved.

Recommendation

Clean tests-only change with pre-approved scope. Ship once rebased onto main and CI confirms green on the two Cursor hook cases. No rework needed.


Full per-persona 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 PR Review Panel for #3198 · copilot · auto · 76.3 AIC · ⌖ 0.536 AIC · ⊞ 10.1K · ◷

@github-actions github-actions Bot removed the panel-review Request an advisory PR review; consumed after review. Not human approval or a merge gate. label Oct 16, 2026
Merge main after microsoft#3251 while preserving the native Cursor event, incomplete pointer and inventory assertions, and exact user-entry preservation from microsoft#3198. Addresses Copilot inline findings and the prior panel integration follow-up within approved issue microsoft#3259; production and dated release files match main.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
@danielmeppiel

Daniel Meppiel (danielmeppiel) commented Oct 19, 2026 •

Copy link
Copy Markdown
Collaborator

APM Review Panel: ship_now

Useful tests-only residuals remain after the mainline smoke repair: precise native Cursor audit assertions and exact user-entry preservation.

panel-mode=delta; terminal-personas=apm-ceo; carried-review=test-coverage-expert

cc Mohammed Alkindi (@MohammedAlkindi) Daniel Meppiel (@danielmeppiel) Sergio Sisternes (@sergio-sisternes-epam) -- a fresh advisory pass is ready for your review. Existing requested reviewers remain unchanged.

The actual terminal CEO recommends ship_now for published head c1c64fe6c5bfccf8c60f3833a04b2ba97c28b00c. The two affected specialist passes returned no findings. The original contributor commits remain ancestors; no product code was changed by this PR. Native Cursor preToolUse, the exact incomplete pointer/inventory, and complete equality of the inserted user entry are retained. The existing lifecycle_smoke marker now selects both affected functions in PR-time CI.

All observed technical workflows succeeded at that exact head: CI, CodeQL, NOTICE, and the convergence workflow. CI includes both Windows Git Discovery versions and Lifecycle Smoke: 276 passed, 1 skipped in 328.27s. The remote output is aggregate; the nine individual selected cases were verified locally. Historical Windows failures were HTTP/2 dependency-download errors before test execution, not assertion failures.

Local exact-head evidence: 102 affected tests passed in 99.14s; the matching CI selector passed all nine cases in 38.32s. Four isolated source-copy mutations each failed the intended assertion. These mutations establish assertion sensitivity, while the selector run establishes marker coverage. Canonical lint, architecture boundaries, and test ratchets passed. Owner-evidence v2 schema and terminal semantic verification passed against the integrated baseline; no canonical owners are touched. Functional mappings remain separate from the empty owner-test list. Docs impact is no_change under the tests-only L0 rule.

Main subsequently advanced to 4d35e74f1801b1ed2424569f55c927da0d18eccd. A strict merge-tree check, overriding union resolution, succeeded. Its prospective tree preserves current-main source and dated changelog and the exact published test residuals. Both affected suites plus the newly landed nested-metadata tests passed there: 114 tests in 76.29s. This is separate local prospective-merge evidence, not a claim of remote CI on an unpublished revision.

Panel summary

Persona B R N Takeaway
Test Coverage Expert (carried actual delta reviews) 0 0 0 Precise assertions and the narrow marker additions have passing evidence.

B = blocking-severity findings, R = recommended, N = nits.
Counts are signal strength, not gates. The maintainer ships.

Folded in this run

  • cd723e40: integrated main's existing smoke repair and retained only meaningful residual assertions in the two approved test files.
  • c1c64fe6: added two existing lifecycle markers, eliminating the zero-selected-case gap without workflow or production changes.

Copilot signals reviewed

  • 4232935679 - LEGIT: native Cursor event and precise audit assertions. Addressed in the contributor head, preserved here, and the inline thread is replied and resolved.
  • 4232935770 - LEGIT: complete inserted-entry equality rather than command-only matching. Addressed in the contributor head, preserved here, and the inline thread is replied and resolved.

Recommendation

The actual terminal CEO recommends ship_now; no in-scope implementation work remains. The next daily scheduled smoke run is routine post-landing observation and has not yet been observed. The parent maintainer independently verifies and serializes landing.

PR Head Recommendation Iterations Folds Deferrals Copilot rounds CI Mergeable State Note
#3198 c1c64fe ship_now 2 3 0 2 green MERGEABLE BLOCKED awaiting maintainer
Review provenance

The earlier genuine lean panel was inspected in run 38965811570; its main-integration, CI, and thread-housekeeping follow-ups are satisfied. Two affected test-coverage returns were carried into the actual terminal CEO synthesis rather than repeating an unrelated broad panel. Raw reviewer terminology was checked against execution evidence: local selected-node proof is not presented as remote per-node output, and mutation failures prove assertions rather than scheduling metadata. The owner's deterministic comparison uses integrated baseline 65b3b9f4 and published head; the fresh-main strict tree is separately verified.

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-merge-worker. This comment is AI-generated and may contain errors.

The required PR lifecycle selector deselected all 102 tests in these files, so the approved Cursor cases had only local and post-merge coverage. Mark only the two affected functions using the existing lifecycle_smoke scheduler; this adds nine hermetic cases without workflow or product changes. Completes the CI evidence requirement of microsoft#3259.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: f79e464c-180e-4be3-9056-0773aa679a1a
@danielmeppiel
Daniel Meppiel (danielmeppiel) merged commit b263264 into microsoft:main Oct 19, 2026
21 checks passed
@MohammedAlkindi
Mohammed Alkindi (MohammedAlkindi) deleted the fix/cursor-flat-hooks-smoke branch October 20, 2026 00:10
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.

4 participants