mjalmoudy94
c6488bfadf
Phase 2 of TODO-agent-2: one employee, one turn, real tools in the sandbox. - engine/prompt.py: the stable, cache-friendly prefix in the plan's order (rules → role protocol → toolkit → app profile → memory), a task brief capped at 1200 chars, role-specific stage context (spec for the developer, the diff only for the reviewer, test output for the tester) and a hard prompt cap that drops the oldest tool results first. - engine/tools.py: READ/WRITE/EDIT/LIST_DIR/SEARCH/RUN executed inside the sandbox through SandboxProviderPort, rooted at the worktree's sandbox path. Path escapes, .. and .git are refused; git push/remote/config are refused with "the platform does git"; RUN clamps its timeout (120s default, 600s max) and keeps the tail of its output; every execution writes a tool_calls row so the approvals UI shows real work. - engine/runner.py: the loop — model call, parse tags, execute tool tags, feed results back — until a terminal tag, the tool cap, the turn timeout or a cancellation. The employee's model is checked against ModelGrantPort before the first call (ungranted → error_kind=auth, no spend), errors are classified transient vs fatal, and every model call writes usage events through the one shared record_model_usage path (extracted from WorkforceService so manager chat counts too). Writes engine_turns, run_steps and tool_calls. - engine/events.py: RunFeed batches run.log (≤1 event / 500 ms / stream, ≤200 lines) and emits run.tool and task.gate through the generic envelope. - tests: engine_fakes.py (ScriptedModel, FakeSandbox, FakeGrants, RecordingMemory) plus 54 new tests in test_engine_runner, test_engine_prompt and test_engine_tools. Suite: 648 passed, ruff clean, mypy clean.
146 lines
7.6 KiB
Markdown
146 lines
7.6 KiB
Markdown
# Agent 2 — Phase 2 report: the agent turn runner (prompt, tools, loop)
|
|
|
|
**Date:** 2026-10-14 · **Branch:** `dev` · **Status:** done, all checks green
|
|
|
|
## What was done
|
|
|
|
### 2.1 `app/application/engine/prompt.py` (new)
|
|
|
|
`build_turn_prompt(TurnContext) -> TurnPrompt` builds the **stable prefix** in
|
|
exactly the plan's order, because that order is what makes prompt caching
|
|
worth anything:
|
|
|
|
1. `PLATFORM_RULES` — five short platform rules (never leaks model/infra detail,
|
|
git belongs to the platform, ask a human when in doubt).
|
|
2. The role protocol — the allowed-tag list for `(role, mode)` plus a worked
|
|
example from `_PROTOCOL_HINTS` (manager triage / final / error / chat,
|
|
planner, developer, reviewer, tester).
|
|
3. `TOOLKIT_DESCRIPTION` — the six tool tags with the rules that matter
|
|
(relative paths, git refused, output clipped). Shown **only** to roles that
|
|
actually have tools: the manager never sees a file tool.
|
|
4. The app profile text (language/framework/commands — the caller passes text,
|
|
never the row, so no secret can leak in).
|
|
5. `MemoryPort.build_context(org, role, query=task text, budget_chars=2500)`.
|
|
6. The task brief, capped at `MAX_TASK_BRIEF_CHARS = 1200`, plus the
|
|
**stage-specific** context: the planner's spec for the developer, **the
|
|
diff only** for the reviewer, the test output for the tester, and
|
|
spec+diff+tests for the manager's final verdict. A reviewer never sees the
|
|
whole tree.
|
|
7. The in-turn conversation (tool results), newest last.
|
|
|
|
Hard cap `engine_prompt_char_cap` (new setting, 24,000 chars): when exceeded the
|
|
builder drops the **oldest tool results first**, then clips from the front as a
|
|
last resort. The rules, memory and brief are never the things that get cut.
|
|
|
|
### 2.2 `app/application/engine/tools.py` (new)
|
|
|
|
`ToolExecutor.execute(tag) -> ToolOutcome` runs the toolkit **inside the
|
|
sandbox** through `SandboxProviderPort`, rooted at `WorktreeRef.sandbox_path`:
|
|
|
|
- `safe_join()` — pure, synchronous, unit-tested: rejects absolute paths, `..`
|
|
traversal, `~`, and anything under `.git`.
|
|
- `READ_FILE` / `WRITE_FILE` (body = content) / `EDIT_FILE`
|
|
(`<<<OLD … === … NEW>>>` blocks; OLD must appear **exactly once**, otherwise a
|
|
clear failure instead of a half-applied edit).
|
|
- `LIST_DIR` and `SEARCH` run as quoted `ls` / `grep -RIn` commands.
|
|
- `RUN cmd timeout` — timeout defaults to 120 s and is clamped to 600 s; the
|
|
output is capped to the **last** `engine_tool_output_chars` (8,000) because a
|
|
traceback is at the end. `git push`, `git remote` and `git config` are refused
|
|
with a Persian message that says the platform does git.
|
|
- Every execution writes a `tool_calls` row (mapped onto the existing
|
|
`ToolKey` catalog: read/write/run) with `succeeded`/`failed`, so the approvals
|
|
UI finally shows real executions rather than intentions.
|
|
- No failure raises: a refusal comes back as tool output the model reads.
|
|
|
|
### 2.3 `app/application/engine/runner.py` (new)
|
|
|
|
`TurnRunner.run_turn(TurnRequest) -> TurnResult` — the loop: model call → parse
|
|
tags → execute **tool** tags → feed results back → repeat until a terminal tag,
|
|
the tool-call cap (`engine_max_tool_calls_per_turn`), the turn timeout
|
|
(`engine_turn_timeout_seconds`) or a cancellation.
|
|
|
|
- The employee's model comes from `request.model_id` (the future
|
|
`employee_profiles.ai_model_id`) and is checked against `ModelGrantPort` **before
|
|
the first call**; ungranted or missing → `error_kind=auth`, zero model calls,
|
|
zero spend.
|
|
- Every model call writes usage events through the **same** code path the run
|
|
engine uses: `record_model_usage` was extracted from
|
|
`WorkforceService._record_usage` into a module-level function in
|
|
`workforce_service.py` (my file) so there is exactly one billing path — the
|
|
manager chat will use it too (agybot bug).
|
|
- `classify_error()` maps provider failures onto the domain vocabulary:
|
|
`rate_limit` / `unavailable` / `network` / `timeout` are transient
|
|
(`TurnResult.retryable`), `auth` / `model` / `protocol` are not.
|
|
- Cancellation is checked between steps, both through the caller's
|
|
`should_cancel` hook and by re-reading the run row.
|
|
- Records written: `engine_turns` (role, mode, stage, prompt chars, reply
|
|
excerpt ≤ 4,000, tag counts, tool calls, ok/error_kind), `run_steps`
|
|
(timeline entries for tools and errors) and `tool_calls`.
|
|
- `TERMINAL_TAGS` names the decision tag of each role/mode; **only tool tags are
|
|
executed here** — `ASSIGN`, `COMPLETE`, `CREATE_TASK`, `KNOWLEDGE`… are
|
|
decisions the caller interprets, which keeps orchestration out of the loop.
|
|
|
|
### `app/application/engine/events.py` (new)
|
|
|
|
`RunFeed` implements PLAN-v2 §5's batching for `run.log`, plus `run.tool` and
|
|
`task.gate` (which have no typed emitter in the frozen realtime port, so they go
|
|
through the generic envelope): at most one log event per 500 ms per stream, at
|
|
most 200 lines per event, with a forced flush at the end of a turn.
|
|
|
|
### New settings (inside the agent-2 block of `config.py`)
|
|
|
|
`engine_prompt_char_cap`, `engine_tool_output_chars`, `engine_reply_excerpt_chars`,
|
|
`engine_log_flush_seconds`, `engine_log_max_lines_per_event`.
|
|
|
|
## Tests
|
|
|
|
- `tests/engine_fakes.py` (new): `ScriptedModel` (queued replies **and** queued
|
|
exceptions, records every system prompt and message it was given),
|
|
`FakeSandbox` (dict filesystem + scripted command results per command prefix,
|
|
records every execution), `FakeGrants`, `RecordingMemory`.
|
|
- `tests/test_engine_runner.py` (22 tests): write → run → report; tool results
|
|
fed back into the next prompt; a `tool_calls` row per execution (succeeded and
|
|
failed); the `engine_turns` row; path escapes refused (4 shapes); git refused
|
|
(3 commands, nothing executed); EDIT_FILE applied and refused when not unique;
|
|
the tool-call cap; the turn timeout; cancellation between steps; rate limit
|
|
transient; **ungranted model refused before any call**; error classification;
|
|
usage events per model call; no settings secret in any prompt.
|
|
- `tests/test_engine_prompt.py` (11 tests): the prefix order; toolkit only for
|
|
tool roles; brief cap; stage context per role; oldest-results-dropped-first
|
|
truncation; tiny-cap keeps rules and brief; no secrets; per-mode protocol hints.
|
|
- `tests/test_engine_tools.py` (11 tests): path safety matrix; git refusal;
|
|
timeout clamp and default; output tail truncation; round trip; quoted
|
|
list/search; tool-key mapping; malformed edit body.
|
|
|
|
## Checks (PLAN-v2 §7.4)
|
|
|
|
```
|
|
cd apps/api && .venv/bin/python -m pytest -q → 648 passed, 1 skipped (54 new)
|
|
cd apps/api && .venv/bin/ruff check . → All checks passed!
|
|
cd apps/api && .venv/bin/mypy app → Success: no issues found in 148 source files
|
|
```
|
|
|
|
No web files were touched, no migration was needed (no schema change).
|
|
|
|
## Deviations & notes
|
|
|
|
1. `WorkforceService._record_usage` now delegates to the new module-level
|
|
`record_model_usage`; behaviour is identical and the existing 14 workforce
|
|
tests still pass. This keeps one billing code path instead of a copy.
|
|
2. The runner takes the sandbox session id from the request
|
|
(`session_external_id`) and fails cleanly when there is none. Wiring the real
|
|
session (workspace container per app, SandboxService) belongs to Phase 4 with
|
|
the scheduler; deliberately not faked here.
|
|
3. `TERMINAL_TAGS` treats the manager-chat tags as the chat layer's tags: the
|
|
chat turn emits decisions, not work.
|
|
|
|
## Open questions / CHANGE REQUESTs
|
|
|
|
- None. Expected next: org automation policy behind `ModelGrantPort` (Phase 4).
|
|
|
|
## Next
|
|
|
|
Phase 3 — manager chat (orders → tasks → team), triage/final/error turns,
|
|
`KNOWLEDGE` → `MemoryPort.record`, `WorkforceBootstrapPort.bootstrap_team`, and
|
|
`tests/test_engine_manager.py`.
|