mjalmoudy94
36564e3f2b
The AI workforce engine's driver. One tick moves every ready task one step:
a turn, a gate, or a platform merge. The AI never runs git.
- scheduler.py: tick() over TURN_STAGES/MERGE_STAGES; selection caps
(per-org 3, global 8); optimistic advance so two workers cannot both win;
worktree created for code pipelines, released on completion/cancel/failure
(kept during error_review); budget pause before any turn is spent; merge
conflict back-off; transient-failure retries then the manager's error turn.
- gates.py: diff guard (empty/.git/CI/symlink/oversize), no-progress hash,
tests in the sandbox, green gate that only trusts a green tests row or an
explicit waiver. Every verdict is a gate_results row + a task.gate event.
Fixed forbidden_paths: lstrip("./") stripped all leading dots, hiding
.git and .github from the guard.
- container_engine.build_scheduler + worker drives the engine tick.
- ManagerOutcome.error_kind; TaskSpendRepository.spend_for_task.
- New settings engine_max_concurrent_turns, engine_max_turns_per_org,
engine_tests_timeout_seconds, engine_max_merge_conflicts,
engine_max_task_credits (+ .env.example).
- tests: test_engine_gates.py (13), test_engine_flow.py (11).
Checks: pytest 746 passed/1 skipped, ruff clean, mypy clean (160 files).
198 lines
10 KiB
Markdown
198 lines
10 KiB
Markdown
# Agent 2 — Phase 4 report: the scheduler (one tick → one step → merged code)
|
|
|
|
**Date:** 2026-10-14 · **Branch:** `dev` · **Status:** done, all checks green
|
|
|
|
## What was done
|
|
|
|
### 4.1 The tick — `app/application/engine/scheduler.py` (new)
|
|
|
|
`Scheduler.tick()` is the engine's driver. One tick:
|
|
|
|
1. reads every task whose stage **waits for a turn** (`TURN_STAGES`) and every task
|
|
ready to **merge** (`MERGE_STAGES = (merge_request,)`),
|
|
2. applies the selection caps, then
|
|
3. runs **one** step per selected task — a turn, or the platform's merge.
|
|
|
|
A step never aborts the tick: `_guarded` catches, records and continues. A lost
|
|
optimistic race surfaces as `ConflictError` and is counted as *skipped*, because
|
|
"another worker moved it first" is a normal outcome, not a failure.
|
|
|
|
**Selection caps, not execution caps.** `_select` honours
|
|
`engine_max_turns_per_org` (3) and `engine_max_concurrent_turns` (8) by *choosing*
|
|
that many tasks per tick. Turns themselves run one at a time inside a tick: an
|
|
`AsyncSession` is not safe to share across concurrent turns, and correctness
|
|
across worker processes comes from the optimistic `advance` (two workers ticking
|
|
the same task both try; exactly one wins). This is a deliberate deviation from a
|
|
literal reading of "run 8 turns at once" — see *Deviations*.
|
|
|
|
`TickReport` (selected / acted / skipped / failed / merged / paused, decisions,
|
|
notes) is what the worker logs and the tests assert on.
|
|
|
|
### 4.2 Worktree lifecycle
|
|
|
|
`_worktree_for` calls `RepoWorkspacePort.create_task_worktree(project_id, task_id,
|
|
base="dev")` for any pipeline that `requires_worktree` (i.e. not `analysis_only`)
|
|
and a task with a `project_id`; otherwise `None`. The branch is saved onto the task
|
|
state (`engine.state` → `task_pipeline_state.branch`) the moment the developer's
|
|
diff is accepted, so a merged task records *what* it merged.
|
|
|
|
The worktree is removed on **completion, cancellation and failure** (TODO 4.2):
|
|
`_merge_step` removes it after a successful push, and `_move` calls
|
|
`_release_worktree` whenever the target is `failed`/`cancelled`. It is *kept*
|
|
during `error_review`, so an `error_review` retry still has the code to move
|
|
forward.
|
|
|
|
### 4.3 The gate chain — `app/application/engine/gates.py` (new)
|
|
|
|
A `WORK_REPORT` is a claim, not evidence. Four gates check it, and **every verdict
|
|
is a `gate_results` row and a `task.gate` event** — a gate that only lives in a log
|
|
is unauditable.
|
|
|
|
- **diff guard** — empty diff for a code task; `.git`/CI paths; symlinks escaping
|
|
the repo; files over `MAX_FILE_BYTES` (1 MiB).
|
|
- **no-progress** — hashes the diff; the same hash twice in a row means the agent
|
|
is spinning, so the task goes back to the developer (and eventually to the
|
|
manager via `max_cycles`). `no_progress` returns the new `(hash, count)`, which
|
|
the scheduler persists on the state.
|
|
- **tests** — the app profile's `test_command` runs in the sandbox with
|
|
`engine_tests_timeout_seconds`; the failure tail is stored and handed back. No
|
|
command defined → green (nothing to run is not a failure).
|
|
- **green** — the tester's `TEST_PASS` is only believed when the tests gate is
|
|
green, or when the tester **waives** it with a reason. A waiver is stored on the
|
|
state (`waiver_reason`) and in the gate row; it is never inferred.
|
|
|
|
`GateRunner.record(task, outcome)` is the public entry the scheduler uses to write
|
|
a verdict it reasoned about itself (a merge conflict is a diff-guard verdict that
|
|
comes from git, not from hashing).
|
|
|
|
`_escalate_after_repeated_red`: a **second** red `tests` row moves the developer
|
|
onto the strongest *granted* model (one escalation only; no stronger model granted
|
|
→ left alone).
|
|
|
|
### 4.4 Commit / merge / push — the platform owns git
|
|
|
|
The AI never runs git. After `final_review → COMPLETE` the task reaches
|
|
`merge_request`, and `_merge_step` does the git:
|
|
|
|
1. `commit` in the worktree, author `"<employee> <platform@smart-org.local>"`
|
|
(`ENGINE_COMMIT_EMAIL`, matching the delivery adapter),
|
|
2. `sync_with_base`,
|
|
3. `merge_into(branch, target="dev")`,
|
|
4. `push(branch="dev")` → `diff_guard` note,
|
|
5. `move → completed`, `remove_worktree`, `DeliveryHooks.on_branch_pushed(source="engine")`,
|
|
6. `MemoryPort.on_task_finished`.
|
|
|
|
A merge conflict is retried by sending the task back; after
|
|
`engine_max_merge_conflicts` (3) the manager takes over (`error_review`). A failed
|
|
push is `ErrorKind.NETWORK` → `error_review`.
|
|
|
|
### 4.5 Budget
|
|
|
|
Before any turn, `_budget_reason` checks the org's wallet. A wallet **present** with
|
|
`balance <= 0` pauses the task (`Stage.PAUSED`, status `waiting_approval`) with the
|
|
Persian note `BUDGET_PAUSED_MESSAGE` — no turn is spent. A **missing** wallet means
|
|
no budget was ever configured, and a fresh org must not be frozen out of its first
|
|
task (so `get_for_org`, never `ensure`, and `None` is not a pause). An optional
|
|
per-task ceiling (`engine_max_task_credits > 0`) reads projected spend from
|
|
`TaskSpendRepository.spend_for_task` (a read-only join of the usage ledger to the
|
|
task's runs).
|
|
|
|
### 4.6 Flow test — `tests/test_engine_flow.py` (new)
|
|
|
|
The end-to-end test: real repositories, real scheduler, real manager, real turn
|
|
runner; only the model (scripted), the sandbox (scripted) and git
|
|
(`FakeRepoWorkspace`) are fakes. The happy path walks
|
|
`manager_review → analysis → implementation → test → final_review → merge →
|
|
completed` one tick at a time and asserts the stage, the coarse status, the gate
|
|
rows, the git calls (author, target, branch), the removal of the worktree, the
|
|
delivery hook and the memory callback. Eleven tests cover: the happy path; git
|
|
never flowing through the model; red tests → back → then green; a forbidden diff
|
|
never reaching git; a tester failure; a `TEST_PASS` with no green tests; the
|
|
`analysis_only` pipeline (no worktree ever); the budget pause; dependencies; a
|
|
transient provider failure retried then handed to the manager; and a merge conflict.
|
|
|
|
### 4.7 Wiring — `container_engine.build_scheduler`, the worker, `.env.example`
|
|
|
|
- `build_scheduler(session)` assembles the runner, manager, gate runner, git
|
|
workspace, sandbox, delivery hooks, wallet and per-task spend repos on the
|
|
caller's session — the same rule as the bootstrap (`container.py` is frozen, so
|
|
the engine wires itself in my file).
|
|
- `sandbox_session_for` reuses the **active** sandbox session of the task's project
|
|
(`_EngineSandboxSessions`). The engine does not create sandboxes; when there is
|
|
none, the tests gate goes red and sends the work back — never "merge untested
|
|
because there was no container".
|
|
- `workers/ai_runtime.py` now drives **both** engines in one pass: the legacy run
|
|
queue, then one `Scheduler.tick()`. A broken tick is recorded against the worker
|
|
and never looks like a clean pass. `__main__.py` defaults the `ai-runtime`
|
|
interval to `engine_tick_seconds`; the one-pass (cron) default is unchanged.
|
|
- `.env.example` gains the six new `ENGINE_*` settings.
|
|
|
|
### Bug found and fixed by the tests
|
|
|
|
`forbidden_paths` normalised with `path.lstrip("./")`, which strips **all** leading
|
|
`.` and `/` characters: `.git/config` became `git/config` and
|
|
`.github/workflows/ci.yml` became `github/...`, so the diff guard would have
|
|
silently allowed changes to git internals and CI configuration. It now strips a
|
|
single leading `./` prefix. Caught by `test_forbidden_paths_flag_git_internals_and_ci`.
|
|
|
|
### Other support changes
|
|
|
|
- `ManagerOutcome.error_kind` — the manager carries the runner's error vocabulary
|
|
so the scheduler can tell a transient provider failure from a permanent one
|
|
without re-classifying a string.
|
|
- `TaskSpendRepository` (`repositories/engine.py`) — `spend_for_task`, read-only.
|
|
- `Scheduler._patch` refreshes the caller's snapshot version after each write, so a
|
|
second patch in the same step locks against the version the first write produced
|
|
instead of silently losing the write to `StaleStageError`.
|
|
- `tests/engine_fakes.py`: `RecordingMemory` now records `record` and
|
|
`on_task_finished` (it previously returned `None` and asserted nothing).
|
|
|
|
## Tests
|
|
|
|
`tests/test_engine_gates.py` (new, **13 tests**): pure helpers (`diff_hash`,
|
|
`forbidden_paths`, `code_task`); the diff guard's four refusals; no-progress; the
|
|
tests gate with and without a command; the green gate's waiver rule.
|
|
|
|
`tests/test_engine_flow.py` (new, **11 tests**): see 4.6.
|
|
|
|
Together **+24 tests**, all through the real repositories.
|
|
|
|
## Checks (PLAN-v2 §7.4)
|
|
|
|
```
|
|
cd apps/api && .venv/bin/python -m pytest -q → 746 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 160 source files
|
|
```
|
|
|
|
No web files were touched; no migration was needed (Phase 1's `a2b3c4d5e6f2` already
|
|
has the engine tables).
|
|
|
|
## Deviations & notes
|
|
|
|
1. **Concurrency is a selection cap, not 8 live turns.** A tick runs at most one
|
|
turn per task, sequentially. An `AsyncSession` is not concurrency-safe, so
|
|
running several turns on one session in parallel would be a corruption bug, not
|
|
a speed feature. `engine_max_concurrent_turns` / `engine_max_turns_per_org`
|
|
therefore bound how many tasks are *chosen*, and multi-process safety comes from
|
|
the optimistic `advance`. Recorded because it is a deliberate reading of 4.1.
|
|
2. **The engine does not create sandbox workspaces.** It reuses the active session
|
|
of the task's project; creating one automatically is a product decision the plan
|
|
does not make. Absent session → tests gate red → work sent back.
|
|
3. **The commit author e-mail is fixed** (`platform@smart-org.local`) and matches the
|
|
delivery adapter; the *name* is the employee's, which is what a reviewer reads.
|
|
4. `no_progress`/diff verdicts are persisted on the task state
|
|
(`last_diff_hash`, `same_diff_count`), so progress is judged across ticks and
|
|
across worker processes, not within one process.
|
|
|
|
## Open questions / CHANGE REQUESTs
|
|
|
|
- None new. The org automation policy (`automation_enabled`) is still behind
|
|
`ModelGrantPort` as planned.
|
|
|
|
## Next
|
|
|
|
Phase 5 — the web side: the engine board, the task page (live log, gates, diff),
|
|
`/workforce`, `/dashboard` + `/traces`, `src/test/engine-*.test.tsx`, and the final
|
|
workstream report.
|