Skip to content

fix(runner): persist failure checks when server scenarios throw - #538

Open
mohammedmessaoudene-cmd wants to merge 1 commit into
modelcontextprotocol:mainfrom
mohammedmessaoudene-cmd:codex/conformance451-persist-checks
Open

mohammedmessaoudene-cmd wants to merge 1 commit into
modelcontextprotocol:mainfrom
mohammedmessaoudene-cmd:codex/conformance451-persist-checks

Conversation

@mohammedmessaoudene-cmd

Copy link
Copy Markdown

Refs #451 — this addresses only the missing checks.json after a server scenario exception.

Problem

When scenario.run throws synchronously or rejects, the server runner exits before writing checks.json. In suite mode the CLI reports a synthetic failure in memory, but the result directory has no report.

Change

  • Convert scenario exceptions into the same scenario-identity FAILURE check used by the existing suite fallback, preserve the console diagnostic, then use the existing wire-check and report-writing path.
  • Clear the runner timeout in finally when the scenario promise rejects.
  • Keep filesystem and wire-check infrastructure errors outside the scenario catch. Genuine spec-version inapplicability still skips without checks.json.

Behavior note

Single-scenario exceptions now participate in the existing expected-failures policy, as suite exceptions already do. Without a baseline they still exit with code 1. If explicitly covered by --expected-failures, they may exit with code 0, while the report retains FAILURE. Uncovered failures still exit with code 1. This is an intentional normalization, not a claim that all exit behavior is unchanged.

Validation

Validated on base 7169291ec0b68eb370fddcd9947313ab0d5e4156, Windows x64 / Node 22.16.0:

  • 20 new regression tests: 10 fail / 10 pass on the base, all 20 pass with the patch. They read real temporary JSON reports; targeted mocks cover infrastructure errors.
  • Full repository suite: 642/642 passed. Build, typecheck, ESLint, Prettier and git diff --check passed.
  • Real TypeScript SDK 1.29.0 and C# SDK 2.2.0 (.NET 8.0.31), loopback-only: 14 controlled CLI cases each. Test-only source scenario injection exercises success, failure, synchronous throw, rejection after an SDK tools/list exchange, timeout, skips, suite continuation and baseline handling. The compiled stock CLI also passes server-initialize against both SDKs; a controlled HTTP failure stays red and a genuinely inapplicable revision stays skipped.
  • A five-scenario injected suite writes three reports on the base and five with this patch, and exits with code 1 in both cases.

Scope

These are minimal real SDK servers and controlled runner tests, not full SDK conformance certification. No Go test was run. No new normative scenario, check identifier, traceability manifest or baseline file is changed.

This does not address all of #451, the scenario corrections in #537, or the setup-failure classification in #327. It does not add atomic/crash-safe writing, cancellation, or recovery of checks never returned by a throwing scenario.

@chatgpt-codex-connector

Copy link
Copy Markdown

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.
To continue using code reviews, add credits to your account and enable them for code reviews in your settings.

@mohammedmessaoudene-cmd

Copy link
Copy Markdown
Author

The CI workflow for this PR is awaiting approval (action_required), with no jobs executed. After reviewing the diff, could a maintainer approve the CI run?

@rinaldofesta, the focused checks.json persistence follow-up you offered to review in #451 is ready here. The regression tests, TypeScript/C# validation, and expected-failures behavior are documented in the PR description. Your review would be appreciated. Thank you!

This branch has not been deployed

No deployments
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.

1 participant