Skip to content

Commit 976a8f6

Browse files
pcarletonclaude
andauthored
fix: validate scope checks ran and report overall test status (#80)
* fix: validate scope checks ran and report overall test status - Add FAILURE checks to scope scenarios when authorization flow doesn't complete (ScopeFromWwwAuthenticateScenario, ScopeFromScopesSupportedScenario, ScopeOmittedWhenUndefinedScenario) - Report client timeout and exit errors in test results - Add OVERALL: PASSED/FAILED summary at end of test output - Exit with code 1 on any failure (check failures, client timeout, or exit error) Previously, if the OAuth authorization flow didn't complete, the scope checks were never triggered and the test would incorrectly report success. Now these scenarios emit a FAILURE if the expected scope check wasn't recorded. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Count warnings as failures in overall test result Warnings now contribute to the overall failure status, causing the test to exit with code 1 when any warnings are present. Also displays warning checks in the output summary, similar to how failures are displayed. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> * Count warnings as failures in suite runner Update the suite summary to show ✗ for scenarios with warnings, and exit with code 1 if any warnings are present across the suite. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]> --------- Co-authored-by: Claude <[email protected]>
1 parent e5cdffe commit 976a8f6

File tree

3 files changed

+99
-7
lines changed

3 files changed

+99
-7
lines changed

src/index.ts

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,7 @@ program
125125
totalFailed += failed;
126126
totalWarnings += warnings;
127127

128-
const status = failed === 0 ? '✓' : '✗';
128+
const status = failed === 0 && warnings === 0 ? '✓' : '✗';
129129
const warningStr = warnings > 0 ? `, ${warnings} warnings` : '';
130130
console.log(
131131
`${status} ${result.scenario}: ${passed} passed, ${failed} failed${warningStr}`
@@ -145,7 +145,7 @@ program
145145
console.log(
146146
`\nTotal: ${totalPassed} passed, ${totalFailed} failed, ${totalWarnings} warnings`
147147
);
148-
process.exit(totalFailed > 0 ? 1 : 0);
148+
process.exit(totalFailed > 0 || totalWarnings > 0 ? 1 : 0);
149149
}
150150

151151
// Require either --scenario or --suite
@@ -173,8 +173,12 @@ program
173173
timeout
174174
);
175175

