Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions PATCHES.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,3 +86,4 @@ default; do **not** retire such a row on a PR-merge signal. See the #44338 row.
| fork PR (TBD β€” no upstream PR; fork-internal dispatcher semantics) | Fix the two SIBLING crash-classification paths #47 missed, so the kanban dispatcher stops false-flagging cleanly-finished and transiently-failed workers as `crashed`/`gave_up`. #47 (`426a8a9c7`) carved out the provably-done case for a reap-registry `clean_exit` (rc=0 captured), but two same-class paths in `detect_crashed_workers` (`hermes_cli/kanban_db.py`) still counted a false failure: **(A) transient endpoint-unreachable failures** β€” a run that died on `APIConnectionError` / connection-refused / provider 5xx (classified `timeout`/`overloaded`/`server_error`/`upstream_rate_limit` after in-process retries) exited plain `1`, which the reaper read as a real crash toward `failure_limit`; a proxy blip thus `gave_up` a card that would otherwise succeed. Fix: new `KANBAN_TRANSIENT_EXIT_CODE` (69, EX_UNAVAILABLE) sentinel + a single-source-of-truth `kanban_worker_exit_code(failure_reason)` helper the CLI kanban-worker exit path delegates to (replacing the inline rate-limit-only mapping); `_classify_worker_exit` maps 69 β†’ a new `transient` kind; `detect_crashed_workers` treats it exactly like the rate-limit carve-out (requeue to `ready`/`review`, NO failure counted, distinct `transient` run outcome + event, stamped `last_failure_error` so the respawn-guard cooldown spaces the retry), surfaced via `DispatchResult.transient` + the `_last_transient` side-channel; `check_respawn_guard`'s cooldown gate now recognizes both `rate_limited` and `transient` outcomes. **(B) clean exit misread as a crash on the `pid not alive`/`unknown` reap path** β€” #47's provably-done carve-out was gated on `kind == "clean_exit"` only, so a worker that finished its lane cleanly (draft-PR handoff / edit-in-place card) but whose exit was NOT captured in the reap registry (reaped by init, or gone between the reap tick and the liveness check β†’ `_classify_worker_exit` returns `unknown`) fell into the generic `crashed` branch and counted a failure. Fix: extend the `_lane_work_provably_done` carve-out from `kind == "clean_exit"` to `kind in ("clean_exit", "unknown")`, using the SAME durable proof signals (recent_success / active_pr) #47 trusts. Non-regression preserved: a genuine mid-work death with NO proof still `crashed`β†’`gave_up` after the limit, and a real non-zero crash (not the sentinel) still counts. Two-file change (`hermes_cli/kanban_db.py` + `cli.py`) + 8 behavior-contract tests in `tests/hermes_cli/test_kanban_db.py` (transient-sentinel classify; transient requeue-without-failure across 6 hits; transient cooldown defer/allow; unknown-exit-after-completed-run and unknown-exit-after-draft-PR not counted; unknown-exit-without-proof still crashes; real-crash non-regression; exit-code mapping). **Retire trigger:** open the upstream PR, then auto-retire when it merges in a tagged release β‰₯ base; until then carry as upstream-pending. | upstream-pending | main@9be292f1e |
| fork PR (TBD β€” no upstream PR; fork-internal review-lane semantics) | Make `unblock_task` also transition a card out of `triage`, so the OUTER feedback loop (Casey-feedback -> author on a card carrying an open PR) self-heals the `active_pr` respawn-guard wedge that only the INNER review-bounce loop was covered for (composes with rows 84/85 -- the block-loop breaker + reset-recovery -- extending them to the non-review author-bounce case). Live symptom 2026-07-11: a writing card (`t_f6126dcd`, author orwell) carrying an OPEN PR needed to bounce back to its author for a Casey-feedback revision; the hand-move `blocked->todo`+author did not clear the `active_pr` guard (a raw status flip emits no `unblocked` cutoff event), so `hermes kanban dispatch` refused to spawn the author every tick (`respawn_guarded {active_pr}`). Repeated churn inflated `block_recurrences` past `BLOCK_RECURRENCE_LIMIT`, escalating the card to `triage`, where the standard block->unblock cutoff SILENTLY no-oped -- `unblock_task` (`hermes_cli/kanban_db.py`) matched only `status IN ('blocked','scheduled')`, so a triage card matched zero rows, returned False, emitted no `unblocked` event, and the guard stayed tripped forever (only a hand `reset_block_recurrences` + status=blocked + `unblock_task` rescued it). Root cause: the INNER review loop self-heals via `auto_route_review_bounce` (reassigns + `unblock_task`, emitting the `unblocked` cutoff that `check_respawn_guard` honors), but the outer feedback loop has no `review-changes-requested` reason so it is never routed, and the only skill-layer primitive (`onecard_common.move_card`) is a raw status flip that emits no cutoff. Fix (minimal, well-scoped): add `'triage'` to both `WHERE status IN (...)` clauses in `unblock_task` -- the stale-run-pointer SELECT and the status UPDATE -- so a card escalated to `triage` transitions back to `ready`/`todo` (parent-gate re-checked) and emits the `unblocked` cutoff, clearing `active_pr` the same way a normal block->unblock does. Preserves the loop breaker: `block_recurrences` is deliberately NOT reset (counter still survives the unblock; reset only on completion or the explicit `reset_block_recurrences` API), so a genuine same-finding loop still escalates. This supersedes row 85's parenthetical about a card already escalated to `triage` where `unblock_task` did not apply -- with this patch `unblock --reset-loop` fully recovers a triaged card (reset + lane flip). Single-file change in `hermes_cli/kanban_db.py` (+ stale-comment sync in `hermes_cli/kanban.py`) + 5 behavior-contract regression tests in `tests/hermes_cli/test_kanban_reset_block_loop.py` (RED-then-GREEN reproducing the exact triage-with-open-PR wedge; parent-gate re-check; counter preserved; the pre-fix wedge documented). Clean upstream candidate -- a sanctioned unblock that no-ops from triage is a real bug -- but tracked fork-internal for now. **Retire trigger:** open the upstream PR, then auto-retire when it merges in a tagged release >= base; until then carry as permanent-local. | permanent-local | main@9be292f1e |
| fork PR (TBD — no upstream PR; fork-internal completion-lane semantics) | Add a **required-artifact completion guard** to `complete_task` (`hermes_cli/kanban_db.py`) — the third false-`done` guard, beside the phantom-`created_cards` and acceptance-lane guards, in the same before-write-txn shape. Root cause (live 2026-07-08, card `t_e3e5bc16`): a worker can build its deliverable in a git worktree but exit WITHOUT committing / pushing / opening a PR, and its `kanban_complete` still flips the card to `done`; that card wrote three research files into its worktree, opened no PR, completed anyway, and its gated children auto-promoted onto a foundation that did not exist (one began running before it was caught). `done` MUST mean "the declared reviewable artifact exists"; a `done` card with no PR is a false state, and no guard covered "the card's own contract required a PR and none exists." Fix (opt-in per card, derived from the workspace — zero schema footprint, no `requires_pr` column, no migration, existing cards classified correctly with no backfill): a new `_card_requires_pr(workspace_kind, workspace_path)` returns True only for `workspace_kind == 'worktree'` (the sole kind that materializes an isolated linked git worktree on a `topic/*` branch via `_ensure_git_worktree` — precisely the implementer→PR shape; `branch_name` is worktree-only) whose `workspace_path` is NOT anchored under any `~/.hermes` tree (`get_default_hermes_root()` and `$HOME/.hermes` both excluded, so edit-in-place config / live-install worktrees stay completable); `scratch` (throwaway tmp dir) and `dir` (plain shared directory — persistent build dirs, `cwest/hermes-config` edits) are NOT PR-requiring and complete exactly as before. The artifact check `_card_has_pr_artifact(conn, id)` reuses the existing PR→card linkage — a resolvable `pull/<n>` URL in any task comment (the implementer's ready-for-review handoff), matched by the SAME `_RESPAWN_GUARD_PR_URL_RE` the `active_pr` respawn guard and `_lane_work_provably_done` already trust (extend, not duplicate). For a guarded card with no PR, `complete_task` is a clean no-op refusal (returns `False`, no task-state mutation) that emits an auditable `completion_refused_missing_pr` event with a `summary_preview`, mirroring `completion_refused_acceptance`. Casey's merge path (`allow_acceptance_complete=True`, the `github-pr-closed` webhook) BYPASSES the guard exactly as it bypasses the acceptance guard. The guard lives inside `complete_task` — the single chokepoint every completion path calls (worker `kanban_complete`, `hermes kanban complete`, the swarm root helper, the dashboard) — so the whole class is covered by construction. Tests: `tests/hermes_cli/test_kanban_complete_missing_pr_guard.py` (7 behavior-contract cases against a real temp kanban DB — worktree+no-PR refused, refusal emits the audit event + no `completed` event, `dir`/`scratch` cards complete with no PR, worktree+PR-comment completes, `~/.hermes` worktree completes, merge-override completes without a PR). Full `test_kanban_db.py` (271) + the kanban surface green; no regressions. **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR — fork-internal completion-lane invariant); remove only if the fork's one-card completion discipline is retired. | permanent-local | main@9be292f1e |
| fork PR (TBD β€” no upstream PR; fork-internal dispatcher semantics) | Recognize a **no-PR edit-in-place clean exit as provably-done** so the kanban crash-detector stops false-`gave_up`ing a finished edit-in-place card (the last defect from the code-enforcement batch; live 2026-07-19 on cards `t_3f07b138` and `t_ae057580`, both false-`gave_up`'d with deliverables complete and green on disk). Root cause: `_lane_work_provably_done` (`hermes_cli/kanban_db.py`) β€” the SAME helper both `detect_crashed_workers`' `clean_exit_after_done` carve-out and `check_respawn_guard` trust β€” accepted exactly TWO proofs (a `task_runs` row with `outcome='completed'` in the success window, and a GitHub PR URL in a recent comment). An edit-in-place / no-PR card (`workspace_kind='scratch'`/`'dir'`, or a `~/.hermes` workdir) completes its lane by exiting rc=0 WITHOUT a terminal kanban verb β€” by design, since `done ≑ merged` only applies to PR-backed cards and there is no PR to open β€” so it satisfies NEITHER proof (opens no PR β†’ fails proof 2; the verb-less exit is exactly why no `outcome='completed'` run row is written β†’ fails proof 1). The carve-out therefore never fired for exactly the card shape that legitimately exits verb-less β†’ guaranteed false `protocol_violation`/`gave_up`, stranding a completed card in `blocked` for a human to hand-reconcile. Fix: add a THIRD proof, proof-gated and scoped to the no-PR shape via the SHARED `_card_requires_pr(workspace_kind, workspace_path)` predicate (the same predicate the required-artifact completion guard uses β€” not a new ad-hoc definition): for a card that `_card_requires_pr` classifies as NOT PR-requiring, a durable self-verification / lane-done HANDOFF comment within the reused `_RESPAWN_GUARD_SUCCESS_WINDOW` (matched by a conservative, line-anchored `_LANE_DONE_HANDOFF_RE` β€” `review-required:` / `self-verified` / `lane-done` / `handoff` / `verified green|on disk` / `completed lane`) reads as landed-work proof. Absence of proof KEEPS the strict protocol-violation behavior (a genuinely-incomplete quiet exit on a no-PR card STILL counts β€” the carve-out is proof-gated and cannot mask real breakage), the existing two proofs are unchanged, and a PR-requiring worktree card is still held to a real PR/completed-run artifact (PR-backed behavior byte-for-byte unchanged β€” the handoff-comment match never leaks into it). No new config, no new schema, no user-facing env var β€” reuses the existing window constant and the shared shape predicate. Single-file change in `hermes_cli/kanban_db.py` + 3 behavior-contract regression tests in `tests/hermes_cli/test_kanban_db.py` (no-PR edit-in-place done+handoff β†’ benign no-op surfaced via the `_last_clean_exit_after_done` side-channel; no-PR edit-in-place with no landed-work proof β†’ still `protocol_violation`/`gave_up`; PR-requiring card with a handoff comment but no PR β†’ still `protocol_violation`). Real `kanban_db`, temp `HERMES_HOME`, no mocks; full `test_kanban_db.py` (274) + the kanban surface green, ruff clean, 0 regressions. **Retire trigger:** never auto-retires on an upstream PR-merge signal (no upstream PR β€” fork-internal dispatcher/lifecycle invariant); remove only if the fork's no-PR edit-in-place card model is retired. | permanent-local | main@9be292f1e |
55 changes: 54 additions & 1 deletion hermes_cli/kanban_db.py
Original file line number Diff line number Diff line change
Expand Up @@ -7332,6 +7332,23 @@ def schedule_task(
re.IGNORECASE,
)

# Pattern matching a durable self-verification / lane-done HANDOFF comment on a
# no-PR edit-in-place card. Such a card has no PR to open and (because a
# verb-less clean exit is exactly why no ``outcome='completed'`` run row was
# written) satisfies neither of the two PR/completed-run proofs β€” so a landed
# lane is proven ONLY by a deliberate handoff comment the worker posts before
# exiting. Matched conservatively: only an explicit handoff / self-verified /
# completion signpost counts, so a chatty progress comment ("looking into it")
# is NOT mistaken for proof and the carve-out stays proof-gated (an incomplete
# quiet exit still trips the protocol violation). Anchored to the start of any
# line so the token is a deliberate signpost, not an incidental mention.
_LANE_DONE_HANDOFF_RE = re.compile(
r"^\s*(?:review-required\b|self-verified\b|lane[ -]?done\b|"
r"lane work (?:provably )?done\b|handoff\b|verified(?: green| on disk)\b|"
r"completed(?: the)? lane\b)",
re.IGNORECASE | re.MULTILINE,
)


def _resolve_pr_state(pr_url: str) -> str:
"""Return the live GitHub state of ``pr_url`` for the active_pr respawn guard.
Expand Down Expand Up @@ -8725,7 +8742,20 @@ def _lane_work_provably_done(conn: sqlite3.Connection, task_id: str) -> bool:

* a completed run within ``_RESPAWN_GUARD_SUCCESS_WINDOW`` (recent_success), or
* a GitHub PR URL in a comment within ``_RESPAWN_GUARD_PR_WINDOW`` (active_pr) β€”
the ready-for-review handoff the implementer lane posts on PR open.
the ready-for-review handoff the implementer lane posts on PR open, or
* for a **no-PR edit-in-place card** (``scratch`` / ``dir`` / a ``~/.hermes``
workdir β€” the shape :func:`_card_requires_pr` classifies as NOT
PR-requiring): a durable self-verification / lane-done HANDOFF comment
within ``_RESPAWN_GUARD_SUCCESS_WINDOW`` (matched by
:data:`_LANE_DONE_HANDOFF_RE`). Such a card completes its lane by exiting
rc=0 with no terminal verb β€” by design, since ``done ≑ merged`` only
applies to PR-backed cards and there is no PR to open β€” so it satisfies
NEITHER of the first two proofs (it opens no PR, and the verb-less exit is
exactly why no ``outcome='completed'`` run row exists). Its landed lane is
provable only from a deliberate handoff comment. This proof is scoped to
the no-PR shape so it can never leak into a PR-requiring worktree card
(which is still held to a real PR/completed-run artifact β€” PR-backed
behavior is unchanged).

Only proof is a carve-out; absence of proof keeps the strict
protocol-violation behavior (a genuinely-incomplete quiet exit STILL
Expand All @@ -8751,6 +8781,29 @@ def _lane_work_provably_done(conn: sqlite3.Connection, task_id: str) -> bool:
if c["body"] and _RESPAWN_GUARD_PR_URL_RE.search(c["body"]):
return True

# Proof 3: a no-PR edit-in-place card with a durable lane-done HANDOFF
# comment in the success window. Gated on the shared no-PR shape predicate
# so it applies ONLY to cards that legitimately exit verb-less (no PR to
# open); a PR-requiring worktree card falls through to the strict
# protocol-violation path unchanged. The handoff-comment match is
# deliberately conservative (an explicit signpost, not any comment) so an
# incomplete quiet exit that never posted a landed-work handoff still counts.
trow = conn.execute(
"SELECT workspace_kind, workspace_path FROM tasks WHERE id = ?",
(task_id,),
).fetchone()
if trow is not None and not _card_requires_pr(
trow["workspace_kind"], trow["workspace_path"]
):
handoff_cutoff = now - _RESPAWN_GUARD_SUCCESS_WINDOW
for c in conn.execute(
"SELECT body FROM task_comments "
"WHERE task_id = ? AND created_at >= ?",
(task_id, handoff_cutoff),
).fetchall():
if c["body"] and _LANE_DONE_HANDOFF_RE.search(c["body"]):
return True

return False


Expand Down
Loading
Loading