[AI-2092][AI-2102] De-flake two CI-only flaky tests

[realtonyyoung]

De-flakes two CI-only flaky tests, each a transient/environmental race rather than a real regression. Test-only changes.

Fixes

Issue	Test	Root cause	Fix
AI-2092	AgentVerbDispatchTests.Start_without_a_server… (ubuntu)	RunCli cleared only KCAP_URL; the child inherits the assembly's shared KCAP_CONFIG_DIR, and the CLI resolves the server from a persisted profile too — a sibling test writing a server_url there defeats clearServerUrl	give RunCli a fresh per-call KCAP_CONFIG_DIR so the test-controlled KCAP_URL is the only server signal
AI-2102	CursorHookCommandTests.CancelledFetch_leaves_lease_uncommitted (windows)	the 250ms memory budget started before the request entered the handler, so under load the token could fire before SendAsync ran (Entered false)	make cancellation deterministic — the handler signals entry (EnteredSignal) and the test triggers Release() only after awaiting it; the request still binds to and honours its own budget/deadline token

Verification (local, macOS arm64)

• AgentVerbDispatchTests: 11 passed / 0 failed.
• CursorHookCommandTests.CancelledFetch_leaves_lease_uncommitted: passes.
• The other 7 CursorHookCommandTests failures locally are a pre-existing baseline (coordination-notice injection with no isolated config) — identical on unmodified origin/main in a detached baseline worktree, and untouched by this diff.
• scripts/check-linear-ids.sh: green (issue IDs kept out of .cs).

🤖 Generated with Claude Code

[linear-code]

AI-2092

AI-2102

[qodo-code-review]

PR Summary by Qodo

De-flake CLI tests via per-run config isolation and deterministic cancellation

🧪 Tests 🐞 Bug fix 🕐 20-40 Minutes

AI Description

• Isolate CLI integration runs with a fresh KCAP_CONFIG_DIR to avoid shared-profile races.
• Make cancellation in CursorHookCommand test deterministic by canceling only after handler entry.
• Reduce CI-only timing/environment sensitivity without changing production behavior.

Diagram

graph TD
  IT["AgentVerbDispatchTests"] --> RC["RunCli()"] --> ENV["Env vars"] --> TD[("Temp config dir")]
  ENV --> CLI["Capacitor CLI process"]
  UT["CursorHookCommandTests"] --> HC["CursorHookCommand.HandleCore"] --> HND["CancelAwareHandler"] --> SIG["EnteredSignal then Release()"]

  subgraph Legend
    direction LR
    _test["Test"] ~~~ _code["Code path"] ~~~ _dir[("Temp dir")]
  end

Loading

High-Level Assessment

The following are alternative approaches to this PR:

1. Reset shared IntegrationGlobalSetup config between tests

• ➕ Keeps KCAP_CONFIG_DIR stable while still preventing cross-test pollution
• ➕ Centralizes isolation policy in one place
• ➖ Harder to guarantee correctness if tests run in parallel or crash mid-run
• ➖ Requires careful cleanup semantics and may still be racy under load

2. Disable/serialize the affected integration test collection

• ➕ Simple mitigation if cross-test writes are the only issue
• ➕ Avoids per-test filesystem churn
• ➖ Slows CI and masks underlying isolation problems
• ➖ Doesn’t help if other tests later introduce similar shared-state flakiness

3. Use a fake clock/time-controlled cancellation for the HTTP test

• ➕ Avoids real Task.Delay/cancellation timing entirely
• ➕ Can be more explicit about the intended timeout semantics
• ➖ May require non-trivial harness changes if production code relies on real CancellationTokens
• ➖ Risk of diverging from real cancellation plumbing being exercised

Recommendation: Keep the PR’s approach: per-invocation KCAP_CONFIG_DIR is the most robust fix for shared-profile interference, and the EnteredSignal/Release handshake makes the cancellation assertion deterministic while still validating cancellation propagation through the code under test. The alternatives either reduce parallelism/coverage or require broader harness changes for less direct value.

Files changed (2) +48 / -14

Tests (2) +48 / -14

AgentVerbDispatchTests.csIsolate CLI runs with per-call KCAP_CONFIG_DIR +8/-2

Isolate CLI runs with per-call KCAP_CONFIG_DIR

• Updates the CLI spawning helper to create a fresh TempDir and set KCAP_CONFIG_DIR for each invocation. This prevents other integration tests from influencing server resolution through a shared persisted profile when KCAP_URL is cleared.

test/Capacitor.Cli.Tests.Integration/AgentVerbDispatchTests.cs

CursorHookCommandTests.csMake cancellation test deterministic using EnteredSignal + Release +40/-12

Make cancellation test deterministic using EnteredSignal + Release

• Reworks the flaky cancellation test to start HandleCore, wait until the handler is entered, then trigger cancellation explicitly via Release() instead of relying on a short time budget. Enhances CancelAwareHandler to expose an entry signal, link cancellation tokens, and dispose its CTS.

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

[qodo-code-review]

Code Review by Qodo

🐞 Bugs (0) 📘 Rule violations (0) 📎 Requirement gaps (0) 🎨 UX issues (0) 🔗 Cross-repo conflicts (0) 📜 Skill insights (0)

1. Unbounded EnteredSignal await ✓ Resolved 🐞 Bug ☼ Reliability

Description

CancelledFetch_leaves_lease_uncommitted now awaits handler.EnteredSignal.Task with no timeout,
so if the HTTP request is never issued (e.g., a future change skips the memory fetch), the test can
hang until the runner kills it. This can stall CI rather than failing fast with a clear assertion.

Code

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[R892-893]

+        await handler.EnteredSignal.Task; // the fetch has entered the handler...
+        handler.Release();                // ...only now cancel it — entry can never lose the race.

