From ac428ba7477b8f31d54215e5b9dba28d70c4219a Mon Sep 17 00:00:00 2001 From: chungyeong Date: Sun, 17 May 2026 18:52:35 +0900 Subject: [PATCH] fix(engine): bugs surfaced by manual Web-GUI verification MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both bugs landed during `mydeepagent serve` + real OpenRouter run via /api/runs. Neither was caught by the test suite — each test uses a fresh sqlite tmp_path or per-test Postgres DB, so a "second run against existing data" code path was never exercised. 1. `_compose_final_report` did not persist `RunRow.final_report_path` - CLI users received the path from the RunResult return value and never noticed. - API/GUI users read from the DB → got `null` → no link to the report showed up in the run detail page. - Fix: at the end of `_compose_final_report`, open a DB session, load the RunRow, set `final_report_path = str(json_path)`, commit. Both code paths now see the path. 2. `_run_approval_gate` built `idempotency_key = f"{phase_key}:{artifact_name}"` - The 2nd run of the same workflow on a populated DB hit `approval_requests_idempotency_key_key` UNIQUE violation on the first approval gate (`spec:spec.json` already existed from the previous run). - The background task died; the run stayed `executing` forever; the GUI loop never updated. - Fix: prefix with `run_id`: `f"{run_id}:{phase_key}:{artifact_name}"`. Same-run replay (resume / repair retry) still collides idempotently as intended. ApprovalDecisionRow inherits the new key shape automatically. Verification - 4th /api/runs POST against the populated Postgres DB completed in ~2 min, spec + review + verify all `completed`, 3 artifacts schema-valid, and `RunRow.final_report_path` now resolves to the report .json path. - Gates: ruff / mypy --strict / 19 engine+resume+wiring tests PASS. Co-Authored-By: Claude Opus 4.7 (1M context) --- my-deepagent/CHANGELOG.md | 22 ++++++++++++++++++++++ my-deepagent/src/my_deepagent/engine.py | 14 +++++++++++++- 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/my-deepagent/CHANGELOG.md b/my-deepagent/CHANGELOG.md index cdb34e2..04ce164 100644 --- a/my-deepagent/CHANGELOG.md +++ b/my-deepagent/CHANGELOG.md @@ -2,6 +2,28 @@ ## [Unreleased] +### Fixed +- **bugfix(engine): two production bugs surfaced by manual Web-GUI verification + (`mydeepagent serve` + real OpenRouter run via /api/runs)**. + 1. `_compose_final_report` wrote the report files to disk and returned the + path but **did not update `RunRow.final_report_path`**. CLI users + received the path via the `RunResult` return value and never noticed; + API/GUI users read from the DB and got `null`. Fix: after writing the + JSON / MD report, update the RunRow column in a fresh session inside + `_compose_final_report` itself so both consumers see the path. + 2. `_run_approval_gate` built `idempotency_key = f"{phase_key}:{artifact_name}"` + for `approval_requests`. The column has a UNIQUE constraint, so the + **second run** of the same workflow against a non-empty DB raised + `asyncpg.UniqueViolationError` on the first approval gate — the + background task died, the run stayed `executing` forever, the GUI + never updated. Confirmed by reading the server log after the GUI got + stuck on a 2nd run. Fix: prefix with `run_id` + (`f"{run_id}:{phase_key}:{artifact_name}"`) so each run has its own + key namespace; same-run replay still collides idempotently as intended. + E2E + integration tests did not catch this because each test uses a + fresh sqlite tmp_path or per-test Postgres DB — no second run ever + hit the same table. + ### Added - **v0.2 PR #3 — FastAPI + SSE + minimal Web GUI (`mydeepagent serve`)**. Localhost Web UI for run start / list / detail / resume / abort + live diff --git a/my-deepagent/src/my_deepagent/engine.py b/my-deepagent/src/my_deepagent/engine.py index eb9ee03..ef17dff 100644 --- a/my-deepagent/src/my_deepagent/engine.py +++ b/my-deepagent/src/my_deepagent/engine.py @@ -603,7 +603,12 @@ class WorkflowEngine: artifact_path: Path, ) -> ApprovalDecisionAction: request_id = uuid4() - idem_key = f"{phase_def.key}:{artifact_path.name}" + # idempotency_key must include run_id — without it, two separate runs + # of the same workflow trip the `approval_requests_idempotency_key_key` + # UNIQUE constraint on the second run's first approval gate. + # Reproduced 2026-05-17 via /api/runs POST against a fresh DB + + # second run (first run's spec:spec.json key already present). + idem_key = f"{run_id}:{phase_def.key}:{artifact_path.name}" payload: dict[str, Any] = { "phase_key": phase_def.key, "artifact_path": str(artifact_path), @@ -712,6 +717,13 @@ class WorkflowEngine: md_path = worktree_root / f"{run_id}.report.md" json_path.write_text(json.dumps(report, indent=2, ensure_ascii=False), encoding="utf-8") md_path.write_text(_render_report_md(report), encoding="utf-8") + # Persist the report path on RunRow so the API/GUI can surface it + # (CLI reads it from the RunResult return value; GUI reads via DB). + async with self._db.session() as s: + row = await s.get(RunRow, str(run_id)) + if row is not None: + row.final_report_path = str(json_path) + await s.commit() return json_path # ------------------------------------------------------------------