Exclude the Cursor telemetry spawn test from the whole assembly

[realtonyyoung]

Closes #720 — AI-2387

What & why

telemetry_hook_does_not_recovery_spawn_while_an_earlier_canonical_event_is_still_stuck installs WatcherManager.SpawnOverrideForTesting, a process-wide static, while its class carries only the keyed [NotInParallel("VendorEnvOverrides")] — a cohort of env-override readers, not of every test that can trigger a watcher spawn. A concurrent peer's spawn therefore lands in this test's spawned list and the emptiness assertion fails. Bare [NotInParallel] on the method makes it assembly-exclusive, matching what every other override mutator already carries.

Where to look

The method-level attribute shadows the class-level key rather than adding to it. That loses nothing: assembly-exclusive is strictly stronger than the env-override cohort the class key buys.

Verification

The red run's evidence is the key it saw — 9dc2775376454e4691ecc2d69973c152 is ClaudeHookCommandTests.Sid, a constant this test (fresh Guid.NewGuid()) cannot produce. Full CLI unit suite on this branch: 3857 total, 0 failed. Test-only change, so AOT publish is untouched.

🤖 Generated with Claude Code

[qodo-code-review]

PR Summary by Qodo

Make Cursor telemetry spawn test assembly-exclusive

🐞 Bug fix 🧪 Tests 🕐 Less than 5 minutes

AI Description

• Runs the Cursor telemetry spawn test exclusively across the unit-test assembly.
• Prevents concurrent watcher spawns from contaminating its process-wide override assertions.
• Documents why the class-level environment cohort provides insufficient isolation.

Diagram

graph TD
  Runner["Test Runner"] --> Lock["Assembly Lock"] --> Test["Cursor Spawn Test"] --> Override["Spawn Override"] --> Manager["Watcher Manager"]
  Peers["Peer Tests"] --> Lock

Loading

High-Level Assessment

Method-level assembly exclusivity is the appropriate targeted fix because the override is process-wide and keyed isolation cannot cover every peer capable of spawning a watcher. Broader production refactoring or custom test synchronization would add complexity without improving this focused regression test.

Files changed (1) +3 / -1

Bug fix (1) +3 / -1

CursorHookCommandTests.csIsolate the process-wide watcher spawn override test +3/-1

Isolate the process-wide watcher spawn override test

• Marks the telemetry recovery-spawn test with bare 'NotInParallel', making it exclusive across the test assembly rather than only within the class-level vendor environment cohort. Adds context explaining why process-wide spawn interception requires stronger isolation.

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📜 Skill insights (0)

1. Method combines parallel controls ✗ Dismissed 📘 Rule violation ☼ Reliability

Description

The changed test now has a method-level bare NotInParallel while its class already carries
[NotInParallel("VendorEnvOverrides")]. This violates the requirement that a test not carry both
method- and class-level NotInParallel constraints.

Code

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[149]

+    [Test, NotInParallel]

Evidence

The PR adds [Test, NotInParallel] at the method while the branch file shows the class already has
[NotInParallel("VendorEnvOverrides")], so both constraint levels apply to this test. Compliance ID
16 explicitly lists carrying both method- and class-level NotInParallel constraints as a failure
criterion.

CLAUDE.md: Constrain Process-Heavy and Shared-State Tests with Correct Parallelism Controls
test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[18-19]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[147-149]

Agent prompt

The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
The test method adds a bare `NotInParallel` while `CursorHookCommandTests` already has a class-level keyed `NotInParallel`, creating both method- and class-level constraints on the same test.

## Issue Context
The applicable parallelism rule requires process-global state to use bare `NotInParallel`, while also prohibiting tests from carrying both method- and class-level `NotInParallel` constraints.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[18-19]
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[147-149]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

---

Context sources

Review mode: 🚀 Fast: This is a localized, test-only concurrency-isolation attribute change in one method, with no production behavior or high-risk surface.

Tip of the day

💡 Did you know, you can type 'qodo, fix this' on a finding and the fix lands right on your PR

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

[linear-code]

AI-2387

[realtonyyoung]

Codex review flow — one P1, taken (f34827b).

The reviewer found the same defect class one class over: UnusableUrlGuardTests installs WatcherManager.ProcessStarterForTesting in two tests and nulls it in Dispose, with no exclusion at all, while every other writer of that seam (ClaudeSessionEndHandoffTests, SkillsAutoSyncTests) already carries bare [NotInParallel]. Its starts == 0 assertions were contaminable exactly as the one this PR started with. It now carries bare [NotInParallel] too.

The flow also settled the parallelism question qodo raised: method-level bare [NotInParallel] shadowing a class-level keyed one is the mechanism the convention documents, it is strictly stronger, and it loses no exclusion.