test(mcp): extend mcporter smoke for job policy path - #175
tonythethompson wants to merge 1 commit into
Conversation
Cover policy-gated submit, status, cancel, and idempotent reuse through MCP to an ephemeral Studio, including one forbidden denial, while keeping the existing list/passes/capabilities checks. Co-authored-by: Anthony Thompson <github@trackdub.com>
|
Deployment failed for project olive-studio with the following error: Learn More: https://vercel.com/trackdub?upgradeToPro=build-rate-limit |
📝 WalkthroughWalkthroughThe smoke script now runs an isolated local Studio environment, executes asynchronous MCP capability and job-control checks, validates policy and terminal states, and centralizes cleanup for temporary files, processes, and settings. ChangesMCP smoke validation
Estimated code review effort: 4 (Complex) | ~45 minutes Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 8✅ Passed checks (8 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 Generate docstrings
🧪 Generate unit tests (beta)
✨ Simplify code
Warning Review ran into problems🔥 ProblemsLinked repositories: Public OSS repositories can only analyze public repositories installed in this organization. Analyzed Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 11
🤖 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 `@scripts/mcp-agent-smoke.mjs`:
- Around line 333-358: Extend the concurrent submission coverage around
callToolAsync so a third request uses a distinct idempotency_key while retaining
the same recipe fingerprint. Include its result in the success and job identity
assertions, then validate the Studio contract’s expected single-job reuse or
explicit conflict outcome for the different-key case.
- Around line 261-275: Coordinate teardown across scripts/mcp-agent-smoke.mjs:
in stopStudio, await the child exit event and keep the SIGKILL escalation timer
referenced; at lines 139-142, replace process.exit with throwing so the outer
catch invokes cleanup; in cleanup at lines 404-421, make it idempotent, await
stopStudio, and install SIGINT, SIGTERM, and SIGHUP handlers that run cleanup
before exiting.
- Around line 180-183: Update the timeout handler around the child process timer
to send SIGTERM first, then schedule a short-delay SIGKILL escalation if the
process remains alive, matching the existing stopStudio behavior. Ensure the
escalation is cleared when the child exits and preserve the immediate timeout
rejection.
- Around line 379-400: Update the cancellation verification around the fallback
and final fetches in the smoke flow to poll olive.get_optimization_job until
terminal === true or a bounded deadline expires, rather than asserting a single
response. Reuse the polling result for the final cancelled-state validation, and
align the comment near the fallback with the implemented condition so terminal
failures are either explicitly accepted or no longer described as counting.
- Around line 172-177: Update the child-process environment construction in the
spawn flow to default opts.env to process.env, matching the existing run
function behavior. Preserve any explicitly supplied environment overrides while
ensuring omitted opts.env inherits the parent process environment.
- Around line 147-155: Update parseJsonPayload to scan candidate “{” and “[”
offsets from the beginning of stdout, attempting JSON.parse on each suffix and
selecting the first candidate that parses successfully through the end of the
text. Remove the lastIndexOf-based start selection while preserving the existing
empty-output and no-JSON error handling.
- Around line 101-119: Update readStudioDiskConfig to distinguish a missing
configuration file from read or parse failures: return an empty object only when
STUDIO_CONFIG_PATH does not exist, and otherwise propagate the error. Keep
patchAgentAccessDisk unchanged so it cannot overwrite an existing malformed or
unreadable configuration with a partial default.
- Line 3: Resolve the mismatch between the header comment and behavior in the
script: either add a non-blocking failure gate around the exit paths in the main
execution flow and the raw npx-status handling, or revise the header to state
that non-blocking behavior is provided by the specific workflow that enforces
it. If revising the header, name that workflow explicitly and remove the
unsupported claim.
- Around line 229-243: Update waitForStudio to monitor the spawned Studio child
process and immediately reject when it exits, including its exit code in the
error; clean up the exit listener once readiness succeeds or the timeout is
reached. Add an AbortController-based timeout to each fetch attempt so hung
requests are bounded and the loop can continue until the overall deadline.
- Around line 246-255: Update the spawn options in startStudio to set shell:
true so the Windows pnpm.cmd shim executes correctly, matching the existing npx
spawn behavior while preserving the current command, environment, working
directory, and stdio settings.
- Around line 423-446: Move the three pre-runJobControlSmoke checks—list,
get_olive_passes, and get_mcp_capabilities—into runJobControlSmoke and invoke
each with studioEnv. Preserve their existing arguments and timeout settings,
while ensuring they no longer use the default process.env.
🪄 Autofix
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: d0b4882f-6e54-4eba-b00e-eb95b95a1c95
📒 Files selected for processing (1)
scripts/mcp-agent-smoke.mjs
🔗 Linked repositories identified
CodeRabbit considers these linked repositories for cross-repo context during reviews:
tonythethompson/QuickShell(manual)tonythethompson/numan(manual)tonythethompson/dependency-chain-substrate(manual)
📜 Review details
⏰ Context from checks skipped due to timeout. (3)
- GitHub Check: validate
- GitHub Check: python-tests
- GitHub Check: olive-pass-availability
⚠️ CI failures not shown inline (1)
Commit Status: Vercel: Vercel
Conclusion: failure
Deployment rate limited — retry in 24 hours.
🧰 Additional context used
📓 Path-based instructions (1)
**/*
📄 CodeRabbit inference engine (AGENTS.md)
Do not implement the listed backburner AI providers unless explicitly requested; prefer Custom or OpenAI-compatible providers for OpenAI-shaped hosts.
Files:
scripts/mcp-agent-smoke.mjs
🔍 Remote MCP Context7, DeepWiki, GitHub Copilot
Relevant review context
- The related Olive Studio PR
#171defines the contracts exercised by this smoke test: MCP calls Studio via loopback HTTP; job submission is policy-gated, asynchronous, idempotent, and cancellation uses/api/olive/agent/cancel. - Submission returns quickly with a
setting_upjob; cancellation during setup must leave the jobcancelledand prevent Olive from spawning. - Idempotency is scoped to MCP-origin jobs. Failed/cancelled jobs are retryable; completed jobs are reusable; a key reused with a different fingerprint should produce a conflict.
- An external review identified a remaining concurrency edge: submissions with different idempotency keys can select different locks despite sharing a fingerprint, potentially creating duplicate jobs. The proposed smoke coverage only tests reuse of the same key.
- The existing smoke script uses
npx --yes mcporter@0.13.0, an explicit--config,list --status, and JSON tool calls. Current mcporter documentation confirms explicit config support and thelist/calltimeout and machine-readable-output patterns., - The related PR’s current checks show core validation, Python tests, security, Docker, CodeQL, and Olive-pass checks succeeding, while CodeFactor is failing; no dedicated mcporter agent-smoke check appears in the listed check runs.
- GitHub and DeepWiki could not retrieve the specified
mta1124-1629472/Babel-Playerrepository, so repository-specific architectural context could not be verified.,
🔇 Additional comments (6)
scripts/mcp-agent-smoke.mjs (6)
2-56: LGTM!
62-76: LGTM!
78-99: LGTM!
217-227: LGTM!
277-280: LGTM!
312-320: 🗄️ Data Integrity & IntegrationNo change needed for the submit denial assertion.
POST /api/olive/jobs/submitreturnsok: false,error: "forbidden", and includesreasonfor disabled submission access, so the smoke assertion matches that contract.> Likely an incorrect or invalid review comment.
| /** | ||
| * Pinned mcporter canary smoke for Olive MCP (Phase 0). | ||
| * Pinned mcporter canary smoke for Olive MCP (Phase 0 + job-control path). | ||
| * Non-blocking CI by default — third-party CLI must not hard-break product PRs. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
The "Non-blocking CI by default" claim is not implemented.
Line 3 states the third-party CLI must not hard-break product PRs. The script has no such gate. Line 453 exits 1 on any failure, and Line 141 exits with the raw npx status. Any consumer that reads this header and wires the script into a required check gets a blocking job.
Either implement the gate, or correct the header to state that the non-blocking behavior is enforced by the workflow, and name the workflow.
♻️ Option: implement the documented gate
} catch (err) {
console.error("FAIL:", err instanceof Error ? err.message : err);
cleanup();
- process.exit(1);
+ // Non-blocking by default; set MCP_SMOKE_STRICT=1 to fail the job.
+ process.exit(process.env.MCP_SMOKE_STRICT === "1" ? 1 : 0);
}🤖 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 `@scripts/mcp-agent-smoke.mjs` at line 3, Resolve the mismatch between the
header comment and behavior in the script: either add a non-blocking failure
gate around the exit paths in the main execution flow and the raw npx-status
handling, or revise the header to state that non-blocking behavior is provided
by the specific workflow that enforces it. If revising the header, name that
workflow explicitly and remove the unsupported claim.
| function readStudioDiskConfig() { | ||
| try { | ||
| if (!existsSync(STUDIO_CONFIG_PATH)) return {}; | ||
| const parsed = JSON.parse(readFileSync(STUDIO_CONFIG_PATH, "utf8")); | ||
| if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; | ||
| return parsed; | ||
| } catch { | ||
| return {}; | ||
| } | ||
| } | ||
|
|
||
| /** Patch agentAccess on disk — resolveAgentAccess re-reads each request (avoids PUT rate limit). */ | ||
| function patchAgentAccessDisk(patch) { | ||
| const cfg = readStudioDiskConfig(); | ||
| cfg.agentAccess = { ...(cfg.agentAccess || {}), ...patch }; | ||
| mkdirSync(path.dirname(STUDIO_CONFIG_PATH), { recursive: true }); | ||
| writeFileSync(STUDIO_CONFIG_PATH, JSON.stringify(cfg, null, 2), "utf8"); | ||
| return cfg.agentAccess; | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not overwrite .olive-studio/config.json after a silent read failure.
readStudioDiskConfig returns {} for any read or parse error, including a file that exists but is malformed or temporarily unreadable. patchAgentAccessDisk then writes that empty object back and drops every other Studio setting on disk. That is a destructive local-state loss, not a test-only side effect.
If the file exists, fail fast instead of defaulting to {}.
🛡️ Proposed fix
function readStudioDiskConfig() {
- try {
- if (!existsSync(STUDIO_CONFIG_PATH)) return {};
- const parsed = JSON.parse(readFileSync(STUDIO_CONFIG_PATH, "utf8"));
- if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {};
- return parsed;
- } catch {
- return {};
- }
+ if (!existsSync(STUDIO_CONFIG_PATH)) return {};
+ let parsed;
+ try {
+ parsed = JSON.parse(readFileSync(STUDIO_CONFIG_PATH, "utf8"));
+ } catch (e) {
+ throw new Error(
+ `refusing to patch unreadable Studio config at ${STUDIO_CONFIG_PATH}: ${
+ e instanceof Error ? e.message : e
+ }`,
+ );
+ }
+ if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) {
+ throw new Error(`unexpected Studio config shape at ${STUDIO_CONFIG_PATH}`);
+ }
+ return parsed;
}📝 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.
| function readStudioDiskConfig() { | |
| try { | |
| if (!existsSync(STUDIO_CONFIG_PATH)) return {}; | |
| const parsed = JSON.parse(readFileSync(STUDIO_CONFIG_PATH, "utf8")); | |
| if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) return {}; | |
| return parsed; | |
| } catch { | |
| return {}; | |
| } | |
| } | |
| /** Patch agentAccess on disk — resolveAgentAccess re-reads each request (avoids PUT rate limit). */ | |
| function patchAgentAccessDisk(patch) { | |
| const cfg = readStudioDiskConfig(); | |
| cfg.agentAccess = { ...(cfg.agentAccess || {}), ...patch }; | |
| mkdirSync(path.dirname(STUDIO_CONFIG_PATH), { recursive: true }); | |
| writeFileSync(STUDIO_CONFIG_PATH, JSON.stringify(cfg, null, 2), "utf8"); | |
| return cfg.agentAccess; | |
| } | |
| function readStudioDiskConfig() { | |
| if (!existsSync(STUDIO_CONFIG_PATH)) return {}; | |
| let parsed; | |
| try { | |
| parsed = JSON.parse(readFileSync(STUDIO_CONFIG_PATH, "utf8")); | |
| } catch (e) { | |
| throw new Error( | |
| `refusing to patch unreadable Studio config at ${STUDIO_CONFIG_PATH}: ${ | |
| e instanceof Error ? e.message : e | |
| }`, | |
| ); | |
| } | |
| if (!parsed || typeof parsed !== "object" || Array.isArray(parsed)) { | |
| throw new Error(`unexpected Studio config shape at ${STUDIO_CONFIG_PATH}`); | |
| } | |
| return parsed; | |
| } | |
| /** Patch agentAccess on disk — resolveAgentAccess re-reads each request (avoids PUT rate limit). */ | |
| function patchAgentAccessDisk(patch) { | |
| const cfg = readStudioDiskConfig(); | |
| cfg.agentAccess = { ...(cfg.agentAccess || {}), ...patch }; | |
| mkdirSync(path.dirname(STUDIO_CONFIG_PATH), { recursive: true }); | |
| writeFileSync(STUDIO_CONFIG_PATH, JSON.stringify(cfg, null, 2), "utf8"); | |
| return cfg.agentAccess; | |
| } |
🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 101 - 119, Update
readStudioDiskConfig to distinguish a missing configuration file from read or
parse failures: return an empty object only when STUDIO_CONFIG_PATH does not
exist, and otherwise propagate the error. Keep patchAgentAccessDisk unchanged so
it cannot overwrite an existing malformed or unreadable configuration with a
partial default.
| function parseJsonPayload(stdout) { | ||
| const text = (stdout || "").trim(); | ||
| if (!text) throw new Error("empty mcporter stdout"); | ||
| const startObj = text.lastIndexOf("{"); | ||
| const startArr = text.lastIndexOf("["); | ||
| const start = Math.max(startObj, startArr); | ||
| if (start < 0) throw new Error(`no JSON in mcporter stdout: ${text.slice(0, 200)}`); | ||
| return JSON.parse(text.slice(start)); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
lastIndexOf selects an inner brace, not the payload start.
For any nested payload, text.lastIndexOf("{") points at the innermost object. JSON.parse(text.slice(start)) then either throws or silently returns the wrong nested fragment. Example: for {"ok":true,"result":{"job_id":"x"}} this parses {"job_id":"x"} and every top-level assertion in runJobControlSmoke reads undefined.
Scan candidate start offsets and take the first one that parses to the end of the text.
🐛 Proposed fix
function parseJsonPayload(stdout) {
const text = (stdout || "").trim();
if (!text) throw new Error("empty mcporter stdout");
- const startObj = text.lastIndexOf("{");
- const startArr = text.lastIndexOf("[");
- const start = Math.max(startObj, startArr);
- if (start < 0) throw new Error(`no JSON in mcporter stdout: ${text.slice(0, 200)}`);
- return JSON.parse(text.slice(start));
+ for (let i = 0; i < text.length; i += 1) {
+ const ch = text[i];
+ if (ch !== "{" && ch !== "[") continue;
+ try {
+ return JSON.parse(text.slice(i));
+ } catch {
+ /* not a complete document at this offset */
+ }
+ }
+ throw new Error(`no JSON in mcporter stdout: ${text.slice(0, 200)}`);
}📝 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.
| function parseJsonPayload(stdout) { | |
| const text = (stdout || "").trim(); | |
| if (!text) throw new Error("empty mcporter stdout"); | |
| const startObj = text.lastIndexOf("{"); | |
| const startArr = text.lastIndexOf("["); | |
| const start = Math.max(startObj, startArr); | |
| if (start < 0) throw new Error(`no JSON in mcporter stdout: ${text.slice(0, 200)}`); | |
| return JSON.parse(text.slice(start)); | |
| } | |
| function parseJsonPayload(stdout) { | |
| const text = (stdout || "").trim(); | |
| if (!text) throw new Error("empty mcporter stdout"); | |
| for (let i = 0; i < text.length; i += 1) { | |
| const ch = text[i]; | |
| if (ch !== "{" && ch !== "[") continue; | |
| try { | |
| return JSON.parse(text.slice(i)); | |
| } catch { | |
| /* not a complete document at this offset */ | |
| } | |
| } | |
| throw new Error(`no JSON in mcporter stdout: ${text.slice(0, 200)}`); | |
| } |
🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 147 - 155, Update parseJsonPayload
to scan candidate “{” and “[” offsets from the beginning of stdout, attempting
JSON.parse on each suffix and selecting the first candidate that parses
successfully through the end of the text. Remove the lastIndexOf-based start
selection while preserving the existing empty-output and no-JSON error handling.
| return new Promise((resolve, reject) => { | ||
| const child = spawn("npx", full, { | ||
| cwd: root, | ||
| env: { ...opts.env }, | ||
| shell: process.platform === "win32", | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Default env to process.env, as run does.
env: { ...opts.env } yields {} when the caller omits opts.env. The npx child then starts with no PATH and no HOME, and the spawn fails. run at Line 127 defaults to process.env. Match that behavior.
🐛 Proposed fix
const child = spawn("npx", full, {
cwd: root,
- env: { ...opts.env },
+ env: { ...(opts.env ?? process.env) },
shell: process.platform === "win32",
});📝 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.
| return new Promise((resolve, reject) => { | |
| const child = spawn("npx", full, { | |
| cwd: root, | |
| env: { ...opts.env }, | |
| shell: process.platform === "win32", | |
| }); | |
| return new Promise((resolve, reject) => { | |
| const child = spawn("npx", full, { | |
| cwd: root, | |
| env: { ...(opts.env ?? process.env) }, | |
| shell: process.platform === "win32", | |
| }); |
🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 172 - 177, Update the child-process
environment construction in the spawn flow to default opts.env to process.env,
matching the existing run function behavior. Preserve any explicitly supplied
environment overrides while ensuring omitted opts.env inherits the parent
process environment.
| const timer = setTimeout(() => { | ||
| child.kill("SIGTERM"); | ||
| reject(new Error(`mcporter ${selector} timed out after ${timeoutMs}ms`)); | ||
| }, timeoutMs + 15_000); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win
Escalate to SIGKILL after the timeout SIGTERM.
child.kill("SIGTERM") targets npx. On timeout the promise rejects immediately, but npx and its Python grandchild can survive. The script then exits with an orphaned MCP server holding the temporary config. Add a short kill escalation, as stopStudio already does.
♻️ Proposed fix
const timer = setTimeout(() => {
child.kill("SIGTERM");
+ setTimeout(() => {
+ if (!child.killed) child.kill("SIGKILL");
+ }, 2000).unref?.();
reject(new Error(`mcporter ${selector} timed out after ${timeoutMs}ms`));
}, timeoutMs + 15_000);🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 180 - 183, Update the timeout
handler around the child process timer to send SIGTERM first, then schedule a
short-delay SIGKILL escalation if the process remains alive, matching the
existing stopStudio behavior. Ensure the escalation is cleared when the child
exits and preserve the immediate timeout rejection.
| function startStudio(port) { | ||
| const child = spawn("pnpm", ["exec", "tsx", "server.ts"], { | ||
| cwd: root, | ||
| env: { | ||
| ...process.env, | ||
| PORT: String(port), | ||
| // Do not set OLIVE_MCP_ALLOW_JOBS — deny path must be reachable via disk policy. | ||
| }, | ||
| stdio: ["ignore", "pipe", "pipe"], | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
Node.js spawn .cmd file ENOENT shell true Windows requirement
💡 Result:
The ENOENT (Error No Entity) error when spawning.cmd or.bat files on Windows occurs because Node.js child_process.spawn does not automatically resolve file extensions or search the PATH for batch scripts [1][2]. Unlike a standard Windows terminal, the native spawn mechanism (CreateProcess) requires an absolute path to an executable or a file with an explicit extension [2]. To resolve this issue, you can use one of the following approaches: 1. Enable the shell option: Setting { shell: true } in the options argument allows the command to run within the system's default shell (cmd.exe on Windows) [3][4]. This enables the shell to handle extension resolution and PATH lookup automatically [4]. Note that the Node.js documentation advises caution with this approach due to potential security risks if passing unsanitized user input [5][6], and some newer versions may flag this usage (e.g., DEP0190) [7][6]. 2. Use the explicit file extension: If you know the command is a.cmd or.bat file, include the extension directly in the command string (e.g., spawn('npm.cmd', [...])) [1][8]. This avoids the overhead and security concerns of spawning a full shell [4]. 3. Use a cross-platform helper: The industry standard for handling these inconsistencies is to use a package like cross-spawn [4]. It is a drop-in replacement for the native child_process.spawn that automatically handles Windows path resolution, extension appending, and other platform-specific quirks, allowing you to write your code once for all operating systems [4]. Avoid manual cross-platform conditional logic (e.g., if/else checks for process.platform) where possible, as it increases code complexity and is prone to errors [4]. Always ensure the executable or script is available in the system PATH if not providing an absolute path [9][4].
Citations:
- 1: https://stackoverflow.com/questions/17516772/using-nodejss-spawn-causes-unknown-option-and-error-spawn-enoent-err
- 2: https://fixclaw.dev/troubleshooting/windows-spawn-enoent-node.js-child_process.spawn-fails-to-resolve-.cmd-cli-shims/
- 3: https://nodejs.org/docs/latest-v22.x/api/child_process.html
- 4: https://errornotes.dev/en/errors/nodejs/fixing-error-spawn-enoent-in-nodejs-childprocess
- 5: https://nodejs.org/download/release/v25.5.0/docs/api/child_process.html
- 6: https://beta.docs.nodejs.org/child_process.html
- 7: https://nodejs.org/api/child_process.html
- 8: https://stackoverflow.com/questions/43230346/error-spawn-npm-enoent
- 9: https://stackoverflow.com/questions/27688804/how-do-i-debug-error-spawn-enoent-on-node-js
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## file outline"
ast-grep outline scripts/mcp-agent-smoke.mjs || true
echo
echo "## relevant sections"
nl -ba scripts/mcp-agent-smoke.mjs | sed -n '1,80p'
nl -ba scripts/mcp-agent-smoke.mjs | sed -n '230,270p'
echo
echo "## spawn occurrences in file"
rg -n "spawn\\(" scripts/mcp-agent-smoke.mjs -C 2Repository: tonythethompson/Olive-Studio
Length of output: 1085
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## file size"
wc -l scripts/mcp-agent-smoke.mjs
echo
echo "## relevant sections"
python3 - <<'PY'
from pathlib import Path
p = Path("scripts/mcp-agent-smoke.mjs")
lines = p.read_text().splitlines()
for start, end in [(1, 90), (228, 275)]:
print(f"--- {start}-{end} ---")
for n in range(start, min(end, len(lines))+1):
print(f"{n:4d}\t{lines[n-1]}")
PY
echo
echo "## spawn occurrences in file"
python3 - <<'PY'
import re
from pathlib import Path
p = Path("scripts/mcp-agent-smoke.mjs")
for i, line in enumerate(p.read_text().splitlines(), 1):
if "spawn(" in line:
print(f"--- line {i} ---")
for start in range(max(1, i-3), min(len(line lines:=p.read_text().splitlines()))+1, i+4):
print(f"{start:4d}\t{lines[start-1]}")
PYRepository: tonythethompson/Olive-Studio
Length of output: 5169
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "## spawn/Sync occurrences and nearby options"
python3 - <<'PY'
from pathlib import Path
p = Path("scripts/mcp-agent-smoke.mjs")
lines = p.read_text().splitlines()
for i, line in enumerate(lines, 1):
if "spawn(" in line or "spawnSync(" in line:
print(f"--- line {i} ---")
for n in range(i-10 if i>10 else 1, min(i+20, len(lines))+1):
print(f"{n:4d}\t{lines[n-1]}")
PY
echo
echo "## package node engines"
if [ -f package.json ]; then jq ".engines.packageManager, .engines.node" package.json; fi
echo
echo "## deterministic shell/platform lookup comparison"
node - <<'JS'
try {
console.log("platform", process.platform);
const candidates = ["pnpm", "pnpm.cmd"];
for (const name of candidates) {
try {
const result = spawnSync("node", ["-e", "require('child_process').execSync('node -v'); console.log(process.chdir('C:/Windows/System32')); process.exit(0);"], {shell: false});
console.log("spawnSync pnpm without shell", { status: result.status, stderr: result.stderr.toString().slice(0, 120) });
} catch (e) {
console.log("spawnSync pnpm without shell", e.code, e.message.slice(0, 120));
}
}
} catch (e) {
console.error("probe failed", e.message);
}
JSRepository: tonythethompson/Olive-Studio
Length of output: 3881
🌐 Web query:
Node.js child_process.spawn Windows .cmd shell option Node 22 pnpm.cmd
💡 Result:
When using Node.js 22 (or other versions containing the security fix for CVE-2024-27980) on Windows, attempting to directly spawn a.cmd or.bat file—such as pnpm.cmd—using child_process.spawn without the shell option will cause the process to throw an EINVAL error [1][2]. To resolve this and successfully execute pnpm.cmd, you must pass the shell: true option in the options object [1][2]: const { spawn } = require('child_process'); const child = spawn('pnpm.cmd', ['install'], { shell: true }); Key considerations when using the shell option: 1. Security Warning: Setting shell: true enables shell execution, which exposes your application to arbitrary command injection if you pass unsanitized user input to the command or its arguments [3][4][5]. Always sanitize inputs if they come from untrusted sources. 2. Behavior and Limitations: - The shell option executes the command via the default Windows command processor, which is typically cmd.exe (as defined by process.env.ComSpec) [3][5]. - Because it invokes a shell, behavior regarding argument passing, quoting, and output piping may differ from standard execution. For example, some users have reported hanging issues or unexpected shell output when using { shell: true } [6]. - Libraries like cross-spawn are frequently used in the Node.js ecosystem as a more robust, cross-platform alternative to native child_process.spawn, as they handle platform-specific executable resolution (e.g., automatically detecting.cmd/.bat shims) without requiring you to manually set shell: true or construct platform-specific code [7][8]. If you encounter issues with shell: true or prefer to avoid it, you can manually target the executable or use cross-spawn to abstract the complexity of Windows path and executable resolution [7][9].
Citations:
- 1: Spawning some commands on windows fails with error EINVAL in Node 20 LTS (20.12.2) only nodejs/node#52681
- 2: Windows build fails with spawn EINVAL after Node.js security fix for CVE-2024-27980 (batch/.cmd spawning now requires { shell: true }) electron-userland/electron-builder#9488
- 3: https://nodejs.org/docs/latest-v24.x/api/child_process.html
- 4: https://beta.docs.nodejs.org/child_process.html
- 5: https://nodejs.org/api/child_process.html
- 6: [BUG]
npm.ps1hangs indefinitely when invoked via Node.jsspawn({ shell: true })on Windows npm/cli#8259 - 7: fix(app-builder): use cross-spawn for pnpm on Windows cursor/cookbook#9
- 8: https://stackoverflow.com/questions/43230346/error-spawn-npm-enoent
- 9: https://github.com/glacayo/website-multipages/blob/main/packages/create-contractor-site/src/run-command.mjs
Set shell for the Windows pnpm spawn.
On Windows, resolving pnpm starts pnpm.cmd, which requires shell: true so the shell can execute the .cmd shim. This already appears for npx in this script, and resolvePython() targets Windows venv paths too.
🐛 Proposed fix
const child = spawn("pnpm", ["exec", "tsx", "server.ts"], {
cwd: root,
+ shell: process.platform === "win32",
env: {📝 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.
| function startStudio(port) { | |
| const child = spawn("pnpm", ["exec", "tsx", "server.ts"], { | |
| cwd: root, | |
| env: { | |
| ...process.env, | |
| PORT: String(port), | |
| // Do not set OLIVE_MCP_ALLOW_JOBS — deny path must be reachable via disk policy. | |
| }, | |
| stdio: ["ignore", "pipe", "pipe"], | |
| }); | |
| function startStudio(port) { | |
| const child = spawn("pnpm", ["exec", "tsx", "server.ts"], { | |
| cwd: root, | |
| shell: process.platform === "win32", | |
| env: { | |
| ...process.env, | |
| PORT: String(port), | |
| // Do not set OLIVE_MCP_ALLOW_JOBS — deny path must be reachable via disk policy. | |
| }, | |
| stdio: ["ignore", "pipe", "pipe"], | |
| }); |
🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 246 - 255, Update the spawn options
in startStudio to set shell: true so the Windows pnpm.cmd shim executes
correctly, matching the existing npx spawn behavior while preserving the current
command, environment, working directory, and stdio settings.
| function stopStudio(child) { | ||
| if (!child || child.killed) return; | ||
| try { | ||
| child.kill("SIGTERM"); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| setTimeout(() => { | ||
| try { | ||
| if (!child.killed) child.kill("SIGKILL"); | ||
| } catch { | ||
| /* ignore */ | ||
| } | ||
| }, 2000).unref?.(); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Process termination is not coordinated with cleanup completion. Three sites share one root cause: the script exits before or without finishing teardown, so the Studio child, the temporary directory, and the patched agentAccess state can all survive a run.
scripts/mcp-agent-smoke.mjs#L261-L275: makestopStudioawait the childexitevent and use a non-unref'd escalation timer, soSIGKILLcan actually fire before the process exits.scripts/mcp-agent-smoke.mjs#L139-L142: throw instead of callingprocess.exit, so the outercatchrunscleanup().scripts/mcp-agent-smoke.mjs#L404-L421: makecleanupidempotent,await stopStudio, and registerSIGINT/SIGTERM/SIGHUPhandlers that run it before exit.
📍 Affects 1 file
scripts/mcp-agent-smoke.mjs#L261-L275(this comment)scripts/mcp-agent-smoke.mjs#L139-L142scripts/mcp-agent-smoke.mjs#L404-L421
🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 261 - 275, Coordinate teardown
across scripts/mcp-agent-smoke.mjs: in stopStudio, await the child exit event
and keep the SIGKILL escalation timer referenced; at lines 139-142, replace
process.exit with throwing so the outer catch invokes cleanup; in cleanup at
lines 404-421, make it idempotent, await stopStudio, and install SIGINT,
SIGTERM, and SIGHUP handlers that run cleanup before exiting.
| // Concurrent submits: mcporter cold-starts are slow enough that a sequential | ||
| // replay can miss reuse after a fast failed setup. The Studio MCP lock still | ||
| // serializes check-then-act into one job_id + reused:true. | ||
| const [first, second] = await Promise.all([ | ||
| callToolAsync("olive.submit_optimization_job", submitArgs, { | ||
| timeoutMs: 120_000, | ||
| env: studioEnv, | ||
| }), | ||
| callToolAsync("olive.submit_optimization_job", submitArgs, { | ||
| timeoutMs: 120_000, | ||
| env: studioEnv, | ||
| }), | ||
| ]); | ||
| if (!first.ok || !second.ok) { | ||
| throw new Error(`submit failed: ${JSON.stringify({ first, second })}`); | ||
| } | ||
| if (first.job_id !== second.job_id) { | ||
| throw new Error( | ||
| `idempotency split jobs: ${JSON.stringify({ first, second })}`, | ||
| ); | ||
| } | ||
| if (!first.reused && !second.reused) { | ||
| throw new Error( | ||
| `expected one reused submit, got ${JSON.stringify({ first, second })}`, | ||
| ); | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick win
Add coverage for the same-fingerprint, different-key case.
This block only exercises reuse of one identical idempotency_key. The known concurrency edge is different: two submissions with different keys but the same recipe fingerprint can select different locks and create duplicate jobs. The current assertions cannot detect that.
Add a third concurrent submit with a distinct key and the same recipe, then assert the expected outcome (single job, or an explicit conflict, whichever the Studio contract defines).
🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 333 - 358, Extend the concurrent
submission coverage around callToolAsync so a third request uses a distinct
idempotency_key while retaining the same recipe fingerprint. Include its result
in the success and job identity assertions, then validate the Studio contract’s
expected single-job reuse or explicit conflict outcome for the different-key
case.
| if (!cancelled.ok || cancelled.status !== "cancelled") { | ||
| // Already terminal-failed before cancel still counts if status is terminal. | ||
| const after = await callToolAsync( | ||
| "olive.get_optimization_job", | ||
| { job_id: jobId }, | ||
| { timeoutMs: 60_000, env: studioEnv }, | ||
| ); | ||
| if (!(after.terminal === true && after.status === "cancelled")) { | ||
| throw new Error( | ||
| `expected cancel, got ${JSON.stringify({ cancelled, after })}`, | ||
| ); | ||
| } | ||
| } | ||
|
|
||
| const terminal = await callToolAsync( | ||
| "olive.get_optimization_job", | ||
| { job_id: jobId }, | ||
| { timeoutMs: 60_000, env: studioEnv }, | ||
| ); | ||
| if (terminal.status !== "cancelled" || terminal.terminal !== true) { | ||
| throw new Error(`expected terminal cancelled, got ${JSON.stringify(terminal)}`); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Poll for the terminal state instead of asserting it once.
Cancellation during setting_up is asynchronous. Both the fallback fetch on Line 381 and the final fetch on Line 393 read the status exactly once with no wait. The job can still be transitioning, so this smoke is flaky in CI.
Poll olive.get_optimization_job until terminal === true or a deadline expires.
The comment on Line 380 also contradicts the code. It says a terminal failure "still counts", but Line 386 requires status === "cancelled". Fix the comment or the condition.
♻️ Proposed fix
+async function waitForTerminal(jobId, env, timeoutMs = 60_000) {
+ const deadline = Date.now() + timeoutMs;
+ let last = null;
+ while (Date.now() < deadline) {
+ last = await callToolAsync(
+ "olive.get_optimization_job",
+ { job_id: jobId },
+ { timeoutMs: 60_000, env },
+ );
+ if (last.terminal === true) return last;
+ await new Promise((r) => setTimeout(r, 500));
+ }
+ throw new Error(`job ${jobId} not terminal: ${JSON.stringify(last)}`);
+}Then replace the single-shot fetches:
- const terminal = await callToolAsync(
- "olive.get_optimization_job",
- { job_id: jobId },
- { timeoutMs: 60_000, env: studioEnv },
- );
- if (terminal.status !== "cancelled" || terminal.terminal !== true) {
+ const terminal = await waitForTerminal(jobId, studioEnv);
+ if (terminal.status !== "cancelled") {
throw new Error(`expected terminal cancelled, got ${JSON.stringify(terminal)}`);
}🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 379 - 400, Update the cancellation
verification around the fallback and final fetches in the smoke flow to poll
olive.get_optimization_job until terminal === true or a bounded deadline
expires, rather than asserting a single response. Reuse the polling result for
the final cancelled-state validation, and align the comment near the fallback
with the implemented condition so terminal failures are either explicitly
accepted or no longer described as counting.
| try { | ||
| run(["list", "olive", "--status", "--timeout", "60000"]); | ||
| run([ | ||
| "call", | ||
| "olive.get_mcp_capabilities", | ||
| "olive.get_olive_passes", | ||
| "filter=quantization", | ||
| "--timeout", | ||
| "90000", | ||
| "60000", | ||
| "--output", | ||
| "json", | ||
| ], | ||
| 120_000, | ||
| ); | ||
| console.log("PASS: pinned mcporter agent smoke"); | ||
| ]); | ||
| run( | ||
| [ | ||
| "call", | ||
| "olive.get_mcp_capabilities", | ||
| "--timeout", | ||
| "90000", | ||
| "--output", | ||
| "json", | ||
| ], | ||
| { timeoutMs: 120_000 }, | ||
| ); | ||
|
|
||
| await runJobControlSmoke(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Determine which MCP tools depend on OLIVE_STUDIO_API_URL.
set -euo pipefail
echo "=== OLIVE_STUDIO_API_URL readers ==="
rg -n -C 5 'OLIVE_STUDIO_API_URL' --glob '!**/node_modules/**'
echo "=== capability / passes tool definitions ==="
rg -n -C 15 'get_mcp_capabilities|get_olive_passes' --glob '*.py'Repository: tonythethompson/Olive-Studio
Length of output: 203
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== Relevant file outline/stat ==="
wc -l scripts/mcp-agent-smoke.mjs
ast-grep outline scripts/mcp-agent-smoke.mjs --view compact || true
echo "=== Lines 360-470 ==="
sed -n '360,470p' scripts/mcp-agent-smoke.mjs | cat -n | sed 's/^/4:/'
echo "=== Studio env/runJobControlSmoke references ==="
rg -n -C 4 'runJobControlSmoke|studioEnv|OLIVE_STUDIO_API_URL|run\(' scripts/mcp-agent-smoke.mjsRepository: tonythethompson/Olive-Studio
Length of output: 7841
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== lines 1-150 ==="
sed -n '1,150p' scripts/mcp-agent-smoke.mjs | cat -n | sed 's/^/4:/'
echo "=== lines 150-310 ==="
sed -n '150,310p' scripts/mcp-agent-smoke.mjs | cat -n | sed 's/^/4:/'
echo "=== list olive references in repo ==="
rg -n -C 4 'list .*olive|olive\.get_olive_passes|olive\.get_mcp_capabilities|get_mcp_capabilities|get_olive_passes|mcp_capabilities|olive_passes' --glob '!**/node_modules/**'Repository: tonythethompson/Olive-Studio
Length of output: 12391
🔍 Additional research query:
MCPorter mcp-agent-smoke Olive get_olive_passes get_mcp_capabilities list olive Studio OLIVE_STUDIO_API_URL
💡 Result:
Findings
- MCPorter is a TypeScript runtime/CLI for discovering and calling MCP servers. Use
mcporter listto inspect configured servers and tools. - The repository does not document or implement
mcp-agent-smoke, Olive,get_olive_passes,get_mcp_capabilities,Studio, orOLIVE_STUDIO_API_URL. - GitHub code search found no matches for the Olive-related names.
I cannot directly invoke Olive or inspect OLIVE_STUDIO_API_URL from the available tools.
🌐 Web query:
mcporter list --status get_olive_passes get_mcp_capabilities OLIVE_STUDIO_API_URL
💡 Result:
The command mcporter list is a tool used for discovering and inspecting configured Model Context Protocol (MCP) servers [1][2]. When you run mcporter list followed by a server name (such as OLIVE_STUDIO_API_URL, assuming it is a configured server), the command outputs TypeScript-style signatures for the tools available on that server, along with doc comments, optional summaries, and any instructions returned during MCP initialization [1][3]. Regarding your specific input: 1. --status: When you add --status to the command (e.g., mcporter list [server] --status), it prints only a concise status row for the specified server, excluding the detailed tool documentation and signatures [1][3]. 2. get_olive_passes and get_mcp_capabilities: These appear to be the names of specific tools hosted on an MCP server (likely the one represented by the OLIVE_STUDIO_API_URL) [1][4]. 3. OLIVE_STUDIO_API_URL: In this context, this is intended to be the identifier or URL of the MCP server you wish to query [1][5]. To inspect these specific tools on that server, you would typically use the command: mcporter list OLIVE_STUDIO_API_URL If you only want to check the connection status of the server without listing all tools, you would use: mcporter list OLIVE_STUDIO_API_URL --status If you want to view the schema or details for the specific tools (get_olive_passes or get_mcp_capabilities), you can target them directly using the syntax: mcporter list OLIVE_STUDIO_API_URL.get_olive_passes mcporter list OLIVE_STUDIO_API_URL.get_mcp_capabilities Please note that for these commands to work, the server must either be pre-configured in your mcporter settings or provided as an ad-hoc server using flags like --http-url [4][6].
Citations:
- 1: https://mcporter.sh/cli-reference.html
- 2: https://github.com/openclaw/mcporter/
- 3: https://github.com/openclaw/mcporter/blob/main/docs/cli-reference.md
- 4: https://github.com/steipete/mcporter/blob/main/README.md
- 5: https://github.com/openclaw/mcporter/blob/main/docs/call-syntax.md
- 6: https://mcporter.sh/adhoc.html
🌐 Web query:
"OLIVE_STUDIO_API_URL" "get_mcp_capabilities"
💡 Result:
The term get_mcp_capabilities is not a standard part of the Model Context Protocol (MCP) specification nor a specific API endpoint associated with a variable named OLIVE_STUDIO_API_URL. Instead, get_mcp_capabilities appears as a tool or method name implemented by various independent developers within their own MCP servers or agent frameworks [1][2][3][4][5]. 1. MCP Specification: The official MCP protocol uses an initialize method during the initial connection phase to negotiate protocol versions and exchange capabilities between clients and servers [6]. It does not define a get_mcp_capabilities method for runtime tool use [6]. 2. Custom Implementations: Several third-party projects and repositories implement a custom tool or method named get_mcp_capabilities to allow AI agents to query the specific features, capabilities, or version information of a connected MCP server at runtime [1][3][4][5]. 3. Olive Studio / Olive: While there is a Microsoft Olive project that includes an MCP server for model optimization [7], and a separate project called Olive Studio [8] (as well as unrelated projects like OliveTin [9][10] and Open Live Studio [11]), none of these define a standard OLIVE_STUDIO_API_URL environment variable associated with a get_mcp_capabilities command. If you are working with a specific software implementation, this functionality is likely a custom tool defined within that project's codebase rather than a native feature of the MCP standard. To use such a tool, you should consult the documentation or source code of the specific MCP server you are interacting with to confirm how it exposes its capability map [5].
Citations:
- 1: https://github.com/volt-hq/Volt/blob/main/packages/coding-agent/docs/mcp.md
- 2: https://vcoderun.github.io/acpkit/bridges/
- 3: https://github.com/neerajadhav/kai-stock-market-mcp
- 4: https://github.com/deswong/Openhab-MCP
- 5: https://crosstrade.io/docs/api/mcp-trading/tools/get-mcp-capabilities
- 6: https://docs.getmcp.com/api-reference/mcp-protocol/initialize
- 7: https://microsoft.github.io/Olive/blogs/olive-mcp-server.html
- 8: https://github.com/tonythethompson/Olive-Studio
- 9: https://docs.olivetin.app/api/intro.html
- 10: https://docs.olivetin.app/security/api_keys.html
- 11: Eyevinn/open-live-studio@5dcb755
🌐 Web query:
mcporter 0.13.0 list --status call oliveserver call --env
💡 Result:
In the mcporter CLI (version 0.13.0), the syntax you provided appears to be a mix of separate commands rather than a single valid command [1][2]. Based on the mcporter documentation, here is how the components you listed function: 1. mcporter list: This command is used to list configured MCP servers [2][3]. The --status flag is a valid option when used with a specific server target to print only the concise health status row instead of full tool documentation [2][3]. 2. call: This is a separate subcommand (mcporter call) used to execute a tool from an MCP server [2][4]. It is not a flag for the list command [2]. 3. --env: The mcporter CLI uses environment variable interpolation (e.g., ${ENV}) within its configuration files (such as ~/.mcporter/mcporter.json) [5], but there is no --env flag for the list or call commands in the official CLI reference [2][3]. If you are trying to check the status of a specific server (e.g., oliveserver) and then call a tool, you would execute these as two separate commands: # Check the status of oliveserver mcporter list oliveserver --status # Call a tool on oliveserver mcporter call oliveserver.tool_name For more details, you can refer to the official CLI reference at https://mcporter.sh/cli-reference.html [2].
Citations:
- 1: https://newreleases.io/project/github/openclaw/mcporter/release/v0.13.0
- 2: https://mcporter.sh/cli-reference.html
- 3: https://github.com/steipete/mcporter/blob/main/docs/cli-reference.md
- 4: https://mcporter.sh/quickstart.html
- 5: https://mcporter.sh/
Gate the pre-runJobControlSmoke checks with studioEnv.
run(["list", "olive", ...]), run(["call", "olive.get_olive_passes", ...]), and run(["call", "olive.get_mcp_capabilities", ...]) all pass opts.env, which defaults to process.env. Move these checks into runJobControlSmoke and call them with studioEnv so they hit the ephemeral Studio instead of any ambient Studio configuration.
🤖 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 `@scripts/mcp-agent-smoke.mjs` around lines 423 - 446, Move the three
pre-runJobControlSmoke checks—list, get_olive_passes, and
get_mcp_capabilities—into runJobControlSmoke and invoke each with studioEnv.
Preserve their existing arguments and timeout settings, while ensuring they no
longer use the default process.env.
Apply the still-valid #175 review fixes on the squashed stack: non-blocking MCP_SMOKE_STRICT gate, safe Studio config reads, robust JSON parse, callTool env/SIGKILL, Studio readiness + teardown coordination, distinct-key coverage, terminal polling, and Phase-0 checks under studioEnv. Co-authored-by: Anthony Thompson <github@trackdub.com>
Combines the tiny review PRs (#175–#183) into a single commit on MCP_harden: - Extend mcporter agent smoke for policy-gated job submit/status/cancel/reuse - Centralize semantic inflight drain in pytest conftest - resetJobRegistry between integration tests (children + MCP keys) - Harden MCP tool injection integration assertions - Restore OLIVE_MCP_ACCESS after mcpAccess suite - Hoist writeStudioConfig import in olive.stream tests - Deterministic QNN preflight host-mode asserts - Align jobRunner idempotency mocks (executable + python) - Lock MCP submits on fingerprint and idempotency key Co-authored-by: Anthony Thompson <github@trackdub.com>
Apply the still-valid #175 review fixes on the squashed stack: non-blocking MCP_SMOKE_STRICT gate, safe Studio config reads, robust JSON parse, callTool env/SIGKILL, Studio readiness + teardown coordination, distinct-key coverage, terminal polling, and Phase-0 checks under studioEnv. Co-authored-by: Anthony Thompson <github@trackdub.com>
2 participants
Summary
Extends
scripts/mcp-agent-smoke.mjsso the pinned mcporter canary covers MCP → Studio job control, while keeping the existinglist --status,get_olive_passes, andget_mcp_capabilitieschecks.What it validates
allowJobSubmission: false; MCPsubmit_optimization_jobreturnsforbiddenwith the expected reason.job_idandreused: true.get_optimization_jobduring setup,cancel_optimization_job, then terminalcancelled.Notes
setting_up).python3sopythonENOENT does not flake the canary..olive-studioagent access on disk (and restores it) to avoid burning the shared Olive run rate limit onPUT /agent-access.Validation
node scripts/mcp-agent-smoke.mjs # PASS: pinned mcporter agent smoke (incl. job policy path)