smart-org/docs/plan/reports/agent-2-phase-3.md

# Agent 2 — Phase 3 report: the manager (orders → tasks → team)

**Date:** 2026-10-14 · **Branch:** `dev` · **Status:** done, all checks green

## What was done

### 3.1 Manager chat — `app/application/engine/manager.py` (new)

`ManagerTurns.chat()` is the manager's voice on a thread. `WorkforceService.post_user_message`
was rewritten to delegate to it (same signature, same session, so the reply, the tasks
and the usage events commit together).

- The reply the user reads is the **prose outside the tags**; the tags are executed
  beside it, one Persian "•" line per action. A malformed reply is quoted back to
  the model once (`CHAT_MAX_ATTEMPTS = 2`, `CHAT_TURN_TIMEOUT_SECONDS = 120`) — but a
  reply that has protocol *errors* **and** usable tags is kept: the extras the per-reply
  limit dropped (3 tasks / 3 team changes / 3 knowledge items) must not discard the work
  that was valid.
- Executed tags: `CREATE_TASK` (title, kind, priority, `depends_on`, pipeline) →
  `Task` + `task_pipeline_state` at `pending_manager_review` linked to the thread;
  `PAUSE_TASK` / `RESUME_TASK` / `CANCEL_TASK`; `HIRE` / `UPDATE_EMPLOYEE` / `FIRE`;
  `KNOWLEDGE` (3.5). `emit_task_created` goes out per new task, so the chat shows cards live.
- The prompt asks for **atomic tasks that do not touch the same files** (locked by
  `test_the_chat_prompt_asks_for_atomic_tasks`).
- Chat usage is billed through the **one** model entry point (see "one billing path" below).
  A chat turn deliberately writes **no** `engine_turns` row: that table has `task_id`/`run_id`
  FKs and a chat has neither; the usage events and the messages are the record.

### 3.2 Team management

- `hire(role, tier, name)` only hires one of `HIRABLE_ROLES`, refuses past
  `engine_max_employees`, counts **HIRE + UPDATE_EMPLOYEE + FIRE together** against
  `MAX_TEAM_CHANGES_PER_TURN = 3`, and picks a granted model of the requested tier
  **strictly** (`_pick_granted_model(..., fallback=False)`): a hire is an explicit
  request, so a silent downgrade would lie to the manager. No granted model of that
  tier → a Persian `ConflictError`.
- `fire()` refuses the **last manager**; a foreign employee id is a `NotFoundError`
  (the org-scoped repository get is what enforces isolation).
- `update_tier()` re-points the employee profile's model and publishes `team.changed`
  (`engine/events.py::emit_team_changed`, a generic realtime envelope — the frozen port
  has no typed emitter for it).
- Users still hire/fire from the existing UI endpoints with **role + tier only**; a model
  name never crosses the API.

### 3.3 Triage turn (manager, mode triage)

`triage()` reads the task, asks the model, and applies exactly one decision:

- `ASSIGN employee="…" pipeline="…"` → `_apply_assign`: fuzzy name match **inside the org
  only** (exact, then containment either way), or the **least busy** active non-manager
  employee when `employee=""`; the pipeline is resolved from the manager's choice and the
  task kind; the task moves to `pending_analysis` (or `pending_implementation` for
  `code_test`), and `task_pipeline_state.pipeline` is patched.
- `REJECT` → `cancelled`; `ASK_USER` → `pending_user_decision` **and** the question is posted
  in the task's thread as an assistant message (3.3).
- A reply with none of these is `acted=False` with the model's text as the reason.

### 3.4 Final verdict and error handling

- `final_review()` (mode final) sees the plan, diff, gate summary and test output:
  `COMPLETE` → `merge_request`; `REVISE` → cycle + 1 and back to `pending_implementation`,
  carrying the revision reason as the outcome message.
- `error_review()` (mode error): `RESOLVE_ERROR hint="…"` → retry the task with the hint,
  `CANCEL` → `cancelled`.

### 3.5 `KNOWLEDGE` → memory

`_record_knowledge()` records every `KNOWLEDGE` item through `MemoryPort.record(kind=rule|fact,
source="manager")`, on **every** manager turn (chat, triage, final, error).

### 3.6 Bootstrap — `WorkforceBootstrapPort`

- `ManagerTurns.bootstrap_team(org, project)` creates one manager + planner + developer +
  tester on the strongest / balanced / balanced / fast granted tiers, idempotent (it
  returns as soon as the org has any employee). With nothing granted the employees are
  created **inactive** and a system message `"منتظر تخصیص مدل توسط مالک سامانه"`
  (`NO_MODEL_GRANTED_MESSAGE`) is posted, so the page explains itself.
- **`container_engine.py` now wires the real bootstrap** (it was the foundation's no-op
  stub): `build_workforce_bootstrap(session)` returns an adapter that builds `ManagerTurns`
  lazily from the caller's session — the same session the org wizard commits on, so the
  team lands with the organization. Names and return types are the frozen contract; the
  body is mine to fill (PLAN-v2 §6).

### One billing path (agybot bug)

