fix: policy_evaluate crash on missing/malformed request (#2894)

[ruvnet]

Fixes #2894 (finding 1 only — see below for findings 2 and 3, which are not addressed here).

What changed

policy_evaluate's handler cast input.request straight to PolicyRequest with no validation:

handler: async (input, context) => evaluatePolicyRequest(
  input.request as PolicyRequest,
  ...
),

evaluatePolicyRequest → AgenticPolicyEngine.evaluate() (in @claude-flow/security/src/policy/engine.ts) unconditionally reads request.requestId on line 1. When input.request is undefined — e.g. the issue's exact policy_evaluate {} repro, or arguments passed at the top level instead of nested under request — this crashes with a bare TypeError: Cannot read properties of undefined (reading 'requestId'), which the MCP dispatcher (src/mcp-server.ts) surfaces to clients as an opaque JSON-RPC -32603 — reproduced byte-for-byte against the issue's report.

The fix adds a small guard in the policy_evaluate handler (v3/@claude-flow/cli/src/mcp-tools/policy-tools.ts) that validates request/request.identity/request.action are present before calling evaluatePolicyRequest, turning a missing/malformed request into a clear, catchable validation Error instead of an internal crash.

Testing

New test file: v3/@claude-flow/cli/__tests__/policy-evaluate-requestId-2894.test.ts, calling the real policy_evaluate handler directly (no mocks).

cd v3/@claude-flow/cli
npx vitest run __tests__/policy-evaluate-requestId-2894.test.ts

• Pre-fix (verified via git stash): 2 of 3 tests fail — the handler throws the exact reported message "Cannot read properties of undefined (reading 'requestId')" for both policy_evaluate {} and the issue's "plausible arguments" repro ({action:'read', resource:'file'}, i.e. fields at the top level instead of under request).
• Post-fix: all 3 tests pass — missing/malformed request now throws a clear validation error (not the TypeError), and a well-formed request still evaluates normally.

Also ran npx tsc --noEmit: 467 pre-existing errors both before and after this change (confirmed via git stash comparison — all pre-existing, unrelated to this file, coming from unbuilt workspace packages in this fresh worktree). No new errors introduced.

Diff summary

• v3/@claude-flow/cli/src/mcp-tools/policy-tools.ts — added requireWellFormedPolicyRequest() guard, called at the top of the policy_evaluate handler (34 insertions / 4 deletions).
• v3/@claude-flow/cli/__tests__/policy-evaluate-requestId-2894.test.ts — new test file (regression coverage).

No CI/workflow, release-tooling, or unrelated files touched.

Findings NOT addressed here (human input needed)

Finding 2 — validation errors returned in-band, outer isError never set. Confirmed real and broader than the issue's framing: the stdio dispatcher (mcp-server.ts's handleMCPMessage, the actual code path for the reporter's npx -y @claude-flow/cli@latest + stdio setup) never sets isError at all on its tools/call success path, and there are two different, inconsistent error-signaling conventions already in use across ~50+ tool handlers: some (e.g. security-tools.ts's aidefence_* family, browser-tools.ts, ruvllm-tools.ts, wasm-agent-tools.ts, guidance-tools.ts) return a full MCP-envelope-shaped object ({content, isError:true}) on validation failure, which the dispatcher then re-wraps wholesale into content[0].text of a fresh envelope — exactly the "double-wrapped, isError never reaches the outer envelope" bug described in the issue. Others (e.g. memory_search on missing query) return a plain data object with an error field and no content/isError at all, which is a related but structurally different gap. A correct fix requires either normalizing ~50+ handler return shapes to one contract, or teaching the dispatcher a heuristic for "this payload is an error" — both are judgment calls beyond a safe, single-file, unattended nightly patch, so I did not guess at one. Left for a dedicated follow-up.

Finding 3 — swarm_health vs swarm status disagreement. Per the task instructions, not investigated for a fix; the reporter's own caveat (the two commands may be reading different state-store paths) looks plausible from a first read of the two commands, but confirming that needs deliberate product/architecture judgment on what "healthy" should mean with agentCount: 0.

Opened by the nightly triage routine — human review required before merge.

---

Generated by Claude Code

[ruvnet]

CI red on this PR is not caused by this diff. Same pre-existing, repo-wide install failure confirmed on #3134, #3135, and #3136 tonight (tracked in #3095/#3101/#3111): npm install --package-lock-only --ignore-scripts fails with npm error notarget No matching version found for @claude-flow/mcp@3.0.0-alpha.10 before any test code runs — every failing check here ("Audit root", "Security & Code Quality", "Test Suite (ubuntu-latest)", "Integration Test Setup", "Setup Verification", and the various smoke jobs) dies at that same step. Unrelated to policy-tools.ts. No fix ported: publishing the missing package or repinning the dependency is release/publish-tooling, outside this routine's scope. Not re-running — not a flake signature. Should clear once #3095/#3101/#3111 are resolved on main.

---

Generated by Claude Code