---
phase: 08-quality-polish-and-test-coverage
plan: 02
type: execute
wave: 1
depends_on: []
files_modified:
  - crates/pmcp-tasks/src/router.rs
  - crates/pmcp-tasks/src/domain/record.rs
  - crates/pmcp-tasks/tests/property_tests.rs
  - crates/pmcp-tasks/tests/workflow_integration.rs
autonomous: true
requirements: []

must_haves:
  truths:
    - "cargo clippy --package pmcp-tasks --tests -- -D warnings passes with zero warnings"
    - "Property test fresh_task_record_is_not_expired passes with TTL values up to 30 days"
    - "Production code handles u64 TTL values exceeding i64::MAX by treating them as never-expires"
    - "E2E integration test exercises continuation with a succeeding tool through ServerCore::handle_request and verifies store update"
  artifacts:
    - path: "crates/pmcp-tasks/src/router.rs"
      provides: "Clippy-clean test assertions"
      contains: "contains_key"
    - path: "crates/pmcp-tasks/src/domain/record.rs"
      provides: "Saturating TTL arithmetic using i64::try_from"
      contains: "i64::try_from"
    - path: "crates/pmcp-tasks/tests/property_tests.rs"
      provides: "Constrained TTL range (max 30 days) for property tests"
      contains: "2_592_000_000"
    - path: "crates/pmcp-tasks/tests/workflow_integration.rs"
      provides: "E2E continuation test with ConditionalFetchDataTool"
      contains: "ConditionalFetchDataTool"
  key_links:
    - from: "crates/pmcp-tasks/tests/workflow_integration.rs"
      to: "crates/pmcp-tasks/src/router.rs"
      via: "ServerCore::handle_request with _task_id continuation"
      pattern: "handle_request.*task_id"
---

<objective>
Fix clippy warning, property test TTL overflow, and add E2E continuation test with succeeding tool.

Purpose: Close the remaining tech debt items from the v1.1 milestone audit: clippy warning (Phase 4), property test regression (Phase 6), commented-out E2E Stage 2 (Phase 7), and FINDING-02 (E2E continuation coverage gap).

Output: Zero clippy warnings, passing property tests with realistic TTL ranges, and complete E2E lifecycle test coverage including successful continuation.
</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
@crates/pmcp-tasks/src/router.rs
@crates/pmcp-tasks/src/domain/record.rs
@crates/pmcp-tasks/tests/property_tests.rs
@crates/pmcp-tasks/tests/workflow_integration.rs
</context>

<tasks>

<task type="auto">
  <name>Task 1: Fix clippy warning and property test TTL overflow</name>
  <files>
    crates/pmcp-tasks/src/router.rs
    crates/pmcp-tasks/src/domain/record.rs
    crates/pmcp-tasks/tests/property_tests.rs
  </files>
  <action>
**1. Fix clippy warning in router.rs (line 554):**
- Change `assert!(record.variables.get("progress_token").is_none());` to `assert!(!record.variables.contains_key("progress_token"));`
- This is the `unnecessary_get_then_check` lint — mechanical one-line fix

**2. Fix TTL overflow in production code (record.rs, lines 99-105):**
- Current code: `Duration::try_milliseconds(ms as i64)` — the `as i64` cast wraps silently when u64 exceeds i64::MAX, producing a negative duration that creates an expiry time in the past
- Replace `ms as i64` with `i64::try_from(ms).ok()?` — this returns None on overflow, causing `expires_at = None` (never expires), which is correct defensive behavior
- Target code:
  ```rust
  let expires_at = ttl.and_then(|ms| {
      let ms_i64 = i64::try_from(ms).ok()?;
      let duration = Duration::try_milliseconds(ms_i64)?;
      now.checked_add_signed(duration)
  });
  ```

**3. Constrain property test TTL range (property_tests.rs, line ~136):**
- Change the TTL strategy from `proptest::option::of(0u64..=u64::MAX)` to `proptest::option::of(0u64..=2_592_000_000u64)` (30 days in milliseconds: 30 * 24 * 60 * 60 * 1000)
- This makes the test inputs realistic while the production fix handles extreme values defensively

**4. Delete the proptest regression file:**
- Delete `crates/pmcp-tasks/tests/property_tests.proptest-regressions`
- The production code fix makes the shrunk regression case pass, so the file is no longer needed
- Per locked decision: clean up after fix
  </action>
  <verify>
    Run `cargo clippy --package pmcp-tasks --tests -- -D warnings` — zero warnings.
    Run `cargo test --package pmcp-tasks -- property_tests` — all property tests pass including fresh_task_record_is_not_expired.
    Verify regression file is deleted: `test ! -f crates/pmcp-tasks/tests/property_tests.proptest-regressions && echo "DELETED"`.
  </verify>
  <done>
    Clippy passes with zero warnings on pmcp-tasks (including tests). Property test fresh_task_record_is_not_expired passes with constrained 30-day TTL range. Production code uses i64::try_from for safe overflow handling. Proptest regression file deleted.
  </done>
