test: stabilize management discovery cancellation coverage

[mohanagy]

Fixes #267.

Root cause

The cancellation regression waited with Vitest's default expect.poll deadline, even though the assertion was only meant to establish that the actual management discovery request had started. Under full coverage, that implicit one-second deadline could expire before the upstream fixture reached its tool-discovery marker.

Change

• add a fixture-only pre-marker delay control;
• make the regression prove the old poll deadline fails;
• wait for the existing file-arrival synchronization boundary, then cancel the real in-flight request.

No production timeout, policy, containment, or coverage threshold changed.

Validation

• npm test -- --reporter=dot
• npm run test:coverage -- --reporter=dot
• npm run test:core -- --reporter=dot
• npm run lint
• npm run typecheck
• npm run build
• npm run smoke:cli
• npm run check:pack
• npm run test:package -- --reporter=dot
• npm run check:test-fixture

Summary by CodeRabbit

• Bug Fixes

  • Improved cancellation handling during management tool discovery, ensuring cancellation requests are detected reliably while discovery is in progress.
  • Streamlined validation behavior for optional object properties to produce more consistent parsed results and issue reporting.

• Tests

  • Updated cancellation coverage to use reliable filesystem event detection and timing scenarios.

[coderabbitai]

📝 Walkthrough

Walkthrough

The bundled validator fixture updates generated-source formatting and optional-property parsing. The runtime fixture adds a configurable ListTools start delay, while the cancellation test synchronizes on a filesystem marker before aborting discovery.

Changes

Bundled parser fixture

Layer / File(s)	Summary
Bundled validator generation and parsing tests/fixtures/fake-upstream-bundled.mjs	Template compilation newline handling, optional-property issue-path mapping, and parsed-value assignment are updated.

Discovery cancellation timing

Layer / File(s)	Summary
List tools start delay tests/fixtures/fake-upstream-runtime.mjs	Adds TEST_LIST_TOOLS_START_DELAY_MS and awaits the configured delay before emitting start signals.
Filesystem-synchronized cancellation test tests/mcp-wrapper.test.ts	The test waits for startedPath, aborts discovery, verifies rejection and cancellation, and closes the watcher.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Possibly related PRs

• mohanagy/miftah#47: Modifies MCP wrapper fixtures and management-tool discovery timing.
• mohanagy/miftah#227: Uses filesystem watching to synchronize cancellation ordering.

Poem

A rabbit watched the tool-list trail,
Then timed the start before the fail.
The parser shaped each path just right,
While watchers closed by candlelight.
“Hop!” said Bun, “the tests now stay—
In step with signals all the way.”

---

Caution

Pre-merge checks failed

Please resolve all errors before merging. Addressing warnings is optional.

• Ignore

❌ Failed checks (1 error)

Check name	Status	Explanation	Resolution
Out of Scope Changes check	❌ Error	The bundled fixture edits change unrelated runtime/parser logic that doesn't appear necessary for management discovery cancellation.	Drop or justify the bundled runtime changes and keep the PR focused on the cancellation test fixture and watcher updates.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title clearly summarizes the main change: stabilizing management discovery cancellation coverage.
Description check	✅ Passed	The description covers the root cause, change, and validation, though it doesn't use the template headings exactly.
Linked Issues check	✅ Passed	The fix uses a fixture delay and file-arrival boundary to reproduce the timing issue while preserving cancellation and cleanup.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/267-coverage-discovery-lifecycle

---

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@tests/mcp-wrapper.test.ts`:
- Around line 2127-2144: Update the test around client.callTool and
toolListStarted.wait to attach an immediate fulfillment/rejection handler to
pending, race that observation with the start-marker wait, and fail the test if
pending settles before startedPath is reached; preserve the existing
cancellation assertion after the marker is observed.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c98ed49a-8310-4f1c-846b-1c5f8b3b9423

📥 Commits

Reviewing files that changed from the base of the PR and between 94b1088 and a5b7318.

📒 Files selected for processing (3)

• tests/fixtures/fake-upstream-bundled.mjs
• tests/fixtures/fake-upstream-runtime.mjs
• tests/mcp-wrapper.test.ts

[mohanagy]

Review follow-up: tests/fixtures/fake-upstream-bundled.mjs is a checked generated fixture. The runtime fixture change is regenerated through npm run build:test-fixture, and npm run check:test-fixture verifies the committed bundle matches that source exactly. The bundled diff cannot be dropped independently without breaking that contract.

[mohanagy]

Review record: CodeRabbit's current check explicitly reports Review rate limited, after its only actionable thread was resolved in a73f0f8. Per repository policy, I attempted the approved local Claude Code fallback: Fable reported its quota limit and Opus returned no review output before ending with an execution error.

I completed a manual diff review of origin/development...a73f0f8: the marker watcher is armed before the request, the request's completion is immediately observed and raced against the marker, early fulfillment/rejection fails fast, cancellation still happens only after the real start boundary, and watcher cleanup remains in finally. The bundled fixture is verified by npm run check:test-fixture. No further actionable findings.

Dismissed because the sole actionable thread was resolved in a73f0f8, while the current-head CodeRabbit status explicitly reports Review rate limited. The approved local-review fallback attempt and manual review record are documented on this PR.