test(integration): unblock runner, drop stale assertions#28
Conversation
The integration runner used to die at claude-mcp-tools because three test files had a top-level `await run(); process.exit(...)` that fires when the runner imports the module, killing the runner before it could move on. claude-mcp-tools / claude-tool-discovery / claude-user-experience now guard that block with `import.meta.url === pathToFileURL(process.argv[1]).href` and export `run` as default so the runner can import and invoke it cleanly. mcp-schema.test.ts gets the same guard plus a default-export wrapper that converts ECONNREFUSED into a "skipped" result. The Claude-driven tests (claude-tool-discovery, claude-user-experience) also gain a preflight ping to `http://127.0.0.1:8787/sse`: when wrangler dev isn't running, they now skip instead of running `claude -p` against a dead URL and producing 25+ noisy assertion failures. health-check.test.js was asserting against a `/health` response shape that hasn't existed for months (`endpoints`, `authentication`, `supported_formats`, `examples` are all gone — `/health` today is just `status/service/version/timestamp`). Rewrote the test to assert what the endpoint actually returns. authentication.test.js had a single test ("Health endpoint shows OAuth configuration") asserting the same gone field — dropped it. button-rounding-experiment.test.js calls `list_experiments`, `get_experiment`, `create_experiment` — none of which the MCP exposes anymore (the CLI rewrite consolidated to 4 tools: discover_commands/get_command_docs/execute_command/get_auth_status). claude-mcp-tools.test.js already covers equivalent flows via the new tool API. Removed. Result: integration suite goes from "runner dies partway through" to "42 passed, 2 failed" (97.6%). The 2 remaining failures are backend owner-validation issues against the test-1 sandbox — out of scope for this cleanup, called out as Phase 2.
WalkthroughThis pull request refactors the integration test suite infrastructure and simplifies health-check validation. Test modules (claude-mcp-tools, claude-tool-discovery, claude-user-experience, mcp-schema) are converted to importable ESM modules with conditional execution guards that only trigger when run as the main script. New reachability probing functions detect when the MCP endpoint is inaccessible and gracefully skip tests. Health-check assertions are simplified by removing authentication documentation validation and endpoints field checks, whilst introducing shared timing constants. The button-rounding-experiment test suite is removed entirely, and an authentication test is removed from the authentication test file. Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
ESLint skipped: no ESLint configuration detected in root package.json. To enable, add Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 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/integration/claude-mcp-tools.test.js`:
- Around line 476-477: Extract the inline literal process.argv[1] into a
descriptive ALL_CAPS top-level constant (e.g., ENTRY_ARGV_INDEX =
process.argv[1]) declared at the top of
tests/integration/claude-mcp-tools.test.js, then replace the module-entry guard
usage (the if that compares import.meta.url ===
pathToFileURL(process.argv[1]).href) to reference that constant instead (e.g.,
pathToFileURL(ENTRY_ARGV_INDEX).href); ensure the constant name is descriptive
and reused wherever that argv index is needed.
In `@tests/integration/claude-tool-discovery.test.js`:
- Around line 691-692: Replace the inline use of process.argv[1] in the
main-module guard with a top-level ALL_CAPS constant: declare a descriptive
constant (e.g., MAIN_MODULE_ARGV_INDEX = 1) near the top of the file and then
change the guard to use process.argv[MAIN_MODULE_ARGV_INDEX]; keep the rest of
the expression (import.meta.url === pathToFileURL(process.argv[...]).href) and
the run() call unchanged.
- Around line 185-191: The isLocalMcpReachable function currently sets a timeout
(1500) that isn't cleared if fetch throws; move clearTimeout(timer) into a
finally block so the timer is always cleaned up, and promote the magic number
1500 to a top-level constant (e.g. MCP_REACH_TIMEOUT_MS) grouped with other
constants; update isLocalMcpReachable to use that constant and ensure the
AbortController and timer are declared so they can be cleared in finally.
In `@tests/integration/claude-user-experience.test.js`:
- Around line 258-264: The isLocalMcpReachable function leaves the timeout timer
uncleared on fetch failures; extract the numeric timeout (1500) into a top-level
constant (e.g., MCP_TIMEOUT_MS) grouped with other related constants, use that
constant instead of the magic number, and ensure the timer is always cleared by
moving clearTimeout(timer) into a finally block after the await fetch(MCP_URL, {
signal: controller.signal }) so the timer is cleaned up on both success and
failure; keep LIVE_MODE and MCP_URL usage unchanged.
- Around line 736-737: Replace the inline process.argv[1] used in the entry
guard (if (import.meta.url === pathToFileURL(process.argv[1]).href) { ... })
with a descriptive ALL_CAPS constant declared at the top of the test file (e.g.,
SCRIPT_ARG_INDEX or ENTRY_ARG_INDEX) and use that constant in the call to
pathToFileURL; update any references to process.argv[1] in this file to use the
constant so the magic index is declared once and clearly named while preserving
the existing behavior around run() and the entry guard.
In `@tests/integration/health-check.test.js`:
- Line 104: The test contains a hardcoded magic string 'absmartly-mcp' in the
assertion (assertEquals(data.service, 'absmartly-mcp', ...)); extract it into a
descriptive ALL_CAPS constant (e.g. SERVICE_NAME = 'absmartly-mcp') declared at
the top of the file alongside the other constants, then replace the inline
string in the assertEquals call with that constant to follow the project's
no-magic-strings guideline.
- Line 97: Extract the hardcoded content type string by declaring a top-level
constant (e.g., JSON_CONTENT_TYPE = 'application/json') near the other
file-level constants, then replace the inline literal in the assertion (the
expression using contentType?.includes('application/json')) with the new
constant (contentType?.includes(JSON_CONTENT_TYPE)); update any other
occurrences in this test file to use JSON_CONTENT_TYPE to avoid magic strings
and follow ALL_CAPS naming.
- Line 78: Extract the magic string 'healthy' into a top-level constant (e.g.,
HEALTHY_STATUS) declared with the other constants at the top of the file, then
replace the inline literal in the assertion assertTrue(data.status ===
'healthy', ...) with a comparison to that constant (assertTrue(data.status ===
HEALTHY_STATUS, ...)); ensure the constant uses ALL_CAPS and a descriptive name
to follow the project's coding guidelines.
In `@tests/integration/mcp-schema.test.ts`:
- Around line 306-313: The direct-entry main-module block currently calls run()
and thus bypasses the runWrapped() skip/error conversion logic; change the
invocation inside the import.meta.url === pathToFileURL(process.argv[1]).href
block to call runWrapped() (and preserve the same promise handling: .then(ok =>
process.exit(ok ? 0 : 1)).catch(...)) so ECONNREFUSED/fetch failures are
converted to skipped results; update the invocation reference from run to
runWrapped and keep existing console.error/process.exit behavior.
🪄 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: CHILL
Plan: Pro
Run ID: b2277232-079e-46df-867b-7c22c5cf7ec5
📒 Files selected for processing (7)
tests/integration/authentication.test.jstests/integration/button-rounding-experiment.test.jstests/integration/claude-mcp-tools.test.jstests/integration/claude-tool-discovery.test.jstests/integration/claude-user-experience.test.jstests/integration/health-check.test.jstests/integration/mcp-schema.test.ts
💤 Files with no reviewable changes (2)
- tests/integration/button-rounding-experiment.test.js
- tests/integration/authentication.test.js
| if (import.meta.url === pathToFileURL(process.argv[1]).href) { | ||
| const result = await run(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Extract the argv index into a named top-level constant.
Please avoid the inline process.argv[1] literal in the module-entry guard; define a descriptive ALL_CAPS constant at the top of the file and reuse it here.
As per coding guidelines, "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file" and "Use descriptive constant names with ALL_CAPS naming convention".
🤖 Prompt for 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.
In `@tests/integration/claude-mcp-tools.test.js` around lines 476 - 477, Extract
the inline literal process.argv[1] into a descriptive ALL_CAPS top-level
constant (e.g., ENTRY_ARGV_INDEX = process.argv[1]) declared at the top of
tests/integration/claude-mcp-tools.test.js, then replace the module-entry guard
usage (the if that compares import.meta.url ===
pathToFileURL(process.argv[1]).href) to reference that constant instead (e.g.,
pathToFileURL(ENTRY_ARGV_INDEX).href); ensure the constant name is descriptive
and reused wherever that argv index is needed.
| async function isLocalMcpReachable() { | ||
| if (LIVE_MODE) return true; | ||
| try { | ||
| const controller = new AbortController(); | ||
| const timer = setTimeout(() => controller.abort(), 1500); | ||
| await fetch(MCP_URL, { signal: controller.signal }); | ||
| clearTimeout(timer); |
There was a problem hiding this comment.
Always clear the reachability timeout in a finally block.
If fetch throws before success, the timeout is not cleared. Move cleanup to finally, and promote 1500 to a top-level constant.
Suggested patch
+const LOCAL_MCP_REACHABILITY_TIMEOUT_MS = 1_500;
+
async function isLocalMcpReachable() {
if (LIVE_MODE) return true;
+ let timer;
try {
const controller = new AbortController();
- const timer = setTimeout(() => controller.abort(), 1500);
+ timer = setTimeout(() => controller.abort(), LOCAL_MCP_REACHABILITY_TIMEOUT_MS);
await fetch(MCP_URL, { signal: controller.signal });
- clearTimeout(timer);
return true;
} catch {
return false;
+ } finally {
+ if (timer) clearTimeout(timer);
}
}As per coding guidelines, "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file" and "Group related constants together".
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async function isLocalMcpReachable() { | |
| if (LIVE_MODE) return true; | |
| try { | |
| const controller = new AbortController(); | |
| const timer = setTimeout(() => controller.abort(), 1500); | |
| await fetch(MCP_URL, { signal: controller.signal }); | |
| clearTimeout(timer); | |
| const LOCAL_MCP_REACHABILITY_TIMEOUT_MS = 1_500; | |
| async function isLocalMcpReachable() { | |
| if (LIVE_MODE) return true; | |
| let timer; | |
| try { | |
| const controller = new AbortController(); | |
| timer = setTimeout(() => controller.abort(), LOCAL_MCP_REACHABILITY_TIMEOUT_MS); | |
| await fetch(MCP_URL, { signal: controller.signal }); | |
| return true; | |
| } catch { | |
| return false; | |
| } finally { | |
| if (timer) clearTimeout(timer); | |
| } | |
| } |
🤖 Prompt for 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.
In `@tests/integration/claude-tool-discovery.test.js` around lines 185 - 191, The
isLocalMcpReachable function currently sets a timeout (1500) that isn't cleared
if fetch throws; move clearTimeout(timer) into a finally block so the timer is
always cleaned up, and promote the magic number 1500 to a top-level constant
(e.g. MCP_REACH_TIMEOUT_MS) grouped with other constants; update
isLocalMcpReachable to use that constant and ensure the AbortController and
timer are declared so they can be cleared in finally.
| if (import.meta.url === pathToFileURL(process.argv[1]).href) { | ||
| const result = await run(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Avoid inline argv index in the main-module guard.
Please replace process.argv[1] with a named top-level constant to keep the file compliant and self-documenting.
As per coding guidelines, "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file" and "Use descriptive constant names with ALL_CAPS naming convention".
🤖 Prompt for 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.
In `@tests/integration/claude-tool-discovery.test.js` around lines 691 - 692,
Replace the inline use of process.argv[1] in the main-module guard with a
top-level ALL_CAPS constant: declare a descriptive constant (e.g.,
MAIN_MODULE_ARGV_INDEX = 1) near the top of the file and then change the guard
to use process.argv[MAIN_MODULE_ARGV_INDEX]; keep the rest of the expression
(import.meta.url === pathToFileURL(process.argv[...]).href) and the run() call
unchanged.
| async function isLocalMcpReachable() { | ||
| if (LIVE_MODE) return true; | ||
| try { | ||
| const controller = new AbortController(); | ||
| const timer = setTimeout(() => controller.abort(), 1500); | ||
| await fetch(MCP_URL, { signal: controller.signal }); | ||
| clearTimeout(timer); |
There was a problem hiding this comment.
Mirror the timeout cleanup fix here as well.
clearTimeout only runs on success; on fetch failure the timer remains pending. Use finally, and extract 1500 into a top-level constant.
As per coding guidelines, "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file" and "Group related constants together".
🤖 Prompt for 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.
In `@tests/integration/claude-user-experience.test.js` around lines 258 - 264, The
isLocalMcpReachable function leaves the timeout timer uncleared on fetch
failures; extract the numeric timeout (1500) into a top-level constant (e.g.,
MCP_TIMEOUT_MS) grouped with other related constants, use that constant instead
of the magic number, and ensure the timer is always cleared by moving
clearTimeout(timer) into a finally block after the await fetch(MCP_URL, {
signal: controller.signal }) so the timer is cleaned up on both success and
failure; keep LIVE_MODE and MCP_URL usage unchanged.
| if (import.meta.url === pathToFileURL(process.argv[1]).href) { | ||
| const result = await run(); |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Use a named constant for the argv index in the entry guard.
Inline process.argv[1] should be replaced by a descriptive ALL_CAPS constant declared at file top.
As per coding guidelines, "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file" and "Use descriptive constant names with ALL_CAPS naming convention".
🤖 Prompt for 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.
In `@tests/integration/claude-user-experience.test.js` around lines 736 - 737,
Replace the inline process.argv[1] used in the entry guard (if (import.meta.url
=== pathToFileURL(process.argv[1]).href) { ... }) with a descriptive ALL_CAPS
constant declared at the top of the test file (e.g., SCRIPT_ARG_INDEX or
ENTRY_ARG_INDEX) and use that constant in the call to pathToFileURL; update any
references to process.argv[1] in this file to use the constant so the magic
index is declared once and clearly named while preserving the existing behavior
around run() and the entry guard.
| } | ||
| } | ||
|
|
||
| return assertTrue(data.status === 'healthy', 'Status should be healthy'); |
There was a problem hiding this comment.
Extract magic string to a constant.
The string 'healthy' is hardcoded inline. This should be declared as a constant at the top of the file.
📋 Proposed fix to extract the magic string
Add the constant at the top of the file after the existing constants:
const FETCH_TIMEOUT_MS = 10000;
const MAX_HEALTHY_RESPONSE_MS = 5000;
+const EXPECTED_HEALTH_STATUS = 'healthy';Then update the assertion:
- return assertTrue(data.status === 'healthy', 'Status should be healthy');
+ return assertTrue(data.status === EXPECTED_HEALTH_STATUS, 'Status should be healthy');As per coding guidelines: "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file. Use descriptive constant names with ALL_CAPS naming convention."
🤖 Prompt for 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.
In `@tests/integration/health-check.test.js` at line 78, Extract the magic string
'healthy' into a top-level constant (e.g., HEALTHY_STATUS) declared with the
other constants at the top of the file, then replace the inline literal in the
assertion assertTrue(data.status === 'healthy', ...) with a comparison to that
constant (assertTrue(data.status === HEALTHY_STATUS, ...)); ensure the constant
uses ALL_CAPS and a descriptive name to follow the project's coding guidelines.
| } | ||
|
|
||
| return true; | ||
| return assertTrue(contentType?.includes('application/json'), `Wrong content type: ${contentType}`); |
There was a problem hiding this comment.
Extract magic string to a constant.
The string 'application/json' is hardcoded inline. This should be declared as a constant at the top of the file.
📋 Proposed fix to extract the magic string
Add the constant at the top of the file after the existing constants:
const FETCH_TIMEOUT_MS = 10000;
const MAX_HEALTHY_RESPONSE_MS = 5000;
+const EXPECTED_CONTENT_TYPE = 'application/json';Then update the assertion:
- return assertTrue(contentType?.includes('application/json'), `Wrong content type: ${contentType}`);
+ return assertTrue(contentType?.includes(EXPECTED_CONTENT_TYPE), `Wrong content type: ${contentType}`);As per coding guidelines: "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file. Use descriptive constant names with ALL_CAPS naming convention."
🤖 Prompt for 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.
In `@tests/integration/health-check.test.js` at line 97, Extract the hardcoded
content type string by declaring a top-level constant (e.g., JSON_CONTENT_TYPE =
'application/json') near the other file-level constants, then replace the inline
literal in the assertion (the expression using
contentType?.includes('application/json')) with the new constant
(contentType?.includes(JSON_CONTENT_TYPE)); update any other occurrences in this
test file to use JSON_CONTENT_TYPE to avoid magic strings and follow ALL_CAPS
naming.
| const data = await response.json(); | ||
|
|
||
| assertEquals(data.service, 'absmartly-mcp', 'Service name mismatch'); |
There was a problem hiding this comment.
Extract magic string to a constant.
The string 'absmartly-mcp' is hardcoded inline. This should be declared as a constant at the top of the file.
📋 Proposed fix to extract the magic string
Add the constant at the top of the file after the existing constants:
const FETCH_TIMEOUT_MS = 10000;
const MAX_HEALTHY_RESPONSE_MS = 5000;
+const EXPECTED_SERVICE_NAME = 'absmartly-mcp';Then update the assertion:
- assertEquals(data.service, 'absmartly-mcp', 'Service name mismatch');
+ assertEquals(data.service, EXPECTED_SERVICE_NAME, 'Service name mismatch');As per coding guidelines: "Never use magic strings or hardcoded values inline in code - all default values must be declared as constants at the top of the file. Use descriptive constant names with ALL_CAPS naming convention."
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| assertEquals(data.service, 'absmartly-mcp', 'Service name mismatch'); | |
| // At the top of the file with other constants: | |
| const FETCH_TIMEOUT_MS = 10000; | |
| const MAX_HEALTHY_RESPONSE_MS = 5000; | |
| const EXPECTED_SERVICE_NAME = 'absmartly-mcp'; | |
| // Updated assertion at line 104: | |
| assertEquals(data.service, EXPECTED_SERVICE_NAME, 'Service name mismatch'); |
🤖 Prompt for 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.
In `@tests/integration/health-check.test.js` at line 104, The test contains a
hardcoded magic string 'absmartly-mcp' in the assertion
(assertEquals(data.service, 'absmartly-mcp', ...)); extract it into a
descriptive ALL_CAPS constant (e.g. SERVICE_NAME = 'absmartly-mcp') declared at
the top of the file alongside the other constants, then replace the inline
string in the assertEquals call with that constant to follow the project's
no-magic-strings guideline.
| if (import.meta.url === pathToFileURL(process.argv[1]).href) { | ||
| run() | ||
| .then(ok => process.exit(ok ? 0 : 1)) | ||
| .catch(err => { | ||
| console.error('Fatal:', err); | ||
| process.exit(1); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Direct-entry path bypasses runWrapped skip handling.
The main-module block still invokes run(), so ECONNREFUSED/fetch failed is not converted to a skipped result when executed directly. Call runWrapped() here instead.
Suggested patch
if (import.meta.url === pathToFileURL(process.argv[1]).href) {
- run()
- .then(ok => process.exit(ok ? 0 : 1))
+ runWrapped()
+ .then(result => process.exit(result.success ? 0 : 1))
.catch(err => {
console.error('Fatal:', err);
process.exit(1);
});
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (import.meta.url === pathToFileURL(process.argv[1]).href) { | |
| run() | |
| .then(ok => process.exit(ok ? 0 : 1)) | |
| .catch(err => { | |
| console.error('Fatal:', err); | |
| process.exit(1); | |
| }); | |
| } | |
| if (import.meta.url === pathToFileURL(process.argv[1]).href) { | |
| runWrapped() | |
| .then(result => process.exit(result.success ? 0 : 1)) | |
| .catch(err => { | |
| console.error('Fatal:', err); | |
| process.exit(1); | |
| }); | |
| } |
🤖 Prompt for 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.
In `@tests/integration/mcp-schema.test.ts` around lines 306 - 313, The
direct-entry main-module block currently calls run() and thus bypasses the
runWrapped() skip/error conversion logic; change the invocation inside the
import.meta.url === pathToFileURL(process.argv[1]).href block to call
runWrapped() (and preserve the same promise handling: .then(ok =>
process.exit(ok ? 0 : 1)).catch(...)) so ECONNREFUSED/fetch failures are
converted to skipped results; update the invocation reference from run to
runWrapped and keep existing console.error/process.exit behavior.
1 participant
Summary
Brings the integration suite from "runner dies partway through" to 42 passed / 2 failed (97.6%). The 2 remaining failures are backend owner-validation issues against the
test-1sandbox — explicitly out of scope here.Root causes fixed
1. Runner-killing top-level
process.exitclaude-mcp-tools,claude-tool-discovery,claude-user-experienceall had:```js
const result = await run();
process.exit(result.success ? 0 : 1);
```
…at module scope. When
tests/test-runner.jsimported them, that block ran andprocess.exitkilled the runner before it could reach the next file. The 8 files alphabetically afterclaude-mcp-tools.test.jsnever ran at all.Wrapped each in a standard ESM "main module" guard:
```js
export default run;
if (import.meta.url === pathToFileURL(process.argv[1]).href) {
const result = await run();
process.exit(result.success ? 0 : 1);
}
```
Same fix applied to
mcp-schema.test.ts, which had no default export at all and an unguardedrun().then(...).catch(...)at the bottom. Added a default-export wrapper that convertsECONNREFUSEDfrom a missing local wrangler dev into a clean `skipped` result.2. Claude-driven tests had no preflight
claude-tool-discoveryandclaude-user-experienceshell out to `claude -p` against `http://127.0.0.1:8787/sse\` but don't manage their own wrangler dev (onlyclaude-mcp-toolsdoes). When wrangler is down, the previous behavior was 25+ noisy assertion failures ("I don't have access to adiscover_commandstool…").Added a 1.5 s preflight ping to the local MCP URL. When unreachable, both tests now skip cleanly with a clear message: `Start wrangler dev or pass --live`.
3.
health-check.test.jsasserting on a long-gone response shapeThe deployed
/healthtoday returns:```json
{ "status": "healthy", "service": "absmartly-mcp", "version": "1.0.0", "timestamp": "…" }
```
The test was asserting on `endpoints`, `authentication`, `supported_formats`, `examples` — none of which the endpoint has produced for months. Rewrote to match reality (7 tests, all passing).
authentication.test.jshad a single test ("Health endpoint shows OAuth configuration") asserting the same missing `authentication` field — dropped it.4.
button-rounding-experiment.test.jsagainst a tool API that doesn't existThe test calls `list_experiments`, `get_experiment`, `create_experiment` directly via the MCP. None of those tools exist — the CLI rewrite (PR #1) consolidated the surface to four tools (`discover_commands`, `get_command_docs`, `execute_command`, `get_auth_status`). `claude-mcp-tools.test.js` already covers the equivalent flows via the new API. Deleted (492 lines).
Out of scope (Phase 2)
claude-mcp-tools(`experiment lifecycle …`, `feature flag lifecycle …`): backend owner-validation against `test-1.absmartly.com`. Tests run `claude -p` which calls the live API, and the API rejects creation without an owner regardless of which parameter shape we try. Needs either backend changes on `test-1` or fixture data.Test plan
Run with: `npm run test:integration -- --profile test-1`
```
✅ api-client-listings 8 / 8
✅ authentication 10 / 10 (was 10 / 11)
✅ backend-oauth-constraints 10 / 10
⚠ claude-mcp-tools 14 / 16 (sandbox owner-validation)
✅ claude-tool-discovery skipped — local MCP not reachable
✅ claude-user-experience skipped — local MCP not reachable
✅ health-check 7 / 7 (was 6 / 10)
✅ mcp-schema skipped — local MCP not reachable
✅ multi-tenant-oauth skipped — MCP server not reachable
✅ oauth-flow skipped — OAuth server not reachable
✅ resources-prompts-flow skipped — MCP server not reachable
```
Total: 42 passed, 2 failed (97.6%). Down from a runner that died at file #5 of 12.
Summary by CodeRabbit
Tests