176-
const { failed } = printClientResults(result.checks, verbose);
177-
process.exit(failed > 0 ? 1 : 0);
176+
const { overallFailure } = printClientResults(
177+
result.checks,
178+
verbose,
179+
result.clientOutput
180+
);
181+
process.exit(overallFailure ? 1 : 0);
178182
} catch (error) {
179183
if (error instanceof ZodError) {
180184
console.error('Validation error:');

src/runner/client.ts

Lines changed: 46 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -150,15 +150,30 @@ export async function runConformanceTest(
150150

151151
export function printClientResults(
152152
checks: ConformanceCheck[],
153-
verbose: boolean = false
154-
): { passed: number; failed: number; denominator: number; warnings: number } {
153+
verbose: boolean = false,
154+
clientOutput?: ClientExecutionResult
155+
): {
156+
passed: number;
157+
failed: number;
158+
denominator: number;
159+
warnings: number;
160+
overallFailure: boolean;
161+
} {
155162
const denominator = checks.filter(
156163
(c) => c.status === 'SUCCESS' || c.status === 'FAILURE'
157164
).length;
158165
const passed = checks.filter((c) => c.status === 'SUCCESS').length;
159166
const failed = checks.filter((c) => c.status === 'FAILURE').length;
160167
const warnings = checks.filter((c) => c.status === 'WARNING').length;
161168

169+
// Determine if there's an overall failure (failures, warnings, client timeout, or exit failure)
170+
const clientTimedOut = clientOutput?.timedOut ?? false;
171+
const clientExitedWithError = clientOutput
172+
? clientOutput.exitCode !== 0
173+
: false;
174+
const overallFailure =
175+
failed > 0 || warnings > 0 || clientTimedOut || clientExitedWithError;
176+
162177
if (verbose) {
163178
// Verbose mode: JSON goes to stdout for piping to jq/jless
164179
console.log(JSON.stringify(checks, null, 2));
@@ -173,6 +188,16 @@ export function printClientResults(
173188
`Passed: ${passed}/${denominator}, ${failed} failed, ${warnings} warnings`
174189
);
175190

191+
if (clientTimedOut) {
192+
console.error(`\n⚠️ CLIENT TIMED OUT - Test incomplete`);
193+
}
194+
195+
if (clientExitedWithError && !clientTimedOut) {
196+
console.error(
197+
`\n⚠️ CLIENT EXITED WITH ERROR (code ${clientOutput?.exitCode}) - Test may be incomplete`
198+
);
199+
}
200+
176201
if (failed > 0) {
177202
console.error('\nFailed Checks:');
178203
checks
@@ -185,7 +210,25 @@ export function printClientResults(
185210
});
186211
}
187212

188-
return { passed, failed, denominator, warnings };
213+
if (warnings > 0) {
214+
console.error('\nWarning Checks:');
215+
checks
216+
.filter((c) => c.status === 'WARNING')
217+
.forEach((c) => {
218+
console.error(` - ${c.name}: ${c.description}`);
219+
if (c.errorMessage) {
220+
console.error(` Warning: ${c.errorMessage}`);
221+
}
222+
});
223+
}
224+
225+
if (overallFailure) {
226+
console.error('\n❌ OVERALL: FAILED');
227+
} else {
228+
console.error('\n✅ OVERALL: PASSED');
229+
}
230+
231+
return { passed, failed, denominator, warnings, overallFailure };
189232
}
190233

191234
export async function runInteractiveMode(

src/scenarios/client/auth/scope-handling.ts

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,21 @@ export class ScopeFromWwwAuthenticateScenario implements Scenario {
7373
}
7474

7575
getChecks(): ConformanceCheck[] {
76+
// Emit failure check if expected scope check didn't run
77+
const hasScopeCheck = this.checks.some(
78+
(c) => c.id === 'scope-from-www-authenticate'
79+
);
80+
if (!hasScopeCheck) {
81+
this.checks.push({
82+
id: 'scope-from-www-authenticate',
83+
name: 'Client scope selection from WWW-Authenticate header',
84+
description:
85+
'Client did not complete authorization flow - scope check could not be performed',
86+
status: 'FAILURE',
87+
timestamp: new Date().toISOString(),
88+
specReferences: [SpecReferences.MCP_SCOPE_SELECTION_STRATEGY]
89+
});
90+
}
7691
return this.checks;
7792
}
7893
}
@@ -153,6 +168,21 @@ export class ScopeFromScopesSupportedScenario implements Scenario {
153168
}
154169

155170
getChecks(): ConformanceCheck[] {
171+
// Emit failure check if expected scope check didn't run
172+
const hasScopeCheck = this.checks.some(
173+
(c) => c.id === 'scope-from-scopes-supported'
174+
);
175+
if (!hasScopeCheck) {
176+
this.checks.push({
177+
id: 'scope-from-scopes-supported',
178+
name: 'Client scope selection from scopes_supported',
179+
description:
180+
'Client did not complete authorization flow - scope check could not be performed',
181+
status: 'FAILURE',
182+
timestamp: new Date().toISOString(),
183+
specReferences: [SpecReferences.MCP_SCOPE_SELECTION_STRATEGY]
184+
});
185+
}
156186
return this.checks;
157187
}
158188
}
@@ -221,6 +251,21 @@ export class ScopeOmittedWhenUndefinedScenario implements Scenario {
221251
}
222252

223253
getChecks(): ConformanceCheck[] {
254+
// Emit failure check if expected scope check didn't run
255+
const hasScopeCheck = this.checks.some(
256+
(c) => c.id === 'scope-omitted-when-undefined'
257+
);
258+
if (!hasScopeCheck) {
259+
this.checks.push({
260+
id: 'scope-omitted-when-undefined',
261+
name: 'Client scope omission when scopes_supported undefined',
262+
description:
263+
'Client did not complete authorization flow - scope check could not be performed',
264+
status: 'FAILURE',
265+
timestamp: new Date().toISOString(),
266+
specReferences: [SpecReferences.MCP_SCOPE_SELECTION_STRATEGY]
267+
});
268+
}
224269
return this.checks;
225270
}
226271
}

0 commit comments

Comments
 (0)