diff --git a/backend/main.py b/backend/main.py index 839edc5..75ebbdc 100644 --- a/backend/main.py +++ b/backend/main.py @@ -10,7 +10,9 @@ import contextlib import os from contextlib import asynccontextmanager +from pathlib import Path from typing import Any +from uuid import uuid4 import socketio import structlog @@ -46,6 +48,20 @@ async def health() -> dict[str, str]: return {"status": "ok"} +def _run_db_path() -> str: + """A distinct SQLite file for every run. + + Each start_run boots its own state-server PROCESS, and the store's + single-writer guarantee is an in-process asyncio.Lock — it serialises + nothing across processes. Pointing two live runs at one file therefore puts + two OS writers on it, which surfaces as "database is locked" out of + claim_next_task and gets laundered into task retries that can fail the run. + APPFORGE_WEB_DB names the base path; each run takes a unique sibling of it. + """ + base = Path(os.getenv("APPFORGE_WEB_DB", "data/web.db")) + return str(base.with_name(f"{base.stem}-{uuid4().hex}{base.suffix}")) + + async def _poll_and_emit(project_id: str, room: str) -> None: ctx = _runs[project_id] handle: RunHandle = ctx["handle"] @@ -96,7 +112,7 @@ async def start_project(sid, data): idea, workers=4, budget_limit=200.0, - db_path=os.getenv("APPFORGE_WEB_DB", "data/web.db"), + db_path=_run_db_path(), ) project_id = handle.run_id room = f"project:{project_id}" diff --git a/docs/STATUS.md b/docs/STATUS.md index 5713c88..6b12599 100644 --- a/docs/STATUS.md +++ b/docs/STATUS.md @@ -7,23 +7,24 @@ ## State summary -**Version:** 1.0.0 (untagged) · **Branch:** `main` @ `74cadcf` · **Status: feature-frozen, one open defect blocking the release.** - -> **Active handoff:** [`docs/handoffs/2026-07-25-e2e-database-locked.md`](handoffs/2026-07-25-e2e-database-locked.md) — the `e2e` job is red with `database is locked`. The release is deliberately held until it is green. Read that brief before picking this up. +**Version:** 1.0.0 (untagged) · **Branch:** `fix/e2e-per-run-db` (PR open) · **Status: feature-frozen; the `e2e` blocker is fixed and awaiting merge.** AppForge v1.0 is the finished form of what this project set out to prove: a **parallel, MCP-coordinated multi-agent orchestration engine** in which a real MCP state server and a pool of independent OS worker processes drive a product idea through a six-phase dependency graph (Clarify → Design → Code → Test → Deploy → Iterate), with human approval gates and automatic budget-driven model downgrade. -The engine is done, tested, documented, and published under MIT. No new capability is planned — but v1.0.0 is **not yet tagged or released**: the `e2e` CI job fails with `database is locked`, and the user's decision this session was to hold the release until that is resolved. The `v1.0.0` tag exists locally at `ba82ca8` and **points at the wrong commit** (it predates the fresh-clone fix `7bfa00d`); it must be moved before it is ever pushed. +The engine is done, tested, documented, and published under MIT. No new capability is planned. The `e2e` blocker is resolved on `fix/e2e-per-run-db`: the Playwright suite is green locally (4/4, `data/` absent) with zero `database is locked` errors. Once that PR merges and CI is green on `main`, the release can proceed — the `v1.0.0` tag exists locally at `ba82ca8` and **points at the wrong commit** (it predates the fresh-clone fix `7bfa00d`); it must be moved to the release commit before it is ever pushed. + +> **Correction to the previous entry:** the `e2e` job was *not* red because of `database is locked`. It was red because all four Playwright specs predated the engine rewrite and asserted on the retired LangGraph chat flow (`Clarifying question #1` — "element(s) not found"). The lock errors were real but concurrent server-side noise. Both problems are fixed; see the session log below. ### Readiness | Signal | State | |---|---| -| Backend suite | **155 passed** (`uv run pytest tests/`) | -| Coverage | **86.96%** (gate: 70%) | +| Backend suite | **156 passed** (`uv run pytest tests/`) | +| Coverage | **86.99%** (gate: 70%) | | Frontend suite | **28 passed** across 6 files (`cd frontend && npm test`) | | Lint / format | `ruff check` clean · `black --check` clean (67 files) | -| CI on `main` | `backend` · `frontend` · `validate-config` green; **`e2e` red** (see gaps) | +| Browser e2e | **4 passed** in ~1.1m (`cd e2e && npx playwright test`), verified with `data/` absent | +| CI on `main` | `backend` · `frontend` · `validate-config` green; `e2e` red until `fix/e2e-per-run-db` merges | | Version | `pyproject.toml` 1.0.0 · `frontend/package.json` 1.0.0 · `backend/main.py` FastAPI 1.0.0 | | CLI | `uv run appforge run ""` (hatchling build backend, entry point installed) | | License | MIT (`LICENSE`) | @@ -46,15 +47,16 @@ The engine is done, tested, documented, and published under MIT. No new capabili - **Single-user web bridge.** The live UI targets local single-user use. Multi-tenant lifecycle hardening (per-connection dedup, reconnect durability) is unbuilt. - **Historical docs.** `docs/Roadmap.md`, `docs/CoreDesignDocument.md`, and the dated `Status-*.md` files describe the LangGraph-era design and are kept as history only. - **Shutdown noise.** A `CancelledError` traceback prints on CLI teardown when the state-server task is cancelled. Cosmetic — the run reports `done` and exits 0 — but it looks alarming. -- **`e2e` CI job is red — `database is locked`.** *Not* cosmetic and **not yet diagnosed.** The web bridge runs `start_run(workers=4)` against `data/web.db`; under CI those four workers hit lock contention and `claim_next_task` fails with `database is locked`. Ruling one thing out: a missing `busy_timeout` pragma is not the cause, since Python's `sqlite3.connect()` already applies a 5s busy timeout that `aiosqlite` inherits. This was masked until 2026-07-25 by the missing-`data/`-directory bug failing earlier in the same path. -- **`e2e` specs predate the engine.** Separately, the Playwright specs were last touched 2026-06-02 and still drive the retired LangGraph chat flow (`Clarifying question #N`, `Mock PRD`). Even once the lock contention is resolved, they likely need rewriting or retiring. +- **One scratch DB file per web run.** The per-run database fix leaves a `data/web-.db` (plus `-wal`/`-shm`) behind after each run. `data/` is gitignored scratch and the files are useful for post-hoc inspection, so they are deliberately *not* deleted on teardown — but they accumulate over a long local session. +- **Plan-gate content is not rendered live.** At the design gate the approval card shows "Approval needed" with no body: `PlanViewer` reads `adr`/`tasks`/`design_spec` from the store, which only the `project_state` (reload) path populates — the live `approval_required` event with `kind: "plan"` deliberately does not overwrite the approved PRD. Reloading shows it. Cosmetic, pre-existing, and out of scope for the e2e fix. ### Next steps -None required — the project is feature-frozen at 1.0. If it is picked up again, the highest-value candidates, in order: +1. **Merge `fix/e2e-per-run-db` and confirm CI is green on `main`** — then move the `v1.0.0` tag to the release commit, push it, and cut the GitHub Release. +2. Decide the leftovers the 2026-07-25 handoff left open: the two unmerged branches (`feat/parallel-mcp-orchestration-engine`, `windows-changes`) and the merged local `fix/create-db-parent-dir` — delete or keep. + +Beyond that the project is feature-frozen at 1.0. If it is picked up again: -1. **Diagnose the `database is locked` contention** that keeps the `e2e` job red — the only known functional defect, and the reason CI is not fully green. -2. Decide whether the LangGraph-era Playwright specs get rewritten against the engine's flow or retired. 3. Thread real Anthropic token usage into BudgetGuard so budget figures are actual, not simulated. 4. Harden the web bridge for multi-user / reconnect durability. 5. Refresh or archive the LangGraph-era design docs so `docs/` matches the shipped engine. @@ -64,6 +66,17 @@ None required — the project is feature-frozen at 1.0. If it is picked up again ## Session log +### 2026-07-25 — `e2e` unblocked: per-run databases + specs rewritten against the engine + +Worked [`docs/handoffs/2026-07-25-e2e-database-locked.md`](handoffs/2026-07-25-e2e-database-locked.md). It turned out to be **two** independent defects, and the handoff misattributed which one was red. + +- **What actually failed CI.** All four Playwright specs were last touched 2026-06-02 and asserted on the retired LangGraph chat flow — every failure in the job log is `getByText(/Clarifying question #1/)` → "element(s) not found". The `database is locked` tracebacks in the same log were real but were server-side noise, not the failing assertion. The engine runs the clarify Q&A loop *inside* the worker (`product_owner` auto-answers `clarifying_pm`), so the browser never sees per-question chat turns at all. +- **Specs rewritten, not retired.** The engine's flow is fully drivable through the UI, so retiring the job would have thrown away real coverage. `phase3/phase4.spec.ts` → `clarify-gate.spec.ts` (gate opens with the PRD rendered; the rejection loop re-runs the phase; reload rehydrates from the snapshot) and `full-run.spec.ts` (both gates approved, all six phases to completion). +- **The lock defect, confirmed and fixed.** The handoff's hypothesis was right: `backend/main.py` passed a constant `data/web.db` into *every* `start_run`, and each `start_run` boots its own state-server **process**. The store's single-writer guarantee is an `asyncio.Lock`, which serialises nothing across processes. Each run now gets a unique sibling of `APPFORGE_WEB_DB` — which is what makes the README's load-bearing "single SQLite writer" claim actually true, rather than true-only-when-one-run-is-alive. +- **Evidence (A/B, both directions).** Engine level, 4 concurrent runs: shared DB → **4/4 runs failed**, 2 workers killed by `database is locked`; per-run DBs → **4/4 done**, 13/13 tasks each, 0 lock deaths. Browser level, same suite: without the fix → passes but with **3** lock-induced worker crashes; with it → **0**. Contention also *launders* lock errors into task retries that can fail a run outright, so the unfixed build was a live flake source for the new suite. +- Backend suite **155 → 156** (coverage 86.96% → 86.99%); e2e **4 passed** in ~1.1m. Everything verified with `data/` absent. +- **State delta:** the last known functional defect is closed and the `e2e` job proves the shipped engine instead of a retired one — the release is unblocked pending merge. + ### 2026-07-25 — handoff written; release held on the `e2e` defect - PR #11 merged; `main` @ `74cadcf`. `backend`, `frontend`, and `validate-config` are green — `e2e` is not. diff --git a/e2e/README.md b/e2e/README.md index 08cc98f..ea55541 100644 --- a/e2e/README.md +++ b/e2e/README.md @@ -1,7 +1,17 @@ # Browser E2E (Playwright) -Drives the real React UI through the Phase 3 flows in chromium. Playwright -starts both servers automatically (FastAPI in `MOCK_AGENTS=true`, Vite dev). +Drives the real React UI against the MCP orchestration engine in chromium. +Playwright starts both servers automatically (FastAPI in `MOCK_AGENTS=true`, +Vite dev), and each spec drives a live engine run — a state server plus four +worker subprocesses per project. + +- `clarify-gate.spec.ts` — idea → clarify phase → PRD approval gate; the + rejection loop; and snapshot rehydration after a reload. +- `full-run.spec.ts` — both gates approved, all six phases run to completion. + +Note the clarify Q&A loop runs *inside* the worker (`product_owner` +auto-answers `clarifying_pm`), so the browser goes straight from "Start" to the +PRD gate rather than exchanging questions in the chat panel. ## Run diff --git a/e2e/tests/clarify-gate.spec.ts b/e2e/tests/clarify-gate.spec.ts new file mode 100644 index 0000000..76674c7 --- /dev/null +++ b/e2e/tests/clarify-gate.spec.ts @@ -0,0 +1,65 @@ +import { test, expect, type Page } from "@playwright/test"; + +// The engine runs the clarify Q&A loop INSIDE the worker process — the +// product_owner agent auto-answers the clarifying_pm's questions server-side +// (backend/engine/agent_adapter.py::_run_clarify_loop). So the browser never +// sees per-question chat turns the way the retired LangGraph flow did: it goes +// straight from "Start" to the PRD approval gate. +// +// Budget: the clarify task is ~4 mock agent calls at ~1s each, plus state +// server boot and 4 worker subprocess spawns. Generous on a cold CI runner. +const GATE_TIMEOUT = 45_000; + +async function startProject(page: Page) { + await page.goto("/"); + await page.fill('input[name="idea"]', "Build me a todo app"); + await page.getByRole("button", { name: "Start" }).click(); + // create_run ids are uuid4().hex — 32 hex chars, no dashes. + await expect(page).toHaveURL(/\/project\/[0-9a-f]{32}$/); +} + +async function expectPrdGate(page: Page) { + await expect(page.getByText("Approval needed")).toBeVisible({ + timeout: GATE_TIMEOUT, + }); + // PRDViewer renders the stored `prd` markdown; the mock writes "# Mock PRD". + await expect(page.getByRole("heading", { name: /^Mock PRD$/ })).toBeVisible(); +} + +test("idea -> clarify phase runs -> PRD gate opens with the PRD rendered", async ({ + page, +}) => { + await startProject(page); + await expectPrdGate(page); + // The clarify phase completing and its gate opening arrive in the same poll + // batch (store._advance_locked sets status=complete and gate=pending in one + // transaction), so this message is already present alongside the card. + await expect(page.getByText(/Phase 3 success: clarify complete/)).toBeVisible(); +}); + +test("reject re-opens the clarify phase and the gate returns", async ({ page }) => { + await startProject(page); + await expectPrdGate(page); + + // Rejecting sets gate=rejected, re-opens the phase and resets its tasks to + // blocked -> ready, so clarifying_pm is claimed and run a second time. + await page.getByRole("button", { name: "Reject" }).click(); + await expect(page.getByText(/Phase 3 success: clarify complete/)).toHaveCount(2, { + timeout: GATE_TIMEOUT, + }); + await expect(page.getByText("Approval needed")).toBeVisible(); +}); + +test("reload rehydrates the pending gate from the engine snapshot", async ({ + page, +}) => { + await startProject(page); + await expectPrdGate(page); + + await page.reload(); + // ProjectWorkspace calls load_project on mount; the backend replies with a + // project_state built from the last engine snapshot, which carries + // approval_pending and the PRD. Generous timeout for the socket reconnect. + await expect(page.getByText("Approval needed")).toBeVisible({ timeout: 15_000 }); + await expect(page.getByRole("heading", { name: /^Mock PRD$/ })).toBeVisible(); +}); diff --git a/e2e/tests/full-run.spec.ts b/e2e/tests/full-run.spec.ts new file mode 100644 index 0000000..722d2c8 --- /dev/null +++ b/e2e/tests/full-run.spec.ts @@ -0,0 +1,34 @@ +import { test, expect } from "@playwright/test"; + +// Drives the whole six-phase DAG (config/phases.yaml) through the browser: +// clarify -> [gate] -> design -> [gate] -> code -> test -> deploy -> iterate. +// Both gates are approved from the UI; the run then finishes on its own and the +// poller emits a terminal phase_complete with the summary "run done". +test("approve both gates -> all six phases run to completion", async ({ page }) => { + test.setTimeout(120_000); // full pipeline: 13 mock tasks across 4 worker processes + + await page.goto("/"); + await page.fill('input[name="idea"]', "Build me a todo app"); + await page.getByRole("button", { name: "Start" }).click(); + await expect(page).toHaveURL(/\/project\/[0-9a-f]{32}$/); + + // --- clarify gate --- + await expect(page.getByText("Approval needed")).toBeVisible({ timeout: 45_000 }); + await expect(page.getByRole("heading", { name: /^Mock PRD$/ })).toBeVisible(); + await page.getByRole("button", { name: "Approve" }).click(); + + // Approving opens the design phase, whose three agents (solution_architect, + // tech_lead, uiux_designer) have no intra-phase dependencies and so are + // claimed in parallel by separate worker processes. + await expect(page.getByText(/Phase 4 success: design complete/)).toBeVisible({ + timeout: 45_000, + }); + + // --- design (plan) gate --- + await page.getByRole("button", { name: "Approve" }).click(); + + // The remaining phases carry gate: none, so they run unattended to the end. + await expect(page.getByText(/Phase 10 success: run done/)).toBeVisible({ + timeout: 60_000, + }); +}); diff --git a/e2e/tests/phase3.spec.ts b/e2e/tests/phase3.spec.ts deleted file mode 100644 index 10e9490..0000000 --- a/e2e/tests/phase3.spec.ts +++ /dev/null @@ -1,57 +0,0 @@ -import { test, expect, type Page } from "@playwright/test"; - -// Submit an idea and answer the three mock clarifying questions, leaving the -// page parked on the approval gate with the base PRD shown. -async function driveToApprovalGate(page: Page) { - await page.goto("/"); - await page.fill('input[name="idea"]', "Build me a todo app"); - await page.getByRole("button", { name: "Start" }).click(); - - for (const n of [1, 2, 3]) { - await expect( - page.getByText(new RegExp(`Clarifying question #${n}`)), - ).toBeVisible(); - // Chat input placeholder is long; match a stable prefix (regex, not exact). - await page.getByPlaceholder(/Describe your idea/).fill(`answer ${n}`); - await page.locator('button[type="submit"]', { hasText: "Send" }).click(); - } - - await expect(page.getByText("Approval needed")).toBeVisible(); - await expect(page.getByRole("heading", { name: /^Mock PRD$/ })).toBeVisible(); -} - -test("happy path: idea -> clarify -> approve -> phase complete", async ({ page }) => { - await driveToApprovalGate(page); - await page.getByRole("button", { name: "Approve" }).click(); - await expect(page.getByText(/Phase 3 success/)).toBeVisible(); - // NOTE: with ENABLE_PHASE4=true (set in playwright.config.ts webServer.env for - // the whole run), approving the PRD does NOT end the flow — the planning sprint - // runs and a fresh "plan" approval gate re-opens. The old post-approve - // toBeHidden("Approval needed") would race that re-open and is therefore wrong - // here. The full phase-4 path (plan renders -> approve plan -> card hidden) is - // covered end-to-end by phase4.spec.ts; this spec only asserts the preserved - // phase-3 success emit. -}); - -test("revision cycle: reject then modify -> revised PRD", async ({ page }) => { - await driveToApprovalGate(page); - // Plain Reject re-prompts (no comment -> unchanged PRD). Card stays mounted. - await page.getByRole("button", { name: "Reject" }).click(); - // Modify carries a comment, which the mock stamps as "(revision 1)". - await page.getByRole("button", { name: "Modify" }).click(); - await page.getByPlaceholder("What should be changed?").fill("add auth"); - await page.getByTestId("modify-send").click(); - await expect( - page.getByRole("heading", { name: /Mock PRD \(revision 1\)/ }), - ).toBeVisible(); -}); - -test("reload resume: hydrate from snapshot after reload", async ({ page }) => { - await driveToApprovalGate(page); - await expect(page).toHaveURL(/\/project\/[0-9a-f-]+$/); - await page.reload(); - // ProjectWorkspace calls load_project on mount; backend load_snapshot - // reconstructs approval_pending + prd. Generous timeout for reconnect. - await expect(page.getByText("Approval needed")).toBeVisible({ timeout: 15_000 }); - await expect(page.getByRole("heading", { name: /^Mock PRD$/ })).toBeVisible(); -}); diff --git a/e2e/tests/phase4.spec.ts b/e2e/tests/phase4.spec.ts deleted file mode 100644 index 6d61bc7..0000000 --- a/e2e/tests/phase4.spec.ts +++ /dev/null @@ -1,53 +0,0 @@ -import { test, expect, type Page } from "@playwright/test"; - -// Submit an idea and answer the three mock clarifying questions, leaving the -// page parked on the PRD approval gate. Copied from phase3.spec.ts (a small -// helper is cheaper to duplicate than to share across spec files). -async function driveToApprovalGate(page: Page) { - await page.goto("/"); - await page.fill('input[name="idea"]', "Build me a todo app"); - await page.getByRole("button", { name: "Start" }).click(); - - for (const n of [1, 2, 3]) { - await expect( - page.getByText(new RegExp(`Clarifying question #${n}`)), - ).toBeVisible(); - await page.getByPlaceholder(/Describe your idea/).fill(`answer ${n}`); - await page.locator('button[type="submit"]', { hasText: "Send" }).click(); - } - - await expect(page.getByText("Approval needed")).toBeVisible(); - await expect(page.getByRole("heading", { name: /^Mock PRD$/ })).toBeVisible(); -} - -// With ENABLE_PHASE4=true the suite runs the full pipeline: PRD approval → -// planning fan-out (ADR / tasks / design) → plan approval gate → phase 4 done. -test("phase 4 happy path: approve PRD -> plan renders -> approve plan -> done", async ({ - page, -}) => { - await driveToApprovalGate(page); - - // Approve the PRD. Backend emits "Phase 3 success: PRD approved" then begins - // the planning sprint. - await page.getByRole("button", { name: "Approve" }).click(); - await expect(page.getByText(/Phase 3 success/)).toBeVisible(); - - // Planning artifacts render via PlanViewer. The ADR "# ADR" markdown renders - // as a heading; "Build the backend" is one of the mock task titles. Wait for - // these BEFORE the plan card, since the approval card briefly clears on the - // phase-3 phase_complete before re-opening for the plan gate. - await expect(page.getByTestId("plan-viewer")).toBeVisible({ timeout: 15_000 }); - await expect(page.getByRole("heading", { name: /^ADR$/ })).toBeVisible(); - await expect(page.getByText(/Build the backend/)).toBeVisible(); - - // The new planning approval gate (kind == "plan") opens. - await expect(page.getByText("Approval needed")).toBeVisible({ timeout: 15_000 }); - - // Approve the plan gate. Summarizer emits "Phase 4 success: Planning approved". - await page.getByRole("button", { name: "Approve" }).click(); - await expect(page.getByText(/Phase 4 success/)).toBeVisible(); - await expect(page.getByText(/Planning approved/)).toBeVisible(); - - // After the final phase_complete the approval card is gone. - await expect(page.getByText("Approval needed")).toBeHidden(); -}); diff --git a/tests/integration/test_web_bridge.py b/tests/integration/test_web_bridge.py index 4f19042..af018fb 100644 --- a/tests/integration/test_web_bridge.py +++ b/tests/integration/test_web_bridge.py @@ -1,3 +1,5 @@ +from pathlib import Path + import pytest import socketio import uvicorn @@ -8,6 +10,26 @@ def mock_mode(monkeypatch): monkeypatch.setenv("MOCK_AGENTS", "true") +def test_each_run_gets_its_own_database(monkeypatch, tmp_path): + """Concurrent runs must never share one SQLite file. + + Every start_run boots its own state-server PROCESS, and the store's + single-writer guarantee is an in-process asyncio.Lock — it serialises + nothing across processes. Two live runs pointed at one file therefore put + two OS writers on it, which surfaces as "database is locked" out of + claim_next_task (and gets laundered into task retries that fail the run). + """ + from backend.main import _run_db_path + + monkeypatch.setenv("APPFORGE_WEB_DB", str(tmp_path / "web.db")) + paths = [_run_db_path() for _ in range(4)] + + assert len(set(paths)) == 4 # every run gets its own writer + for p in paths: + assert Path(p).parent == tmp_path # still honours APPFORGE_WEB_DB + assert Path(p).suffix == ".db" + + async def _serve_app(port): from backend.main import asgi_app