---
phase: 08-quality-polish-and-test-coverage
plan: 01
type: execute
wave: 1
depends_on: []
files_modified:
  - src/server/workflow/prompt_handler.rs
  - src/server/workflow/task_prompt_handler.rs
autonomous: true
requirements: []

must_haves:
  truths:
    - "PauseReason::SchemaMismatch.missing_fields contains actual missing field names, not ['unknown']"
    - "Every break in the task execution loop sets a PauseReason — no silent breaks"
    - "Both callers of params_satisfy_tool_schema correctly handle Vec<String> return type"
  artifacts:
    - path: "src/server/workflow/prompt_handler.rs"
      provides: "params_satisfy_tool_schema returning Vec<String> of missing field names"
      contains: "Vec<String>"
    - path: "src/server/workflow/task_prompt_handler.rs"
      provides: "PauseReason set on all break paths, SchemaMismatch uses actual field names"
      contains: "tracing::warn!"
  key_links:
    - from: "src/server/workflow/task_prompt_handler.rs"
      to: "src/server/workflow/prompt_handler.rs"
      via: "self.inner.params_satisfy_tool_schema() call"
      pattern: "params_satisfy_tool_schema.*Vec<String>"
---

<objective>
Fix SchemaMismatch diagnostic accuracy and eliminate silent breaks in the task execution loop.

Purpose: Close FINDING-01 from the v1.1 milestone audit (SchemaMismatch.missing_fields hardcoded to ["unknown"]) and two tech debt items from Phase 5 (silent breaks without PauseReason at lines 767 and 771 of task_prompt_handler.rs).

Output: Accurate missing-field diagnostics in _meta JSON and full PauseReason coverage on all execution break paths.
</objective>

<execution_context>
@./.claude/get-shit-done/workflows/execute-plan.md
@./.claude/get-shit-done/templates/summary.md
</execution_context>

<context>
@.planning/PROJECT.md
@.planning/ROADMAP.md
@.planning/STATE.md
@.planning/v1.1-MILESTONE-AUDIT.md
@.planning/phases/08-quality-polish-and-test-coverage/08-CONTEXT.md
@.planning/phases/08-quality-polish-and-test-coverage/08-RESEARCH.md
@src/server/workflow/prompt_handler.rs
@src/server/workflow/task_prompt_handler.rs
</context>

<tasks>

<task type="auto">
  <name>Task 1: Change params_satisfy_tool_schema return type and adapt both callers</name>
  <files>
    src/server/workflow/prompt_handler.rs
    src/server/workflow/task_prompt_handler.rs
  </files>
  <action>
**In `src/server/workflow/prompt_handler.rs`:**

1. Change `params_satisfy_tool_schema` signature from `Result<bool>` to `Result<Vec<String>>`:
   - At line 566, change return type to `Result<Vec<String>>`
   - Replace the early-return-on-first-missing-field logic with a `let mut missing_fields = Vec::new()` collector
   - Instead of `return Ok(false)` when a required field is missing, do `missing_fields.push(field_name.to_string())`
   - Instead of `Ok(true)` at the end, return `Ok(missing_fields)` (empty vec = all satisfied)
   - When `params` is not an object but `required` is non-empty, collect ALL required field names as missing
   - See 08-RESEARCH.md "Pattern 1: params_satisfy_tool_schema Return Type Change" for the exact target code

2. Adapt the inner handler's own caller (around line 893):
   - Currently: `let Ok(satisfies_schema) = self.params_satisfy_tool_schema(step, &params)` used in a condition
   - Change to match on the Vec result: treat non-empty vec the same as the old `Ok(false)` (skip step), treat empty vec as old `Ok(true)` (proceed)
   - Use `let Ok(ref missing) = self.params_satisfy_tool_schema(step, &params)` pattern and check `!missing.is_empty()` for skip

**In `src/server/workflow/task_prompt_handler.rs`:**

3. Adapt the task handler caller at line 770:
   - Change `match self.inner.params_satisfy_tool_schema(step, &params)` arms:
   - Old `Ok(false)` becomes `Ok(ref missing) if !missing.is_empty()` — use `missing.clone()` for the `missing_fields` in `PauseReason::SchemaMismatch` (replacing the hardcoded `vec!["unknown".to_string()]`)
   - Old `Ok(true)` becomes the remaining `Ok(_)` arm (empty vec = proceed)
   - The `missing_fields` now goes into `_meta` JSON only — per locked decision, do NOT change the prompt narrative text