</task>

<task type="auto">
  <name>Task 2: Add E2E continuation test with succeeding tool</name>
  <files>
    crates/pmcp-tasks/tests/workflow_integration.rs
  </files>
  <action>
**Create ConditionalFetchDataTool and fix test_full_lifecycle_happy_path Stage 2:**

1. **Add `ConditionalFetchDataTool` struct** in the test helpers section of workflow_integration.rs:
   - Implements `ToolHandler`
   - `handle()`: checks `source` argument — returns `Err(pmcp::Error::internal("key not found: non_existent_key"))` when `source == "non_existent_key"`, returns `Ok(json!({ "data": "raw_content", "source": source }))` otherwise
   - `metadata()`: returns ToolInfo with name "fetch_data", description "Fetch raw data from a source", schema requiring `source` field
   - See 08-RESEARCH.md "ConditionalFetchDataTool for E2E Test" for the exact pattern

2. **Add `build_conditional_test_server()` function** alongside existing `build_test_server()` and `build_failing_test_server()`:
   - Same structure as `build_failing_test_server()` but registers `ConditionalFetchDataTool` instead of `FailingFetchDataTool`
   - Returns `(ServerCore, Arc<InMemoryTaskStore>)`
   - The workflow definition should use `source` argument set to `"non_existent_key"` so the initial execution hits the failure path

3. **Uncomment and fix Stage 2 of `test_full_lifecycle_happy_path`:**
   - Switch the test to use `build_conditional_test_server()` instead of `build_failing_test_server()`
   - Stage 1 remains: invoke workflow prompt, verify handoff with paused state (fetch_data step fails with "non_existent_key")
   - Stage 2 (previously commented out): Build a `CallTool` request for "fetch_data" with `source: "existing_key"` and include `_task_id` in the request `_meta`
   - Send through `server.handle_request(call_tool_request).await`
   - After the call returns, inspect the task store directly:
     a. `store.get(task_id, "local").await` to get the record
     b. Verify `_workflow.result.fetch` exists in variables and contains the tool output (`{ "data": "raw_content", "source": "existing_key" }`)
     c. Verify `_workflow.progress` in variables shows the fetch step as `"completed"` status
   - Per locked decision: "same tool, different arguments" pattern — workflow uses bad key, client retries with good key

**Key implementation details:**
- The `_task_id` goes in request `_meta` per CONT-01 — check the existing test patterns in the file for how `_meta` is structured in CallTool requests
- The continuation intercept in ServerCore fires after the tool handler returns success (fire-and-forget pattern from Phase 6)
- The store is directly accessible via `Arc<InMemoryTaskStore>` for assertions
  </action>
  <verify>
    Run `cargo test --package pmcp-tasks -- test_full_lifecycle_happy_path` — the test passes including Stage 2 continuation.
    Run `cargo test --package pmcp-tasks -- workflow_integration` — all workflow integration tests pass.
    Run `cargo test --package pmcp-tasks` — full test suite passes.
  </verify>
  <done>
    ConditionalFetchDataTool enables same-tool-different-args testing pattern. test_full_lifecycle_happy_path exercises full E2E lifecycle: workflow invocation with failing args, structured handoff, client continuation with succeeding args through ServerCore::handle_request, store verification of _workflow.result.fetch and _workflow.progress step completion. FINDING-02 from milestone audit is closed.
  </done>
</task>

</tasks>

<verification>
1. `cargo clippy --package pmcp-tasks --tests -- -D warnings` — zero warnings
2. `cargo test --package pmcp-tasks` — full test suite passes (property tests, integration tests, unit tests)
3. `test ! -f crates/pmcp-tasks/tests/property_tests.proptest-regressions && echo "DELETED"` — regression file removed
4. `cargo test --package pmcp-tasks -- test_full_lifecycle_happy_path` — E2E lifecycle with continuation passes
5. `cargo test --package pmcp-tasks -- property_tests::fresh_task_record_is_not_expired` — property test passes
</verification>

<success_criteria>
- cargo clippy --package pmcp-tasks --tests -- -D warnings passes with zero warnings
- Property test fresh_task_record_is_not_expired passes consistently (no TTL overflow)
- Production TTL code uses i64::try_from for safe overflow handling
- Proptest regression file deleted
- E2E test exercises continuation with succeeding tool through ServerCore::handle_request
- E2E test verifies both _workflow.result.fetch and _workflow.progress in task store
- All existing tests continue to pass
</success_criteria>

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