fix(engine): bugs surfaced by manual Web-GUI verification
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) <noreply@anthropic.com>
This commit is contained in:
@@ -2,6 +2,28 @@
|
|||||||
|
|
||||||
## [Unreleased]
|
## [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
|
### Added
|
||||||
- **v0.2 PR #3 — FastAPI + SSE + minimal Web GUI (`mydeepagent serve`)**.
|
- **v0.2 PR #3 — FastAPI + SSE + minimal Web GUI (`mydeepagent serve`)**.
|
||||||
Localhost Web UI for run start / list / detail / resume / abort + live
|
Localhost Web UI for run start / list / detail / resume / abort + live
|
||||||
|
|||||||
@@ -603,7 +603,12 @@ class WorkflowEngine:
|
|||||||
artifact_path: Path,
|
artifact_path: Path,
|
||||||
) -> ApprovalDecisionAction:
|
) -> ApprovalDecisionAction:
|
||||||
request_id = uuid4()
|
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] = {
|
payload: dict[str, Any] = {
|
||||||
"phase_key": phase_def.key,
|
"phase_key": phase_def.key,
|
||||||
"artifact_path": str(artifact_path),
|
"artifact_path": str(artifact_path),
|
||||||
@@ -712,6 +717,13 @@ class WorkflowEngine:
|
|||||||
md_path = worktree_root / f"{run_id}.report.md"
|
md_path = worktree_root / f"{run_id}.report.md"
|
||||||
json_path.write_text(json.dumps(report, indent=2, ensure_ascii=False), encoding="utf-8")
|
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")
|
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
|
return json_path
|
||||||
|
|
||||||
# ------------------------------------------------------------------
|
# ------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user