Skip to content

2f09ad4a - Loosen error-fix ownership to device-level, add a one-shot decide dispatcher - #55

Open
Danswar wants to merge 18 commits into
developfrom
2f09ad4a-906b-4efe-b4d3-ea5d1ddd0b10-error-decide-dispatcher-v2
Open

2f09ad4a - Loosen error-fix ownership to device-level, add a one-shot decide dispatcher#55
Danswar wants to merge 18 commits into
developfrom
2f09ad4a-906b-4efe-b4d3-ea5d1ddd0b10-error-decide-dispatcher-v2

Conversation

@Danswar

@Danswar Danswar commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

EN:
Loosens error-fix conclusion/implement-task ownership from session-exact to device-level, adds agent watch error-decide (a one-at-a-time dispatcher that starts a fresh, one-shot session per unresolved error, instead of relying on a single persistent session), and requires a brief field on error.fix conclusions.

DE:
Lockert die Besitz-Prüfung für error-fix-Konklusionen/Implement-Tasks von session-exakt auf Device-Ebene, fügt agent watch error-decide hinzu (ein Ein-nach-dem-anderen-Dispatcher, der pro offenem Fehler eine frische Einmal-Session startet, statt auf eine dauerhafte Session zu setzen), und macht ein brief-Feld auf error.fix-Konklusionen verpflichtend.

Details

Why

The existing error-fix skill's single-persistent-session model has no mechanism to auto-continue between queued items — each session does one thing, then sits idle until manually re-knocked. Rather than build that "keep it going" nudge, this drops the persistent session entirely: a fresh, uniquely-named session is started per error, does its one job (investigate, conclude), and stops.

What changed

  • src/agent_cli/error_fix_act.py, src/agent_cli/errors.py: _error_seen, _lookup_implement_task, incident_closed, validate_conclusion all dropped their session_id parameter/filter, keeping only device-level (_origin_device_id) ownership checks. Necessary because a fresh one-shot session writes the conclusion under its own session id, different from the error.seen row's session id (the fixed scanning session named in error-fix.json).
  • src/agent_cli/error_decide_act.py (new) + main.py: agent watch error-decide drains unconcluded error.seen rows oldest-first (by payload.first_seen), one at a time, inside a lock held for the whole scan so overlapping invocations can't double-dispatch the same row. Per row: ensures a runner session (skills error-fix, spine, review-loop, pr-review; reused and reopened on retry rather than re-created), waits for the pane to read as idle on two consecutive checks before knocking directly (not via knock.deliver, which would target the wrong session), retries the Enter keypress with an is_busy() confirmation, polls for the conclusion with a timeout, then always stops the pane. A row's own setup/knock/stop failures (StoreError/SystemExit/OSError) are recorded as that row's outcome without aborting the rest of the backlog, and a failure while stopping a pane can never overwrite a real decided/timeout result. Not wired into agent daemon in this revision.
  • validate_conclusion requires a brief field on error.fix (short text: what's broken, likely cause, where to look), matching the existing reason requirement on error.skip; the same check now also runs on the executor's read path (_pending_fix), not just at write time.
  • DESIGN.md §21 and the error-fix skill's SKILL.md updated to match throughout.

Verification

Full local suite: 714 passed, 1 pre-existing unrelated skip (missing optional agent_core package). Ran the actual dispatcher twice against a real, isolated Postgres store and a real grok session (not the repo's own store) — confirmed a real error.seen row gets picked up, investigated, and concluded with a real error.skip, the knock lands without manual intervention, and the session is cleanly stopped afterward.

…rting the whole scan, and refresh the skill and design docs to match.
@Danswar
Danswar force-pushed the 2f09ad4a-906b-4efe-b4d3-ea5d1ddd0b10-error-decide-dispatcher-v2 branch from 1fb847b to badabfa Compare September 2, 2026 03:47
…runcating error ids in decide session names.
…emoving id truncation, and lock the format down with a test.
… failure from hiding a real conclusion, and let a closed decide session be reopened.
… idle wait does not stall on a real 60-second timeout.
…rief field on the error-fix executor path too.
…d give the decide-dispatcher test fixture a brief field too.
@Danswar

Danswar commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator Author

EN:
Ready after 17 review passes.
Loosen error-fix conclusion ownership from session-exact to device-level and add a one-shot decide dispatcher so errors no longer need a persistent session.

DE:
Bereit nach 17 Review-Durchläufen.
Lockert die Besitzprüfung für error-fix-Konklusionen von session-exakt auf Device-Ebene und fügt einen Einmal-Session-Dispatcher hinzu, sodass Fehler keine dauerhafte Session mehr benötigen.

Details

Grok-PR (quality + logic, 14 passes to 0/0)

Findings fixed across the run: fresh-session skill gaps making implement tasks unreachable via agent task; missing try/finally leaking a tmux pane on a mid-sequence failure; missing CLI-level test coverage for the new watch command; skill list not unioned on session reuse; stale doc wording after the ownership loosening; missing entries in the agent watch subcommand inventories; missing kind==runner guard on session reuse; a lock scoped too narrowly, allowing two overlapping scans to double-dispatch the same row; a knock-acceptance check vulnerable to a false positive on a still-booting or resumed pane; a stop() failure able to hide a real successful conclusion; a manually-closed session with no reopen path; untruncated session ids to remove a naming-collision risk; a single failing row aborting the whole backlog scan instead of being isolated; two separate except clauses missing OSError; several doc/test consistency gaps.

Codex-PR (quality + logic, cross-vendor gate, 4 rounds to 0/0)

Findings fixed: start() called outside the try/finally around a decide session, risking an orphaned pane on a partial failure; a knock retry loop that silently treated exhaustion as success instead of failing that row; the two issues above from the earlier Grok-PR pass, independently re-derived; a brief field required only on the write path, not the read/execution path used by the executor.

Every fix was pushed to the same PR and re-run through both vendors at the new head until each reported zero findings; no finding was accepted or dismissed without independent verification against the actual code.

@Danswar
Danswar marked this pull request as ready for review September 2, 2026 07:07
@TaprootFreakAI

Copy link
Copy Markdown
Collaborator

EN:
Recommendation: move this forward before #56, subject to confirming the required review evidence on the current head. GitHub currently reports no conflicts and pytest is green. The per-error analysis session is a useful foundation for the later fixer, and the device-scoped ownership model is explicit in the updated design.

Before merging, check compatibility with existing error.fix producers and pending records: brief becomes mandatory on both the write and execution paths, so older pending conclusions without it will be rejected. Define how those records and producers are updated. Also keep the rollout scope clear: this PR adds the one-shot command; it does not wire it into the daemon.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants