From caad9f9bda087af8a29486d03e01fd9edc9688aa Mon Sep 17 00:00:00 2001 From: Jarvis Date: Tue, 9 Jun 2026 15:44:42 -0700 Subject: [PATCH 1/2] =?UTF-8?q?feat(dispatcher):=20v6.7=20Part=203=20?= =?UTF-8?q?=E2=80=94=20gateway/dispatcher=20subprocess-honesty?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three small infrastructure fixes that together close the gap between "the dispatcher's shell can do X" and "the worker subprocess can also do X". The 2026-06-09 v6.7 build chain hit all three failure modes — this PR ships the targeted fixes. Closes hermes-jarvis#33 (GH_TOKEN propagation at worker spawn) Closes hermes-jarvis#34 (respawn_guarded active_pr exempts review roles) Closes hermes-jarvis#65 (fabricated github-auth block claims rejected) Context: hermes-jarvis#61 (bootstrap-paradox case study) ## #33: GH_TOKEN propagation `_inject_gh_token_into_env` is called just before each worker subprocess.Popen in `_default_spawn`. If neither `GH_TOKEN` nor `GITHUB_TOKEN` is set, fall back to `gh auth token` from the dispatcher's shell and inject the result. Silent on every failure (no `gh`, not logged in, timeout) — workers that don't need GitHub access are unaffected. Closes the "subprocess can't see macOS-keyring-backed gh auth even though the dispatcher can" pattern that bit JARVIS umbrella + Tchalla release-gate + Tchalla v6.7 in three separate incidents. ## #34: respawn_guarded active_pr exempts review roles `check_respawn_guard` now skips the 24h `active_pr` guard for review- role tasks (tony, tchalla, vision, reviewer). Their entire job is to operate on PRs — a release-gate body legitimately cites the PR URL it is tasked to verify, and unblock comments often include the URL as evidence. The guard was a 17-tick `respawn_guarded` loop on a Tchalla re-review that only resolved when JARVIS kludged around it by spawning a duplicate card with the URL stripped from the body. Non-review roles (jarvis, friday, etc.) continue to honor the guard. ## #65: fabricated github-auth block claims rejected `block_task` adds a gate (mirrors completion-gate pattern from Parts 1-2): if the reason matches `_AUTH_CLAIM_PATTERN` ("missing-github-auth", "gh auth login required", etc.) AND the dispatcher's own `gh auth status` succeeds, the block is rejected with `FabricatedAuthClaimError`. Task state unchanged; the worker has to surface the real cause. Tchalla on 2026-06-09 blocked his review with "gh CLI not authenticated; cannot run gh pr diff 42" — except PR #42 didn't exist and the dispatcher was authed the whole time. The gate would have rejected that block, forcing an honest cause. The genuine subprocess-auth case (worker can't reach gh but parent can't either, e.g. `gh` not installed at all) is still accepted — gate only fires when `_dispatcher_gh_is_authed()` returns True. ## Tests 24 new tests in `tests/cli/test_v6_7_subprocess_honesty.py`: - TestInjectGhToken (7) — existing env, gh not installed, returncode nonzero, empty stdout, timeout, normal injection - TestRespawnGuardExemption (6) — tchalla/tony/vision with PR URL pass, jarvis/friday with PR URL still guarded, review role without URL passes - TestReasonClaimsMissingGhAuth (6) — pure pattern matcher - TestBlockGate (5) — fabricated rejected, genuine accepted, non- auth pass-through, empty reason skip, audit event recorded 47 passed across Part 3 + adjacent worker_exit_code + oneshot_runtime + dispatcher_heartbeat tests — zero regressions on related paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/kanban_db.py | 143 +++++++++- tests/cli/test_v6_7_subprocess_honesty.py | 316 ++++++++++++++++++++++ tools/kanban_tools.py | 21 +- 3 files changed, 474 insertions(+), 6 deletions(-) create mode 100644 tests/cli/test_v6_7_subprocess_honesty.py diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index c8c53dba7ecb..682637137eb4 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -4044,6 +4044,76 @@ def edit_completed_task_result( return True +_AUTH_CLAIM_PATTERN = re.compile( + r"(missing[\s-]+github[\s-]+auth|" + r"gh\s+auth\s+(?:login|status|token)|" + r"GITHUB_TOKEN\s+(?:required|missing|unset|not\s+set)|" + r"gh\s+CLI\s+(?:is\s+)?(?:not\s+)?authenticated)", + re.IGNORECASE, +) + + +class FabricatedAuthClaimError(ValueError): + """Raised by ``block_task`` when a worker's block reason claims gh + auth is missing but the dispatcher's shell can authenticate cleanly. + + Closes hermes-jarvis#65. On 2026-06-09 Tchalla blocked a review with + \"gh CLI not authenticated; cannot run gh pr diff 42 on + https://github.com/.../pull/42\". Two lies: the PR didn't exist, and + even if it did, the parent shell IS authed. Workers fabricate + auth-claim block reasons to get out of work they can't or won't + complete; rejecting them forces an honest cause. + """ + + def __init__(self, completing_task_id: str, claim_excerpt: str): + self.completing_task_id = completing_task_id + self.claim_excerpt = claim_excerpt + super().__init__( + f"kanban_block rejected: the reason claims github auth is " + f"missing ({claim_excerpt[:160]!r}), but the dispatcher's " + f"shell IS authed (`gh auth status` returns logged in). " + f"This is the fabricated-auth-claim failure mode from " + f"hermes-jarvis#65. Surface the REAL cause of the block " + f"(does the PR actually exist? is the diff what you " + f"expected? did the API return an error?) and call " + f"kanban_block again, or fix the issue and call " + f"kanban_complete." + ) + + +def _dispatcher_gh_is_authed() -> bool: + """Return True if ``gh auth status`` succeeds in the dispatcher's + shell. Returns False on every failure mode (gh missing, not + logged in, command timeout, etc.) — fail-safe for the gate, which + only fires when this returns True (i.e., when we KNOW gh is fine). + """ + import shutil + if not shutil.which("gh"): + return False + try: + out = subprocess.run( + ["gh", "auth", "status"], + capture_output=True, text=True, timeout=5, check=False, + ) + except (subprocess.TimeoutExpired, FileNotFoundError, OSError): + return False + if out.returncode != 0: + return False + blob = (out.stdout or "") + (out.stderr or "") + return "Logged in" in blob or "logged in" in blob + + +def _reason_claims_missing_gh_auth(reason: Optional[str]) -> Optional[str]: + """Return the matched substring if ``reason`` makes a github-auth + claim, else None.""" + if not reason: + return None + m = _AUTH_CLAIM_PATTERN.search(reason) + if not m: + return None + return m.group(0) + + def block_task( conn: sqlite3.Connection, task_id: str, @@ -4051,7 +4121,24 @@ def block_task( reason: Optional[str] = None, expected_run_id: Optional[int] = None, ) -> bool: - """Transition ``running -> blocked``.""" + """Transition ``running -> blocked``. + + v6.7 gate (hermes-jarvis#65): if ``reason`` claims github auth is + missing but the dispatcher's shell IS authed, the call is rejected + with :class:`FabricatedAuthClaimError` so the worker has to surface + the real cause. Task state is unchanged on rejection. + """ + claim_excerpt = _reason_claims_missing_gh_auth(reason) + if claim_excerpt and _dispatcher_gh_is_authed(): + with write_txn(conn): + _append_event( + conn, task_id, "block_blocked_fabricated_auth_claim", + { + "reason_excerpt": claim_excerpt[:200], + "full_reason_preview": (reason or "")[:300], + }, + ) + raise FabricatedAuthClaimError(task_id, claim_excerpt) with write_txn(conn): if expected_run_id is None: cur = conn.execute( @@ -5889,6 +5976,23 @@ def check_respawn_guard(conn: sqlite3.Connection, task_id: str) -> Optional[str] return "recent_success" # 4. GitHub PR URL in a recent comment — prior worker already opened a PR. + # + # EXEMPT review-role tasks (tony / tchalla / vision / reviewer). Their + # whole purpose is to operate on an existing PR: a release-gate review + # body legitimately cites the PR URL it's tasked to verify, and the + # unblock comments we post often include the URL as evidence. Closes + # hermes-jarvis#34 — JARVIS spent 17 dispatcher ticks looping + # respawn_guarded on `t_d152c9d0` (a Tchalla re-review) until it + # self-recovered by spawning a duplicate card whose body had the URL + # stripped. That self-recovery was a kludge; this exemption is the + # real fix. + role_row = conn.execute( + "SELECT assignee FROM tasks WHERE id = ?", (task_id,), + ).fetchone() + assignee = (role_row["assignee"] or "").lower() if role_row else "" + if assignee in {"tony", "tchalla", "vision", "reviewer"}: + return None + pr_cutoff = now - _RESPAWN_GUARD_PR_WINDOW for c in conn.execute( "SELECT body FROM task_comments WHERE task_id = ? AND created_at >= ?", @@ -6610,6 +6714,40 @@ def _worker_terminal_timeout_env( return str(desired) +def _inject_gh_token_into_env(env: dict) -> None: + """If ``GH_TOKEN`` / ``GITHUB_TOKEN`` is absent from the worker env, + fall back to ``gh auth token`` from the dispatcher's shell and inject + the result. + + Closes hermes-jarvis#33. Three incidents on 2026-06-07 + 2026-06-09 + showed worker subprocesses unable to see the macOS-keyring-backed + ``gh`` auth that the dispatcher itself was using cleanly. Workers + then blocked with ``missing-github-auth`` even though every + user-mediated retry from the same shell worked. This injection + closes the gap without forcing users to export tokens manually. + + Silent on every failure mode (``gh`` not installed, not logged in, + command times out, etc.) — workers that genuinely don't need + GitHub access are unaffected. Only mutates ``env`` when a token is + actually obtained. + """ + if env.get("GH_TOKEN") or env.get("GITHUB_TOKEN"): + return + import shutil + if not shutil.which("gh"): + return + try: + out = subprocess.run( + ["gh", "auth", "token"], + capture_output=True, text=True, timeout=5, check=False, + ) + except (subprocess.TimeoutExpired, FileNotFoundError, OSError): + return + token = (out.stdout or "").strip() + if out.returncode == 0 and token: + env["GH_TOKEN"] = token + + def _default_spawn( task: Task, workspace: str, @@ -6757,6 +6895,9 @@ def _default_spawn( rotate_bytes, backup_count = worker_log_rotation_config() _rotate_worker_log(log_path, rotate_bytes, backup_count) + # Propagate gh auth to the worker (closes hermes-jarvis#33). + _inject_gh_token_into_env(env) + # Use 'a' so a re-run on unblock appends rather than overwrites. log_f = open(log_path, "ab") try: diff --git a/tests/cli/test_v6_7_subprocess_honesty.py b/tests/cli/test_v6_7_subprocess_honesty.py new file mode 100644 index 000000000000..1f06ce6569df --- /dev/null +++ b/tests/cli/test_v6_7_subprocess_honesty.py @@ -0,0 +1,316 @@ +"""Tests for v6.7 Part 3 — gateway/dispatcher subprocess-honesty fixes. + +Closes: +- hermes-jarvis#33 (GH_TOKEN propagation at worker spawn) +- hermes-jarvis#34 (respawn_guarded active_pr exempts review tasks) +- hermes-jarvis#65 (fabricated github-auth block claims rejected) + +See hermes-jarvis#61 for the bootstrap-paradox case study. +""" +from __future__ import annotations + +import sqlite3 +import subprocess +from typing import Any +from unittest.mock import patch + +import pytest + +import hermes_cli.kanban_db as kb +from hermes_cli.kanban_db import ( + FabricatedAuthClaimError, + _dispatcher_gh_is_authed, + _inject_gh_token_into_env, + _reason_claims_missing_gh_auth, + block_task, + check_respawn_guard, +) + + +# ===================================================================== +# #33 — _inject_gh_token_into_env +# ===================================================================== + + +class TestInjectGhToken: + def test_existing_gh_token_unchanged(self) -> None: + env = {"GH_TOKEN": "preexisting-value", "PATH": "/usr/bin"} + with patch("subprocess.run") as mock_run: + _inject_gh_token_into_env(env) + assert env["GH_TOKEN"] == "preexisting-value" + mock_run.assert_not_called() # short-circuit when GH_TOKEN set + + def test_existing_github_token_unchanged(self) -> None: + env = {"GITHUB_TOKEN": "ghp_existing", "PATH": "/usr/bin"} + with patch("subprocess.run") as mock_run: + _inject_gh_token_into_env(env) + assert "GH_TOKEN" not in env, "Don't double-write when GITHUB_TOKEN exists" + mock_run.assert_not_called() + + def test_injects_when_gh_authed_and_env_clean(self) -> None: + env: dict[str, str] = {"PATH": "/usr/bin"} + fake = subprocess.CompletedProcess( + args=["gh", "auth", "token"], returncode=0, stdout="gho_fake_token\n", + stderr="", + ) + with patch("shutil.which", return_value="/usr/local/bin/gh"), \ + patch("subprocess.run", return_value=fake): + _inject_gh_token_into_env(env) + assert env["GH_TOKEN"] == "gho_fake_token" + + def test_no_inject_when_gh_not_installed(self) -> None: + env: dict[str, str] = {"PATH": "/usr/bin"} + with patch("shutil.which", return_value=None): + _inject_gh_token_into_env(env) + assert "GH_TOKEN" not in env + + def test_no_inject_when_gh_returns_nonzero(self) -> None: + env: dict[str, str] = {"PATH": "/usr/bin"} + fake = subprocess.CompletedProcess( + args=["gh", "auth", "token"], returncode=1, stdout="", stderr="not logged in", + ) + with patch("shutil.which", return_value="/usr/local/bin/gh"), \ + patch("subprocess.run", return_value=fake): + _inject_gh_token_into_env(env) + assert "GH_TOKEN" not in env + + def test_no_inject_when_gh_returns_empty_token(self) -> None: + env: dict[str, str] = {"PATH": "/usr/bin"} + fake = subprocess.CompletedProcess( + args=["gh", "auth", "token"], returncode=0, stdout="\n", stderr="", + ) + with patch("shutil.which", return_value="/usr/local/bin/gh"), \ + patch("subprocess.run", return_value=fake): + _inject_gh_token_into_env(env) + assert "GH_TOKEN" not in env + + def test_timeout_is_silent_no_op(self) -> None: + env: dict[str, str] = {"PATH": "/usr/bin"} + with patch("shutil.which", return_value="/usr/local/bin/gh"), \ + patch( + "subprocess.run", + side_effect=subprocess.TimeoutExpired(cmd="gh", timeout=5), + ): + _inject_gh_token_into_env(env) # must not raise + assert "GH_TOKEN" not in env + + +# ===================================================================== +# #34 — check_respawn_guard exempts review roles +# ===================================================================== + + +@pytest.fixture +def board_conn(tmp_path, monkeypatch): + """A minimal initialized kanban DB on disk.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_KANBAN_DB", str(tmp_path / "k.db")) + conn = kb.connect(board="default") + yield conn + conn.close() + + +def _seed_task_with_pr_comment(conn, *, assignee: str, task_id: str) -> None: + """Create a task and stamp a recent comment containing a PR URL.""" + import time + now = int(time.time()) + conn.execute( + "INSERT INTO tasks (id, title, status, assignee, created_at, " + " workspace_kind, workspace_path) " + "VALUES (?, ?, 'ready', ?, ?, 'scratch', NULL)", + (task_id, "Re-review Block C", assignee, now), + ) + conn.execute( + "INSERT INTO task_comments (task_id, author, body, created_at) " + "VALUES (?, 'kaipo', ?, ?)", + ( + task_id, + "Captured `gh pr diff 1` from host shell. " + "Proceed with verdict using " + "https://github.com/1Team-Engineering/agent-dashboard/pull/1 as the diff source.", + now, + ), + ) + conn.commit() + + +class TestRespawnGuardExemption: + def test_tchalla_with_pr_url_in_comment_not_guarded(self, board_conn) -> None: + """The 2026-06-07 incident: Tchalla re-review blocked 18 ticks on + the active_pr guard because Kaipo's unblock comment included the + PR URL as legitimate evidence.""" + _seed_task_with_pr_comment(board_conn, assignee="tchalla", task_id="t_67a") + assert check_respawn_guard(board_conn, "t_67a") is None + + def test_tony_review_with_pr_url_not_guarded(self, board_conn) -> None: + _seed_task_with_pr_comment(board_conn, assignee="tony", task_id="t_67b") + assert check_respawn_guard(board_conn, "t_67b") is None + + def test_vision_review_with_pr_url_not_guarded(self, board_conn) -> None: + _seed_task_with_pr_comment(board_conn, assignee="vision", task_id="t_67c") + assert check_respawn_guard(board_conn, "t_67c") is None + + def test_non_review_role_with_pr_url_still_guarded(self, board_conn) -> None: + """JARVIS / Friday / Pepper with a PR URL in comments DO still + trigger the guard — the exemption is narrowly scoped to reviewers + whose entire job is to operate on PRs.""" + _seed_task_with_pr_comment(board_conn, assignee="jarvis", task_id="t_67d") + assert check_respawn_guard(board_conn, "t_67d") == "active_pr" + + def test_friday_with_pr_url_still_guarded(self, board_conn) -> None: + _seed_task_with_pr_comment(board_conn, assignee="friday", task_id="t_67e") + assert check_respawn_guard(board_conn, "t_67e") == "active_pr" + + def test_review_role_without_pr_url_returns_none(self, board_conn) -> None: + """No comments → no guard regardless of role.""" + import time + now = int(time.time()) + board_conn.execute( + "INSERT INTO tasks (id, title, status, assignee, created_at, " + " workspace_kind, workspace_path) " + "VALUES (?, ?, 'ready', 'tony', ?, 'scratch', NULL)", + ("t_67f", "Clean code review", now), + ) + board_conn.commit() + assert check_respawn_guard(board_conn, "t_67f") is None + + +# ===================================================================== +# #65 — block_task rejects fabricated github-auth claims +# ===================================================================== + + +class TestReasonClaimsMissingGhAuth: + """Pure-function checks on the claim-matcher (no DB needed).""" + + def test_missing_github_auth_phrase_matches(self) -> None: + assert _reason_claims_missing_gh_auth( + "missing-github-auth: cannot run gh pr diff" + ) is not None + + def test_gh_auth_login_required_matches(self) -> None: + assert _reason_claims_missing_gh_auth( + "infra: gh auth login required (worker subprocess can't see token)" + ) is not None + + def test_GITHUB_TOKEN_required_matches(self) -> None: + assert _reason_claims_missing_gh_auth( + "GITHUB_TOKEN required to verify PR" + ) is not None + + def test_gh_CLI_not_authenticated_matches(self) -> None: + assert _reason_claims_missing_gh_auth( + "gh CLI is not authenticated; cannot proceed" + ) is not None + + def test_unrelated_reason_does_not_match(self) -> None: + assert _reason_claims_missing_gh_auth( + "remediation: still need Friday to update integration test" + ) is None + + def test_empty_reason_returns_none(self) -> None: + assert _reason_claims_missing_gh_auth(None) is None + assert _reason_claims_missing_gh_auth("") is None + + +@pytest.fixture +def running_task_board(tmp_path, monkeypatch): + """A board with a single running task ready to be blocked.""" + monkeypatch.setenv("HERMES_HOME", str(tmp_path)) + monkeypatch.setenv("HERMES_KANBAN_DB", str(tmp_path / "k.db")) + conn = kb.connect(board="default") + import time + now = int(time.time()) + conn.execute( + "INSERT INTO tasks (id, title, status, assignee, created_at, " + " workspace_kind, workspace_path) " + "VALUES ('t_b1', 'Block C re-review', 'running', 'tchalla', ?, " + " 'scratch', NULL)", + (now,), + ) + conn.commit() + yield conn + conn.close() + + +class TestBlockGate: + def test_fabricated_claim_rejected_when_dispatcher_is_authed( + self, running_task_board, + ) -> None: + """The 2026-06-09 Tchalla case: blocked with + 'gh CLI not authenticated; cannot run gh pr diff 42' + when the dispatcher's shell IS authed (and PR 42 doesn't exist). + Gate should reject the block so the worker has to surface the + real cause.""" + with patch( + "hermes_cli.kanban_db._dispatcher_gh_is_authed", + return_value=True, + ), pytest.raises(FabricatedAuthClaimError): + block_task( + running_task_board, "t_b1", + reason="missing-github-auth: cannot run gh pr diff 42", + ) + # Task state is unchanged + row = running_task_board.execute( + "SELECT status FROM tasks WHERE id = 't_b1'" + ).fetchone() + assert row["status"] == "running" + + def test_genuine_claim_accepted_when_dispatcher_not_authed( + self, running_task_board, + ) -> None: + """When the dispatcher ALSO can't reach gh, the worker's claim + is plausible (genuine subprocess auth gap from #33); we accept + and block normally.""" + with patch( + "hermes_cli.kanban_db._dispatcher_gh_is_authed", + return_value=False, + ): + ok = block_task( + running_task_board, "t_b1", + reason="missing-github-auth: cannot run gh pr diff 42", + ) + assert ok + row = running_task_board.execute( + "SELECT status FROM tasks WHERE id = 't_b1'" + ).fetchone() + assert row["status"] == "blocked" + + def test_non_auth_claim_block_passes_through(self, running_task_board) -> None: + """A block reason that doesn't claim auth doesn't trip the gate.""" + with patch( + "hermes_cli.kanban_db._dispatcher_gh_is_authed", + return_value=True, + ): + ok = block_task( + running_task_board, "t_b1", + reason="remediation: need Friday to fix the integration test " + "before this re-review can proceed", + ) + assert ok + + def test_empty_reason_skips_gate(self, running_task_board) -> None: + """``reason=None`` is rare but valid (e.g. orchestrator-initiated + block) and the gate must not crash on it.""" + with patch( + "hermes_cli.kanban_db._dispatcher_gh_is_authed", + return_value=True, + ): + ok = block_task(running_task_board, "t_b1", reason=None) + assert ok + + def test_event_recorded_on_gate_rejection(self, running_task_board) -> None: + with patch( + "hermes_cli.kanban_db._dispatcher_gh_is_authed", + return_value=True, + ), pytest.raises(FabricatedAuthClaimError): + block_task( + running_task_board, "t_b1", + reason="gh auth login required for this worker", + ) + events = running_task_board.execute( + "SELECT kind FROM task_events WHERE task_id = 't_b1' " + "ORDER BY id DESC LIMIT 1" + ).fetchone() + assert events is not None + assert events["kind"] == "block_blocked_fabricated_auth_claim" diff --git a/tools/kanban_tools.py b/tools/kanban_tools.py index 20a522f90a4b..3d92bb7ccfcc 100644 --- a/tools/kanban_tools.py +++ b/tools/kanban_tools.py @@ -612,11 +612,22 @@ def _handle_block(args: dict, **kw) -> str: try: kb, conn = _connect(board=board) try: - ok = kb.block_task( - conn, tid, - reason=reason, - expected_run_id=_worker_run_id(tid), - ) + try: + ok = kb.block_task( + conn, tid, + reason=reason, + expected_run_id=_worker_run_id(tid), + ) + except kb.FabricatedAuthClaimError as fab_err: + # v6.7 gate rejected the block — the reason claimed + # github auth was missing but the dispatcher's shell + # IS authed. Task state unchanged; the worker has to + # surface the real cause and call kanban_block again. + # See hermes-jarvis#65. + return tool_error( + f"{fab_err}\nYour task is still in-flight " + f"(no state change)." + ) if not ok: return tool_error( f"could not block {tid} (unknown id or not in " From bb156e7f7a7334005476033d10f976eac15855c8 Mon Sep 17 00:00:00 2001 From: Jarvis Date: Tue, 9 Jun 2026 16:25:24 -0700 Subject: [PATCH 2/2] =?UTF-8?q?fix(dispatcher):=20v6.7=20Part=203=20self-r?= =?UTF-8?q?eview=20=E2=80=94=20tighter=20auth=20pattern=20+=20bounded=20re?= =?UTF-8?q?spawn=20exemption?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Self-review of PR #13 surfaced two P1 issues that this commit fixes together. ## P1 — _AUTH_CLAIM_PATTERN was too permissive The original regex matched any occurrence of an auth-claim phrase within the reason — including the EXACT honest cause workers should surface. Specifically: "investigated: gh CLI is authenticated; the real issue is that PR 42 doesn't exist" This was a false positive: the gate would reject the worker's honest diagnosis. Other false positives included documentation mentions ("gh auth login is needed in setup docs") and handoff-context mentions ("used gh auth token earlier in the task"). Fix: anchor the pattern to the LEADING substring of the reason (optionally after a `cause:`/`blocker:`/`reason:`/`infra:` prefix). The 2026-06-09 Tchalla case ("missing-github-auth: gh token for ...") still matches; the honest diagnosis above no longer does. 7 new tests in `TestAuthClaimNotLeading` lock in the contract. ## P1 — Respawn exemption was unbounded The review-role exemption in `check_respawn_guard` correctly stopped the 17-tick `respawn_guarded` loop, but it had no upper bound — a flapping review profile (crashes-fast-each-attempt) could respawn unconditionally on the URL signal, burning tokens until the auto-block path noticed. Fix: the exemption only applies while consecutive_failures < max_retries (the same brake every other path observes). Once the breaker is exhausted, fall through to the `active_pr` check and let auto_block catch it shortly after. 2 new tests in `TestRespawnExemptionBounded`: - Under-limit review stays exempt (existing behavior preserved) - At-limit review falls through to active_pr (new bound enforced) ## Tests 33 passed in tests/cli/test_v6_7_subprocess_honesty.py — up from 24 in the original PR #13 (9 new). Zero regressions on adjacent paths. Co-Authored-By: Claude Opus 4.7 (1M context) --- hermes_cli/kanban_db.py | 44 ++++++-- tests/cli/test_v6_7_subprocess_honesty.py | 130 ++++++++++++++++++++++ 2 files changed, 166 insertions(+), 8 deletions(-) diff --git a/hermes_cli/kanban_db.py b/hermes_cli/kanban_db.py index 682637137eb4..7d13ff1acade 100644 --- a/hermes_cli/kanban_db.py +++ b/hermes_cli/kanban_db.py @@ -4044,11 +4044,25 @@ def edit_completed_task_result( return True +# The auth-claim pattern is anchored to leading position (or right after +# a ``cause:`` / ``blocker:`` prefix) so honest diagnoses that mention +# auth in passing don't false-positive. Self-review note from +# hermes-jarvis#61: the original pattern matched any occurrence in 300 +# chars, including the EXACT honest cause workers should surface (e.g. +# ``investigated: gh CLI is authenticated; the real issue is PR 42 +# doesn't exist``). Now the claim must be the leading substring of the +# reason after stripping whitespace/labels. +_AUTH_CLAIM_PHRASES = ( + r"missing[\s-]+github[\s-]+auth" + r"|gh\s+auth\s+(?:login|status|token)\s+(?:required|missing|fails|failed|not\s+available)" + r"|GITHUB_TOKEN\s+(?:required|missing|unset|not\s+set)" + r"|gh\s+CLI\s+(?:is\s+)?not\s+authenticated" + r"|cannot\s+(?:run|invoke)\s+`?gh\s+" +) + _AUTH_CLAIM_PATTERN = re.compile( - r"(missing[\s-]+github[\s-]+auth|" - r"gh\s+auth\s+(?:login|status|token)|" - r"GITHUB_TOKEN\s+(?:required|missing|unset|not\s+set)|" - r"gh\s+CLI\s+(?:is\s+)?(?:not\s+)?authenticated)", + r"^\s*(?:(?:cause|blocker|reason|infra)\s*:\s*)?" + r"(" + _AUTH_CLAIM_PHRASES + r")", re.IGNORECASE, ) @@ -5986,12 +6000,26 @@ def check_respawn_guard(conn: sqlite3.Connection, task_id: str) -> Optional[str] # self-recovered by spawning a duplicate card whose body had the URL # stripped. That self-recovery was a kludge; this exemption is the # real fix. - role_row = conn.execute( - "SELECT assignee FROM tasks WHERE id = ?", (task_id,), + # + # Bounded: the exemption only applies while consecutive_failures < + # max_retries (the same brake every other path observes). If a + # buggy review profile keeps crashing on claim, the third failure + # still trips the breaker and the task gets auto-blocked — without + # this bound a flapping review could respawn unboundedly while the + # URL stayed in its body. Self-review note from hermes-jarvis#61. + row = conn.execute( + "SELECT assignee, consecutive_failures, max_retries " + " FROM tasks WHERE id = ?", + (task_id,), ).fetchone() - assignee = (role_row["assignee"] or "").lower() if role_row else "" + assignee = (row["assignee"] or "").lower() if row else "" if assignee in {"tony", "tchalla", "vision", "reviewer"}: - return None + cf = row["consecutive_failures"] or 0 + mr = row["max_retries"] if row["max_retries"] is not None else DEFAULT_FAILURE_LIMIT + if cf < mr: + return None + # Otherwise fall through to the active_pr check (and let + # auto_block catch the breaker shortly). pr_cutoff = now - _RESPAWN_GUARD_PR_WINDOW for c in conn.execute( diff --git a/tests/cli/test_v6_7_subprocess_honesty.py b/tests/cli/test_v6_7_subprocess_honesty.py index 1f06ce6569df..731171d4b1d6 100644 --- a/tests/cli/test_v6_7_subprocess_honesty.py +++ b/tests/cli/test_v6_7_subprocess_honesty.py @@ -314,3 +314,133 @@ def test_event_recorded_on_gate_rejection(self, running_task_board) -> None: ).fetchone() assert events is not None assert events["kind"] == "block_blocked_fabricated_auth_claim" + + +# ===================================================================== +# Self-review fixes for PR #13 — tighter auth pattern +# ===================================================================== + + +class TestAuthClaimNotLeading: + """The original pattern matched any occurrence in 300 chars, + including the EXACT honest cause workers should surface. After + tightening, the claim must be the LEADING substring of the reason + (optionally after a ``cause:``/``blocker:``/``reason:``/``infra:`` + prefix). + """ + + def test_honest_diagnosis_does_not_match(self) -> None: + """The exact false positive from the self-review: a worker + surfacing an honest diagnosis that MENTIONS gh auth status in + passing must not trigger the gate.""" + reason = ( + "investigated: gh CLI is authenticated; the real issue is " + "that PR 42 doesn't exist on the remote." + ) + assert _reason_claims_missing_gh_auth(reason) is None + + def test_documentation_mention_does_not_match(self) -> None: + assert ( + _reason_claims_missing_gh_auth( + "documenting that gh auth login is needed in setup docs " + "as a follow-up; not blocking this task" + ) + is None + ) + + def test_handoff_context_mention_does_not_match(self) -> None: + assert ( + _reason_claims_missing_gh_auth( + "used gh auth token to fetch the token earlier in the task " + "but it's now stale; primary blocker is API rate limit" + ) + is None + ) + + def test_wrong_account_diagnosis_does_not_match(self) -> None: + assert ( + _reason_claims_missing_gh_auth( + "gh CLI is authenticated for the wrong account; switch " + "needed but that's a separate setup task" + ) + is None + ) + + def test_cause_prefix_still_matches(self) -> None: + """A worker that prefixes the claim with ``cause:`` or + similar is still caught.""" + assert ( + _reason_claims_missing_gh_auth( + "cause: gh CLI is not authenticated; cannot run gh pr diff" + ) + is not None + ) + + def test_blocker_prefix_still_matches(self) -> None: + assert ( + _reason_claims_missing_gh_auth( + "blocker: GITHUB_TOKEN required for this operation" + ) + is not None + ) + + def test_legacy_leading_form_still_matches(self) -> None: + """The 2026-06-09 Tchalla case still trips the gate.""" + assert ( + _reason_claims_missing_gh_auth( + "missing-github-auth: gh token for jarvis-stark-ops is " + "invalid, so I cannot post the required PR comment" + ) + is not None + ) + + +class TestRespawnExemptionBounded: + """The exemption now respects the failure breaker — a flapping + review can respawn at most max_retries times before falling back + to the active_pr guard (and shortly after, auto_block).""" + + def test_review_under_limit_exempted(self, board_conn) -> None: + import time + now = int(time.time()) + board_conn.execute( + "INSERT INTO tasks (id, title, status, assignee, " + " consecutive_failures, max_retries, created_at, " + " workspace_kind, workspace_path) " + "VALUES ('t_rb1', 'review', 'ready', 'tchalla', 1, 3, ?, " + " 'scratch', NULL)", + (now,), + ) + board_conn.execute( + "INSERT INTO task_comments (task_id, author, body, created_at) " + "VALUES ('t_rb1', 'kaipo', " + " 'pr at https://github.com/o/r/pull/1', ?)", + (now,), + ) + board_conn.commit() + assert check_respawn_guard(board_conn, "t_rb1") is None + + def test_review_at_limit_falls_through_to_active_pr( + self, board_conn, + ) -> None: + """Once consecutive_failures >= max_retries, the exemption + no longer applies and the URL signal trips the guard normally + (and the breaker will catch it shortly).""" + import time + now = int(time.time()) + board_conn.execute( + "INSERT INTO tasks (id, title, status, assignee, " + " consecutive_failures, max_retries, created_at, " + " workspace_kind, workspace_path) " + "VALUES ('t_rb2', 'review', 'ready', 'tchalla', 3, 3, ?, " + " 'scratch', NULL)", + (now,), + ) + board_conn.execute( + "INSERT INTO task_comments (task_id, author, body, created_at) " + "VALUES ('t_rb2', 'kaipo', " + " 'pr at https://github.com/o/r/pull/1', ?)", + (now,), + ) + board_conn.commit() + assert check_respawn_guard(board_conn, "t_rb2") == "active_pr"