Evidence

The test now blocks on EnteredSignal.Task before awaiting HandleCore, and EnteredSignal is
only set from within SendAsync. If SendAsync is never invoked, the await has no escape hatch.

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[885-904]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[1166-1172]

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 awaits `handler.EnteredSignal.Task` without any timeout. If `SendAsync` is never entered (because the code path changes, an early return happens, or the memory fetch is skipped), `EnteredSignal` never completes and the test can hang.

## Issue Context
`EnteredSignal` is only completed inside `CancelAwareHandler.SendAsync`, so the test needs a bounded wait to avoid indefinite hangs and to surface a clear failure message.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[885-904]
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[1166-1172]

## Suggested fix
Wrap the wait in a timeout (e.g. `await handler.EnteredSignal.Task.WaitAsync(TimeSpan.FromSeconds(5))`), or use `Task.WhenAny` with `Task.Delay(...)` and assert that `EnteredSignal` won. Include an assertion message explaining that the memory-index request was never started.

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

---

2. CancelAwareHandler comment too long ✓ Resolved 📘 Rule violation ⚙ Maintainability

Description

A very long comment block was added that provides step-by-step narration of mechanics the code
already makes clear, which adds noise (especially in test helper/test-only code) instead of
capturing only the non-obvious rationale or constraints. This conflicts with the project guideline
that comments should be minimal and “why”-focused rather than detailed history or narration.

Code

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[R1146-1152]

+    //
+    // Cancellation is TEST-DRIVEN, not wall-clock-driven: SendAsync signals
+    // <see cref="EnteredSignal"/> the instant it is entered, and the test calls <see cref="Release"/>
+    // only AFTER awaiting that signal — so entry deterministically precedes cancellation and can
+    // never lose a race to a real-time budget the way `memBudget: 250ms` did. The request is still
+    // bound to (and honours) its own budget/deadline token via the linked source, so the fetch is
+    // still proven to be cancellable through the plumbing under test.

Evidence

PR Compliance ID 24 requires short, rationale-focused comments rather than excessive narration. The
cited additions show a multi-line block (including <see cref=...> references, detailed
timing/race-condition history, and narrative about sibling tests and specific failure modes) that is
far longer than the typical 1–2 lines and goes beyond essential non-obvious rationale, demonstrating
a violation of the guideline.

CLAUDE.md: Comments Must Be Minimal and Explain Non-Obvious ‘Why’, Not ‘What’
test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[1146-1152]
test/Capacitor.Cli.Tests.Integration/AgentVerbDispatchTests.cs[122-129]

Agent prompt

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

## Issue description
A multi-line explanatory comment block was added in tests that narrates mechanics and historical details (race/timing history, sibling tests, failure modes) in excessive detail; the project expects comments to be minimal and to capture only the non-obvious “why”/invariants.

## Issue Context
PR Compliance ID 24 calls for short, rationale-focused comments. This PR is test-only and the deeper history/rationale is already documented in PR/issue context, so keeping a large narrative block inline increases noise and maintenance burden; reduce the comment to the smallest statement that preserves the necessary invariant/constraint for maintaining the test.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[1146-1152]
- test/Capacitor.Cli.Tests.Integration/AgentVerbDispatchTests.cs[122-129]

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

---

3. Cancellation token not validated ✓ Resolved 🐞 Bug ≡ Correctness

Description

The new CancelAwareHandler cancels via its own _release token, so the test will still pass even
if the production code mistakenly calls HttpClient.SendAsync with CancellationToken.None (i.e.,
the mem-budget/deadline token is not actually plumbed into the request). This reduces the test’s
ability to catch the specific regression it’s meant to guard: “request cancellation is honored
through the plumbing under test.”

Code

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[R1168-1171]

+            EnteredSignal.TrySetResult();
+            using var linked = CancellationTokenSource.CreateLinkedTokenSource(ct, _release.Token);
            try {
-                await Task.Delay(Timeout.InfiniteTimeSpan, ct);
+                await Task.Delay(Timeout.InfiniteTimeSpan, linked.Token);

Evidence

The handler creates a linked token from the production ct and its own _release token, and the
test triggers cancellation by calling Release(). Therefore, cancellation can occur even if ct is
CancellationToken.None, which would mask regressions where the budget/deadline token isn’t passed
to the HTTP call despite production code’s stated intent.

test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[889-895]
test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[1153-1172]
src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs[535-546]
src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs[579-590]

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 now forces cancellation using a handler-owned `CancellationTokenSource` (`_release`). Because `SendAsync` awaits `Task.Delay(..., linked.Token)` where `linked` includes `_release.Token`, the request will cancel even if the production code fails to pass a cancelable token into `SendAsync` (e.g., passes `CancellationToken.None`). That means the test no longer validates the critical behavior that the request is bound to the mem-budget/deadline token.

## Issue Context
Production code explicitly relies on passing a linked, budget-bound token into the memory request so a cancelled fetch leaves the lease uncommitted.

## Fix Focus Areas
- test/Capacitor.Cli.Tests.Unit/Commands/Harness/CursorHookCommandTests.cs[1153-1174]
- src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs[535-546]
- src/Capacitor.Cli/Commands/Harness/CursorHookCommand.cs[579-590]

## Suggested fix
In `CancelAwareHandler.SendAsync`, record whether the incoming `ct` is actually cancelable (e.g., store `ct.CanBeCanceled` into a field/property). Then, in the test, assert that value is `true` after `EnteredSignal` fires.

Optionally also capture `ct` registration behavior (e.g., create a `TaskCompletionSource` completed by `ct.Register(...)`) to ensure the production token is capable of cancellation independent of `_release`.

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

---

Tip of the day

💡 Did you know, you can commit Qodo's fix in one click with committable suggestions (GitHub & GitLab)

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