Coder implmentation and Tool Enhancement - #1
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
This PR implements a “real tool execution layer” for Roster specialists (READ/EDIT/EXEC + boundary approval gating), adds a new FETCH web tool, and extends the dashboard event contract/UI to surface tool activity and diffs.
Changes:
- Add worktree-based file/shell tooling with boundary classification + approval proposal workflow in the runtime.
- Add FETCH (open URL → extract/cap text) with runtime support, events, and unit tests.
- Extend frontend event/types/store and add a unified-diff parser + VS Code-style DiffView component.
Reviewed changes
Copilot reviewed 49 out of 49 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| specs/004-tool-execution-layer/tasks.md | New task breakdown for spec 004 implementation. |
| specs/004-tool-execution-layer/plan.md | New implementation plan detailing tool loop, gating, and diff UI. |
| runtime/tests/conftest.py | Adds git repo + runtime_config fixtures for runtime/tool tests. |
| runtime/tests/test_workspace.py | Tests worktree lifecycle and path confinement. |
| runtime/tests/test_tools.py | Tests ToolExecutor read/edit/exec behaviors and gating. |
| runtime/tests/test_task_result.py | Tests TaskResult building + diff artifact writing against schema. |
| runtime/tests/test_us1_e2e.py | End-to-end test for US1 (read/edit/exec + diff artifact). |
| runtime/tests/test_us2_gate.py | End-to-end approval gate tests including real git push to bare remote. |
| runtime/tests/test_boundary.py | Tests path resolver + action classifier tiers/auto-vs-gate. |
| runtime/tests/test_diffutil.py | Tests unified diff summarization helper. |
| runtime/tests/test_protocol.py | Extends protocol tests for parsing tool directives. |
| runtime/tests/test_agent_tool_loop.py | Tests Agent tool loop (parse → execute → feedback) and approval pause. |
| runtime/tests/test_orchestrator_tools.py | Tests Run wiring of worktree-backed executor. |
| runtime/tests/test_orchestrator_loop.py | Extends orchestration loop tests (retry attribution, partial failures). |
| runtime/tests/test_fetch.py | Adds FETCH tool unit tests + agent FETCH directive loop tests. |
| runtime/roster/workspace.py | New worktree manager for per-run sandbox branches. |
| runtime/roster/tools.py | New ToolExecutor with read/edit/exec, diff generation, and bounded exec. |
| runtime/roster/task_result.py | New TaskResult builder/writer + diff artifact persistence. |
| runtime/roster/boundary.py | New pure boundary classifier + worktree path resolver. |
| runtime/roster/approval.py | New ActionProposal persistence + decision parsing helpers. |
| runtime/roster/diffutil.py | New pure diff summarizer for per-file stats. |
| runtime/roster/protocol.py | Adds ToolCall parsing + directive-line misformat detection. |
| runtime/roster/fetch.py | New FETCH implementation (URL checks + HTML extraction + caps). |
| runtime/roster/events.py | Adds tool/approval event kinds and emit helpers. |
| runtime/roster/agent.py | Generalizes tool loop (READ/EDIT/EXEC + SEARCH + FETCH), new statuses. |
| runtime/roster/config.py | Adds workspace config (target_repo/worktrees_root) and env overrides. |
| runtime/roster/server.py | Adds /api/approvals/{propId} endpoint and exposes fetch flags. |
| runtime/requirements-dev.txt | Adds jsonschema for dev/test schema validation. |
| runtime/agents.config.example.yaml | Documents workspace config and grants fetch to researcher/qa. |
| researcher-agent/researcher.agent.md | Updates Researcher guidance to use FETCH for evidence. |
| researcher-agent/researcher-runner/SKILL.md | Updates researcher runbook for SEARCH+FETCH workflow. |
| qa-agent/qa.agent.md | Updates QA guidance to fetch cited sources first. |
| qa-agent/qa-runner/SKILL.md | Updates QA runbook verification order (FETCH then SEARCH). |
| planner-agent/planner.agent.md | Rewrites Planner guidance (but frontmatter contract needs attention). |
| README.md | Adds additional narrative/questions (needs cleanup for README suitability). |
| frontend/src/types/models.ts | Extends domain types for tool/diff/approval + new agent statuses. |
| frontend/src/types/events.ts | Adds tool.fetch/tool.file/tool.exec/approval.* event typings. |
| frontend/src/store/store.ts | Adds approval UI state and decision marking. |
| frontend/src/store/handleEvent.ts | Adds reducers/trace for fetch + tool + approval events. |
| frontend/src/components/Sidebar.tsx | Updates “busy” computation to account for fetching. |
| frontend/src/components/orchestration/TraceTimeline.tsx | Adds one-line rendering for fetch/tool/approval trace items. |
| frontend/src/components/lineage/AgentNode.tsx | Updates busy animation logic for fetching. |
| frontend/src/components/lineage/lineage.module.css | Adds fetching styling. |
| frontend/src/components/diff/lib/parseUnifiedDiff.ts | New unified diff parser (pure). |
| frontend/src/components/diff/lib/parseUnifiedDiff.test.ts | Vitest coverage for unified diff parser. |
| frontend/src/components/diff/DiffView.tsx | New VS Code-style diff renderer with collapse/raw fallback. |
| frontend/src/components/diff/diff.module.css | Styles for DiffView component. |
Comment on lines
1
to
4
| --- | ||
| description: "Top-level Planner / PM agent. Reads the principal's goal, decomposes it into a ratifiable plan (DAG of tasks assigned to expert sub-agents), submits the plan to the Approval Gate, and — once ratified — dispatches tasks and supervises execution. Never performs irreversible actions itself." | ||
| tools: [read, edit, search, agent] | ||
| description: "Top-level Planner — the team's CEO. Understands the principal's goal, decomposes it, dispatches sub-tasks to specialist agents, critiques their results before answering, and asks the principal mid-task when genuinely blocked. Never executes work itself." | ||
| emoji: "🧭" | ||
| --- |
Comment on lines
+208
to
+212
| 如果coder没配workspace.target_repo,生成的东西要不直接放在downloads 文件夹里吧 | ||
| "数据只能穿过 planner 传给 coder" 我确实知道这个constraint,但是这里的最优解是什么?我觉得引入subagents互相直接都能call,但是这样的话,context就乱套了,最后和用户交互的planner需要做judge,来把任务返回给用户嘛. maybe只能说subagent不止一个? subagents可以call别的subagents但是是新的instance? 不知道 | ||
| 不会自然地"派 coder 去生产报告"。而且 MAX_CRITIQUE=2. 那这里最好如何改呢? | ||
| 这个打通的方法是什么? | ||
| 没有预览没关系, 扔个链接给用户,用户自己打开就好 No newline at end of file |
Comment on lines
+5
to
+12
| export type AgentStatusValue = | ||
| | "idle" | ||
| | "queued" | ||
| | "thinking" | ||
| | "searching" | ||
| | "fetching" | ||
| | "working" | ||
| | "error"; |
Comment on lines
52
to
55
| // Live queue counts derived from agent statuses (matches the legacy dashboard). | ||
| const busy = Object.values(agents).filter( | ||
| (a) => a.status === "thinking" || a.status === "searching", | ||
| (a) => a.status === "thinking" || a.status === "searching" || a.status === "fetching", | ||
| ).length; |
Comment on lines
8
to
+12
| const busy = | ||
| node.status === "thinking" || node.status === "searching" || node.status === "queued"; | ||
| node.status === "thinking" || | ||
| node.status === "searching" || | ||
| node.status === "fetching" || | ||
| node.status === "queued"; |
Comment on lines
+121
to
+126
| def _cap(text: str, limit: int) -> tuple[str, bool]: | ||
| raw = text or "" | ||
| if len(raw.encode("utf-8", errors="replace")) <= limit: | ||
| return raw, False | ||
| return raw[:limit] + _TRUNC, True | ||
|
|
| "exec", "gated", f"needs approval: {cls.reason}", command=command, tier=cls.tier, reason=cls.reason | ||
| ) | ||
| try: | ||
| argv = shlex.split(command, posix=True) |
Comment on lines
+218
to
+220
| def _git(self, *args: str) -> subprocess.CompletedProcess[str]: | ||
| return subprocess.run(["git", "-C", str(self.wt.path), *args], capture_output=True, text=True) | ||
|
|
Comment on lines
+167
to
+177
| def execute(self, call: ToolCall, *, approved: bool = False) -> ToolResult: | ||
| """Run one tool call. ``approved`` bypasses the auto/gate check (US2 grants it post-approval).""" | ||
| if call.error: | ||
| return ToolResult(call.kind, "error", f"malformed {call.kind} directive", reason=call.error) | ||
| if call.kind == "read": | ||
| return self._read(call.path or "") | ||
| if call.kind == "edit": | ||
| return self._edit(call.path or "", call.body or "") | ||
| if call.kind == "exec": | ||
| return self._exec(call.command or "", approved=approved) | ||
| return ToolResult(call.kind, "error", f"unknown tool {call.kind!r}") |
Comment on lines
+64
to
+68
| export interface ToolFetchEvent extends BaseEvent { | ||
| kind: "tool.fetch"; | ||
| agent: string; | ||
| phase: FetchPhase; | ||
| url: string; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Describe the change and why it is needed.
Validation
Notes
Anything reviewers should pay special attention to.