**Per locked decisions:**
- Collect ALL missing fields, not just the first one
- Missing field names go in _meta JSON only (task layer), prompt narrative unchanged
- Tasks and prompts are independent mechanisms — no coupling
  </action>
  <verify>
    Run `cargo build --package pmcp` — must compile with zero errors (type change enforced by compiler).
    Run `cargo test --package pmcp -- workflow` — all existing workflow tests pass.
    Run `cargo test --package pmcp-tasks -- workflow` — all integration tests pass.
  </verify>
  <done>
    params_satisfy_tool_schema returns Vec<String> with actual missing field names. Both callers (inner handler at ~line 893 and task handler at line 770) correctly handle the new return type. PauseReason::SchemaMismatch.missing_fields contains real field names from the schema check, not ["unknown"]. Zero compilation errors.
  </done>
</task>

<task type="auto">
  <name>Task 2: Fix silent breaks with PauseReason and tracing</name>
  <files>
    src/server/workflow/task_prompt_handler.rs
  </files>
  <action>
**Fix two silent break paths in the execution loop of `task_prompt_handler.rs`:**

1. **Line 767 — resolve_tool_parameters failure:**
   - Currently: `Err(_) => break,` (no PauseReason set)
   - Replace with: Route through `classify_resolution_failure(step, self.workflow.steps(), &step_statuses)` per research recommendation — this path has the same information available as the announcement failure path and will correctly identify dependency issues vs generic unresolvable params
   - Add `tracing::warn!("resolve_tool_parameters failed unexpectedly for step '{}' after announcement succeeded", step.name());`
   - Set `pause_reason = Some(classify_resolution_failure(...));` then `break;`

2. **Line 771 — params_satisfy_tool_schema Err path:**
   - Currently: `Err(_) => break,` (no PauseReason set)
   - This is the `Err` variant (not the `Ok(non-empty-vec)` from Task 1) — it means the schema lookup itself failed (tool not found, etc.)
   - Replace with: Direct `PauseReason::UnresolvableParams` assignment since this is a tool registry / schema lookup error, not a resolution failure
   - Add `tracing::warn!("params_satisfy_tool_schema error for step '{}': {}", step.name(), e);` (capture the error `e`)
   - Set `pause_reason = Some(PauseReason::UnresolvableParams { blocked_step: step.name().to_string(), missing_param: "unknown".to_string(), suggested_tool: step.tool().map(|t| t.name().to_string()).unwrap_or_default() });`
   - Then `break;`

**Notes:**
- Ensure `tracing` is already imported (it should be, used elsewhere in the file)
- The PauseReason here is the local mirror type defined in task_prompt_handler.rs (not the pmcp-tasks crate type)
- Per locked decision: both paths produce `PauseReason::UnresolvableParams` (one via classify_resolution_failure, one direct)
  </action>
  <verify>
    Run `cargo build --package pmcp` — compiles cleanly.
    Run `cargo clippy --package pmcp -- -D warnings` — zero warnings.
    Run `cargo test --package pmcp -- workflow` — all tests pass.
    Grep for `Err(_) => break` in task_prompt_handler.rs — should return zero matches (all break paths now have PauseReason).
  </verify>
  <done>
    Both silent break paths at lines 767 and 771 now set a PauseReason before breaking. Both paths emit `tracing::warn!` for observability. No `Err(_) => break` patterns remain in the task execution loop. Zero clippy warnings. All existing tests pass.
  </done>
</task>

</tasks>

<verification>
1. `cargo build --package pmcp` — zero errors
2. `cargo clippy --package pmcp -- -D warnings` — zero warnings
3. `cargo test --package pmcp -- workflow` — all workflow tests pass
4. `cargo test --package pmcp-tasks -- workflow` — all integration tests pass
5. `grep -n "Err(_) => break" src/server/workflow/task_prompt_handler.rs` — returns empty (no silent breaks)
6. `grep -n "unknown.*missing_fields\|missing_fields.*unknown" src/server/workflow/task_prompt_handler.rs` — the hardcoded `["unknown"]` for SchemaMismatch is replaced with real field names
</verification>

<success_criteria>
- PauseReason::SchemaMismatch.missing_fields returns actual field names from schema validation
- All execution loop break paths set a PauseReason (zero silent breaks)
- Both callers of params_satisfy_tool_schema correctly handle Vec<String> return
- Zero compilation errors, zero clippy warnings
- All existing tests pass without modification
</success_criteria>

<output>
After completion, create `.planning/phases/08-quality-polish-and-test-coverage/08-01-SUMMARY.md`
</output>
