Skip to content

9559e7d4 - Build the error-fix fixer: script-driven implement loop from an error.fix conclusion #53

Description

@Danswar

Plan for the "fixer" capability of the error-catching pipeline: takes an error.fix conclusion from the decide-loop (#48-adjacent, being built on projects-19's branch) and drives it to a draft PR mechanically — script + grok/codex CLI only, no Claude session, no human, in the loop.

EN: Two real ledger gaps need closing: spec_written is currently source-gated to humans only (chain.py HUMAN_KEYS + step kind), and cmd_run doesn't interpret reviewer/PR-reviewer lane output into pass/fail. Plan below scopes both narrowly. Related to #33 (not a blocker, not fixed here).

DE: Zwei echte Lücken in der Akte müssen geschlossen werden: spec_written ist aktuell nur für Menschen freigegeben (chain.py HUMAN_KEYS + Step-kind), und cmd_run interpretiert Reviewer-/PR-Reviewer-Lane-Output nicht zu bestanden/durchgefallen. Der Plan unten grenzt beides eng ein. Bezug zu #33 (kein Blocker, wird hier nicht gefixt).

Full plan

Goal

Take an error.fix conclusion (written by the decide-loop, agent#48's capability 1, currently being built by peer session projects-19 on branch 2f09ad4a...-error-decide-dispatcher) and drive it to a draft PR with no Claude session, no human, in the loop — only script + grok/codex CLI, mechanically. Repo: DFXswiss/agent, base develop.

Read projects-19's in-progress (uncommitted) diff to error_fix_act.py/main.py/DESIGN.md before writing this — it already answers most of the open questions below.

What already exists and needs no new code

  • scan_error_fix (error_fix_act.py) already turns a pending error.fix row into a find-or-create implement task + isolated worktree ($AGENT_HOME/error-fix-work/<task_id>). Ownership is now device-scoped, not session-scoped, per projects-19's branch — any session on this device can act on any task on this device. Ledger tasks created by scan_error_fix are assigned session_id = error-decide-<id8> (the decide session), which is deliberately left status: active forever (never session closed) specifically so later steps can keep acting under it. _require_task_session_active only checks status == "active", not heartbeat recency — so the fixer needs no session of its own and no heartbeat babysitting. It drives the existing task directly.
  • cmd_run (main.py:~2501) is a real, working single-step executor: resolves the next open step, runs script steps (local_check_pass via AGENT_CHECK_COMMAND/pytest -q, pushed via git push, mergeable), and for role=="implementer" already launches the grok lane (lane.py:launch) and auto-closes implementer_done on STATUS: complete. This is the skeleton the fixer's outer loop wraps — not a rewrite.
  • pr.open (opening the actual GitHub draft) is an ungated activity type (no HUMAN_KEYS-style source restriction) processed by agent github pending (github_act.py). It just needs a well-formed title/body. This is a templating problem, not a ledger-permission problem — no rule change needed here, unlike spec_written.

Real gaps to build

1. spec_written ledger rule (the "known open problem")

spec_written is blocked from script authorship by two independent checks: chain.py's _step("spec_written", ..., "human") (kind) and the flat HUMAN_KEYS frozenset (checked by key name regardless of workflow). Fix, scoped and explicit — not a blanket removal:

  • Extend _chain_snapshot() (main.py:2145) to include "payload": task.get("payload") or {}.
  • In chain.py, add a narrow carve-out inside close_allowed: when key == "spec_written", source == "script", and snapshot["payload"].get("error_id") is set (the existing marker _find_or_create_implement_task already stamps on every error-fix-originated task — no new field), accept it alongside the existing source == "human" path. Every other implement task (human-authored, no error_id in payload) is completely unaffected — spec_written stays human-only there.
  • Evidence is still mandatory: f"auto spec from error.fix brief (error_id={error_id[:8]})".
  • Document the exception as a new DESIGN.md §21.5 sub-point. (The dfx:agent-ledger Claude-Code plugin skill doc also describes spec_written as flatly human-only — that's a separate plugin, not this repo; flagging it as stale after this ships is a follow-up, not part of this PR.)

2. Spec authoring from the brief

payload.brief (confirmed field name/shape from projects-19's branch: short text — what's broken, likely cause, where to look) is the only input; never raw log lines (design decision, DESIGN.md §19.2 — log lines are untrusted data). The fixer formats the five-part spec (dfx orchestration convention: context, task, constraints, verification, done-definition — mirroring what dfx:grok-implementer already expects) from: the brief text, the task's repo/worktree, and the error's fingerprint/service/class (available on the linked error.seen row). Write it to <worktree>/.agent/spec.md, pass via --spec-file to cmd_run/launch.

3. Mechanical verdict rule (reviewer / pr-reviewer lane output → pass/fail)

cmd_run today only interprets output for role=="implementer". Reviewer and PR-reviewer-quality/logic lanes get a raw LaneResult (lane.py: status ∈ complete/partial/timeout/unavailable via the STATUS: regex, plus raw stdout) but nothing turns that into agent agent finish --verdict / agent gate record --verdict.

Hardcoded rule (as specified): STATUS: complete and 0 findings → pass (approved/done). Any findings listed → fail (rejected), new round. Anything else (empty/partial/timeout/unavailable/unparseable FINDINGS: section) → retry the same lane once; if still unparseable on retry, stop — set the task failed (chain.py's documented "orchestrator deliberately sets it" escape hatch) with the raw stdout excerpt as evidence, no further rounds.

Findings-counting needs a concrete parser (count non-empty items under a FINDINGS: header up to the next ALL-CAPS header or EOF; empty/"none" section = 0). I don't have a locked-down format for this yet — will pull real transcripts from currently-running gate evidence in the ledger (agent status shows 9 agents currently working) to calibrate before hardcoding the regex, rather than guessing blind.

Note re: issue #33 (verdict/gate binding gap) — related but not a blocker for this build. My driver will derive agent agent finish and agent gate record verdicts from one single parsed result per lane call (never two independently-computed values), as a self-imposed guard against exactly the class of bug #33 describes. Not fixing #33 itself here.

4. Outer driver loop + round cap

New module, e.g. fixer_act.py: for each open (non-terminal, non-gate-blocked) implement task on this device with payload.error_id set — author spec_written if pending, then repeatedly execute one step (refactored cmd_run core) until the task is done/failed/gate-blocked or idle. chain.py/the ledger itself intentionally has no round cap yet ("keine Obergrenze in dieser Version" per dfx:agent-ledger) — that stays untouched. The fixer's own driver adds an outer circuit breaker instead, since an unattended loop with no cap is a real cost/runaway risk: max 5 total rounds per task (counts every return to implementing, whether from a rejected inner review or a rejected PR gate) before forcing failed with a note. gate-blocked (vendor CLI missing/unauthenticated) is left as-is — existing convention already stops new rounds there and expects a human to resolve and reset it.

New CLI surface: agent watch error-fix-work (naming open to bikeshedding), same one-shot-scan shape as error-decide/error-fix, pollable by agent daemon/cron. Not wired into agent daemon in this PR — matching projects-19's own precedent for error-decide ("Not wired into agent daemon in this revision"); daemon wiring is a separate deliberate decision, possibly adjacent to #45.

5. PR title/body templating

Generate EN/DE body (≤4 sentences each, full detail in <details>, per git-konventionen) from the brief + fingerprint + repo, call agent activity add pr.open --title ... --body ... (ungated, see above) then agent github pending. Small, mechanical — no open design question.

Explicitly assumed / out of scope

  • Single device. Fixer runs co-located with the decide-loop (same AGENT_HOME/device_id), per DESIGN §21.1 ("this device" execution plane) and the device-scoped ownership model projects-19's branch just introduced. Cross-device dispatch is a different, bigger design (would touch the refused hub control plane, DESIGN §20) — not attempted here.
  • No claim/race mechanism. One fixer only, as specified.
  • No auto-continue / keep-working wiring. Out of scope, same as projects-19's decide dispatcher and the still-open design question in the findings doc's item 3.
  • checklist set vs close-step guard parity: while reading the enforcement path I noticed cmd_checklist/checklist set (main.py:726) does not call close_allowed at all, despite the dfx:agent-ledger skill doc's claim that it "runs the same guards" as close-step. Flagging as a possible pre-existing discrepancy — not touching it, not in scope here, but worth someone's eyes.

Open questions for review

  1. Naming: agent watch error-fix-work vs. something else, for the new driving loop.
  2. Round cap of 5 — arbitrary, open to adjustment.
  3. FINDINGS: parsing format — will calibrate against real lane output before hardcoding, may post an update here once confirmed.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions