Skip to content

Commit fdca75c

Browse files
nhortonclaude
andcommitted
fix: address review findings
- Add project_root to ActiveStepInfo in doc/mcp_interface.md - Add mypy-required None checks in test_server.py - Remove invalid kwargs from StartWorkflowResponse in test - Update traceability comment to reference JOBS-REQ-001.3.10 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 7f83cea commit fdca75c

3 files changed

Lines changed: 10 additions & 3 deletions

File tree

doc/mcp_interface.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -289,6 +289,7 @@ interface StepInputInfo {
289289
interface ActiveStepInfo {
290290
session_id: string; // Session ID — use this for all subsequent finished_step, abort_workflow, go_to_step calls
291291
step_id: string; // ID of the current step
292+
project_root: string; // Absolute path to the MCP server's project root (use for .deepwork/ operations)
292293
job_dir: string; // Absolute path to job directory (templates, scripts, etc.)
293294
step_expected_outputs: ExpectedOutput[]; // Expected outputs with type and format hints
294295
step_inputs: StepInputInfo[]; // Inputs provided to this step with their values
@@ -445,6 +446,7 @@ Add to your `.mcp.json`:
445446

446447
| Version | Changes |
447448
|---------|---------|
449+
| 2.3.0 | Added `project_root` field to `ActiveStepInfo` — the absolute path to the MCP server's project root. Agents use this as the base directory for `.deepwork/` operations (e.g. creating jobs at `[project_root]/.deepwork/jobs/`). The `make_new_job.sh` script now requires `--project-root` instead of inferring the root from `git rev-parse`. |
448450
| 2.2.0 | `session_id` is now optional (`str | None`) on `start_workflow` only. On Claude Code (platform `"claude"`), the server raises `ToolError` if omitted. On other platforms, omitting it auto-generates a stable UUID; callers use the returned `begin_step.session_id` for all subsequent calls. `finished_step`, `abort_workflow`, and `go_to_step` continue to require `session_id`. Added `inputs` optional parameter to `start_workflow` for passing step argument values directly at workflow start. Added `issue_detected` optional field to all tool responses — present when the server detects configuration issues at startup; instructs agent to suggest repair to the user. |
449451
| 2.1.0 | Added `important_note` field to `StartWorkflowResponse` — instructs agents to clarify ambiguous user requests via `AskUserQuestion` when available. |
450452
| 2.0.0 | **Breaking**: `session_id` is now a required `string` parameter on all mutation tools (`start_workflow`, `finished_step`, `abort_workflow`, `go_to_step`). Added `agent_id` optional parameter for sub-agent scoping — sub-agents get their own isolated workflow stacks. State persistence path changed to `.deepwork/tmp/sessions/<platform>/session-<id>/state.json` (with sub-agent state in `agent_<agent_id>.json`). |

tests/unit/jobs/mcp/test_server.py

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -122,13 +122,15 @@ def test_issue_warning_in_instructions(self, tmp_path: Path) -> None:
122122
with patch("deepwork.jobs.mcp.server.detect_issues", return_value=[issue]):
123123
mcp = create_server(project_root=tmp_path)
124124

125+
assert mcp.instructions is not None
125126
assert "ISSUE DETECTED" in mcp.instructions
126127

127128
def test_no_issue_warning_when_no_issues(self, tmp_path: Path) -> None:
128129
"""When no issues exist, instructions don't contain issue warning."""
129130
with patch("deepwork.jobs.mcp.server.detect_issues", return_value=[]):
130131
mcp = create_server(project_root=tmp_path)
131132

133+
assert mcp.instructions is not None
132134
assert "ISSUE DETECTED" not in mcp.instructions
133135

134136

@@ -300,8 +302,6 @@ async def test_start_workflow_with_inputs_and_agent_id(self, tmp_path: Path) ->
300302
mcp, mock_tools = _make_server_with_mocked_tools(tmp_path)
301303
mock_tools.start_workflow = AsyncMock(
302304
return_value=StartWorkflowResponse(
303-
job_name="j",
304-
workflow_name="w",
305305
begin_step=ActiveStepInfo(
306306
session_id="s",
307307
step_id="s1",
@@ -516,6 +516,7 @@ async def test_get_review_instructions_success(self, tmp_path: Path) -> None:
516516
mcp = create_server(project_root=tmp_path)
517517
result = await mcp.call_tool("get_review_instructions", {"files": ["src/foo.py"]})
518518

519+
assert result.structured_content is not None
519520
data = result.structured_content["result"]
520521
assert data == "Review task list"
521522

@@ -537,6 +538,7 @@ async def test_get_review_instructions_error(self, tmp_path: Path) -> None:
537538
mcp = create_server(project_root=tmp_path)
538539
result = await mcp.call_tool("get_review_instructions", {})
539540

541+
assert result.structured_content is not None
540542
data = result.structured_content["result"]
541543
assert "Review error" in data
542544
assert "git not found" in data
@@ -560,6 +562,7 @@ async def test_get_configured_reviews(self, tmp_path: Path) -> None:
560562
{"only_rules_matching_files": ["test.py"]},
561563
)
562564

565+
assert result.structured_content is not None
563566
data = result.structured_content["result"]
564567
assert len(data) == 1
565568
assert data[0]["name"] == "rule1"
@@ -580,6 +583,7 @@ async def test_mark_review_as_passed_success(self, tmp_path: Path) -> None:
580583
mcp = create_server(project_root=tmp_path)
581584
result = await mcp.call_tool("mark_review_as_passed", {"review_id": "abc123"})
582585

586+
assert result.structured_content is not None
583587
data = result.structured_content["result"]
584588
assert "abc123" in data
585589

@@ -599,6 +603,7 @@ async def test_mark_review_as_passed_validation_error(self, tmp_path: Path) -> N
599603
mcp = create_server(project_root=tmp_path)
600604
result = await mcp.call_tool("mark_review_as_passed", {"review_id": "bad"})
601605

606+
assert result.structured_content is not None
602607
data = result.structured_content["result"]
603608
assert "Validation error" in data
604609
assert "Invalid review_id" in data

tests/unit/jobs/mcp/test_tools.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ class TestStartWorkflow:
211211
"""Tests for start_workflow tool."""
212212

213213
@pytest.mark.asyncio
214-
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-001.3.8).
214+
# THIS TEST VALIDATES A HARD REQUIREMENT (JOBS-REQ-001.3.8, JOBS-REQ-001.3.10).
215215
# YOU MUST NOT MODIFY THIS TEST UNLESS THE REQUIREMENT CHANGES
216216
async def test_creates_session_and_returns_first_step(self, tools: WorkflowTools) -> None:
217217
resp = await _start_main_workflow(tools)

0 commit comments

Comments
 (0)