`call_model()` and `BillingScope` were extracted in `engine/runner.py` as **the** single
model entry point; `TurnRunner._complete` and the manager chat both go through it, and it
bills via `record_model_usage`. A model call that skips it is a call the platform never
charges for — which is exactly what the old manager chat did.

### Fixes to the Phase-1 protocol (prompt and protocol must agree)

Three real bugs surfaced while making the manager tests pass; each has a regression test:

1. **`[[REVISE]]reason[[/REVISE]]` was a protocol error.** `REVISE` was missing from
   `BODY_TAGS`, so the closing tag was reported as "stray" even though `prompt.py` tells the
   model to write exactly that. Added `REVISE` to `BODY_TAGS`.
2. **`ASSIGN employee=""` was rejected.** The docstring and `_pick_assignee("")` both mean
   "pick the least busy one", but `assign_of` used the stripped value as the presence test.
   It now distinguishes an **absent** attribute (still an error — `[[ASSIGN/]]`) from a
   **present-but-empty** one. The triage prompt now documents the empty form.
3. **The runner retried protocol errors forever.** A model that repeated the same mistake was
   fed the error back and called again with no cap, until the 900 s turn timeout — hundreds of
   paid calls and an unbounded prompt. New setting `engine_max_protocol_retries = 3` bounds
   it; exceeding it fails the turn with `error_kind=protocol`, `incomplete=True`.

### New setting (inside the agent-2 block of `config.py`)

`engine_max_protocol_retries`.

### Other support changes

- `TaskRepository.set_assignee()` (my repository file) — the assignee write is its own
  narrow method rather than a whole-row `upsert`.
- `engine/events.py`: `emit_team_changed()`.
- `tests/engine_fakes.py`: `FakeGrants` now takes either a list of ids (all `balanced`) or an
  id → tier mapping, so a test can have a strong and a fast model and assert which one the
  engine picked.

## Tests

`tests/test_engine_manager.py` (new, **20 tests**):

- 3.1: an order becomes tasks with pipeline state; the ≤ 3 cap (extras dropped, the valid
  three kept and the user told); the prompt asks for atomic tasks; a chat model call is
  billed; a chat without a grant answers with the waiting note.
- 3.2: hire picks a granted model of the tier; hire without that tier is refused; the
  employee cap; the last manager cannot be fired; a foreign employee is a 404.
- 3.3: triage assigns the least busy employee; a named employee is matched inside the org;
  the manager of org A can never assign org B's employee; `ASK_USER` parks the task and asks
  in the thread.
- 3.4: `COMPLETE` → `merge_request`; `REVISE` → back and cycle + 1; `RESOLVE_ERROR` retries
  with a hint.
- 3.5: knowledge tags become org memory (read back from `OrgMemoryRow`).
- 3.6: bootstrap creates the team and is idempotent; without grants it creates an inactive
  team and the waiting note.

Plus: `test_engine_runner.py` gains a regression test for the retry cap (a model that repeats
its mistake stops after `1 + engine_max_protocol_retries` calls), and `test_engine_protocol.py`
gains the two protocol-lock tests (`employee=""` is "pick for me"; `REVISE` carries a body).

## Checks (PLAN-v2 §7.4)

```
cd apps/api && .venv/bin/python -m pytest -q   → 722 passed, 1 skipped
cd apps/api && .venv/bin/ruff check .          → All checks passed!
cd apps/api && .venv/bin/mypy app              → Success: no issues found in 158 source files
```

No web files were touched; no migration was needed (Phase 1's `a2b3c4d5e6f2` already has
`task_pipeline_state.organization_id` and the engine tables).

## Deviations & notes

1. `post_user_message` delegates to `ManagerTurns.chat`; the old `emit_chat_pending` /
   `emit_message_created` calls on that path were **not** kept, because the engine's turn is
   synchronous with the request and the thread reload shows the reply. Removing them is a
   behaviour change to `workforce_service.py` (my file) and is covered by the workforce tests.
2. `ManagerTurns` is built inside `_manager_turns()` (and the bootstrap adapter) rather than
   injected: `container.py` is frozen, so the manager must not widen a constructor it does
   not own. Session-scoped ports (model grants, memory) are built from the shared session.
3. The manager chat writes no `engine_turns` row (no task/run FK); task turns do, through
   `TurnRunner`. Recorded here because the phase-2 report implied all turns leave a row.
4. `ASSIGN employee=""` now means auto-pick; the triage prompt documents it, so prompt and
   protocol still agree.
5. `tests/test_e2e_smoke.py` gained a model grant before the manager-chat step: PLAN-v2 D3
   says the engine runs only on granted models, so a chat with no grant now (correctly)
   answers nothing but the waiting note. This is a test fix, not a behaviour regression.

## Open questions / CHANGE REQUESTs

- None. The org automation policy (`automation_enabled`) is still behind
  `ModelGrantPort`, as planned: Phase 4 assumes enabled until the CHANGE REQUEST lands.

## Next

Phase 4 — `engine/scheduler.py::tick()`, worktree lifecycle, the gate chain (diff guard,
no-progress, tests, green gate), commit/merge/push, budget pause, and
`tests/test_engine_flow.py`.