Skip to content

🐛 fix(kanban): recognize a no-PR edit-in-place clean exit as provably-done - #75

Merged
cwest merged 1 commit into
cwest/integrationfrom
topic/kanban-no-pr-clean-exit-provably-done
Jul 19, 2026
Merged

🐛 fix(kanban): recognize a no-PR edit-in-place clean exit as provably-done#75
cwest merged 1 commit into
cwest/integrationfrom
topic/kanban-no-pr-clean-exit-provably-done

Conversation

@cwest

@cwest cwest commented Jul 19, 2026

Copy link
Copy Markdown
Owner

Why

An edit-in-place / no-PR card (workspace scratch/dir, or a ~/.hermes
workdir) completes its lane by exiting cleanly (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. detect_crashed_workers (hermes_cli/kanban_db.py)
then misread that verb-less rc=0 exit as a protocol_violation, tripped
failure_limit=1, and emitted a false gave_up — stranding a fully-completed
card in blocked and forcing a hand-reconcile.

Root cause

_lane_work_provably_done — the same helper both detect_crashed_workers'
clean_exit_after_done carve-out and check_respawn_guard trust — accepted
exactly two proofs:

  1. a task_runs row with outcome='completed' in the success window, and
  2. a GitHub PR URL in a recent comment.

A no-PR edit-in-place clean exit satisfies neither: it opens no PR (fails
proof 2), and a verb-less clean exit is precisely why no outcome='completed'
run row was written (fails proof 1). So the carve-out never fired for exactly
the card shape that legitimately exits verb-less → guaranteed false
protocol_violation.

What

Add a third proof to _lane_work_provably_done, 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 _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) 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, so the carve-out
    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 is
    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.

Done when

  • A no-PR edit-in-place card that finishes its lane and exits verb-less rc=0 is
    released as a benign clean_exit_after_done no-op (surfaced via the existing
    _last_clean_exit_after_done side-channel), not counted as a failure and not
    emitting gave_up/crashed. ✅
  • A no-PR card that exits verb-less rc=0 with NO landed-work proof still trips
    the protocol-violation breaker (proof-gated regression guard). ✅
  • The existing two proofs (completed-run, PR-URL) are unchanged; PR-backed cards
    behave exactly as before. ✅

Tests

3 behavior-contract tests added to tests/hermes_cli/test_kanban_db.py (real
kanban_db, temp HERMES_HOME, no mocks):

  • no-PR edit-in-place done + handoff comment → 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 (carve-out does not leak into PR-backed cards).

Full tests/hermes_cli/test_kanban_db.py (274) green, ruff clean, 0
regressions. Carries the PATCHES.md manifest row (fork supply-chain).

Base: cwest/integration.

…-done

An edit-in-place / no-PR card (workspace 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. detect_crashed_workers then misread that
verb-less rc=0 exit as a protocol_violation, tripped failure_limit=1,
and emitted a false gave_up, stranding a fully-completed card in blocked
for a human to hand-reconcile.

Root cause: _lane_work_provably_done — the helper both the
clean_exit_after_done carve-out and check_respawn_guard trust — accepted
exactly two proofs (a completed-run row in the success window, and a PR
URL in a recent comment). A no-PR edit-in-place clean exit satisfies
neither: it opens no PR (fails proof 2), and the verb-less exit is
precisely why no outcome='completed' run row was written (fails proof 1).
So the carve-out never fired for exactly the card shape that legitimately
exits verb-less.

Add a third proof, proof-gated and scoped to the no-PR shape via the
shared _card_requires_pr predicate (not a new ad-hoc definition): for a
card _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) 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, so this 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
unchanged). No new config, no new schema, no user-facing env var.

Adds 3 behavior-contract tests (real path, temp HERMES_HOME): 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.
@cwest
cwest marked this pull request as ready for review July 19, 2026 22:31
@cwest

cwest commented Jul 19, 2026

Copy link
Copy Markdown
Owner Author

The third proof lands exactly where the root-cause analysis says it should. _lane_work_provably_done now recognizes a no-PR edit-in-place card by the shared _card_requires_pr predicate rather than a new ad-hoc check, so the crash detector and the respawn guard keep reading the same definition of a landed lane. The proof is gated on the not-PR-requiring shape, so it can't fire for a worktree card — a PR-backed card still falls through to the strict path unchanged, which the third test pins down.

The handoff regex is anchored at line start with MULTILINE, so a progress note like "looking into this" won't be mistaken for a completion signpost. That keeps the carve-out proof-gated, and the without-proof test confirms a genuinely quiet exit still trips the protocol violation.

Verified against head f7295bbe: full test_kanban_db.py is 274 passed, ruff clean, and CI is green across all eight test slices, the secret scan, and the attribution check. I disabled the new branch in a throwaway copy and the done-path test failed as expected, so it has teeth. The diff is the three files it should be and nothing else.

No changes needed. Ready to merge.

@cwest
cwest merged commit 4a17268 into cwest/integration Jul 19, 2026
31 checks passed
@cwest
cwest deleted the topic/kanban-no-pr-clean-exit-provably-done branch July 19, 2026 22:33
cwest added a commit that referenced this pull request Jul 26, 2026
…-done (#75)

An edit-in-place / no-PR card (workspace 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. detect_crashed_workers then misread that
verb-less rc=0 exit as a protocol_violation, tripped failure_limit=1,
and emitted a false gave_up, stranding a fully-completed card in blocked
for a human to hand-reconcile.

Root cause: _lane_work_provably_done — the helper both the
clean_exit_after_done carve-out and check_respawn_guard trust — accepted
exactly two proofs (a completed-run row in the success window, and a PR
URL in a recent comment). A no-PR edit-in-place clean exit satisfies
neither: it opens no PR (fails proof 2), and the verb-less exit is
precisely why no outcome='completed' run row was written (fails proof 1).
So the carve-out never fired for exactly the card shape that legitimately
exits verb-less.

Add a third proof, proof-gated and scoped to the no-PR shape via the
shared _card_requires_pr predicate (not a new ad-hoc definition): for a
card _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) 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, so this 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
unchanged). No new config, no new schema, no user-facing env var.

Adds 3 behavior-contract tests (real path, temp HERMES_HOME): 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.

(cherry picked from commit 4a17268)
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.

1 participant