fix(kanban): don't strand babysit/review tasks on the active_pr respawn guard - #53
Conversation
…wn guard The respawn guard's active_pr check (264e85b) returns "active_pr" when a GitHub PR URL appears in any recent task comment, to stop an implementation task from re-spawning and opening a duplicate PR. It misfired on babysit / review / re-verify tasks: their job is to operate on a PR that already exists, so they post that PR's URL in comments and must re-spawn to keep working it. The guard stranded them in `ready` for the full 24h PR window. Discriminator: an impl task at risk of a duplicate has the URL only in its comments; a babysit task names the PR in its title/body. check_respawn_guard now also reads title+body and skips the comment-based active_pr guard when either contains a PR reference (full URL, or bare `PR #N` / `owner/repo#N` via new _RESPAWN_GUARD_PR_REF_RE). Duplicate-PR protection for impl tasks unchanged. Fail-before/pass-after: 2 babysit tests fail with active_pr on old code, pass with fix; impl-task guard test passes both ways. Full respawn suite: 20 passed. Patch note: ~/.hermes/plans/hermes-patches/respawn-guard-babysit-anchored-pr.md
🔎 Lint report:
|
| Rule | Count |
|---|---|
unresolved-attribute |
2 |
First entries
run_agent.py:3026: [unresolved-attribute] unresolved-attribute: Object of type `Self@get_credits_spent_micros` has no attribute `_credits_session_start_micros`
tests/run_agent/test_credits_notices_toggle.py:76: [unresolved-attribute] unresolved-attribute: Unresolved attribute `_credits_session_start_micros` on type `AIAgent`
✅ Fixed issues (1):
| Rule | Count |
|---|---|
invalid-assignment |
1 |
First entries
tests/run_agent/test_credits_notices_toggle.py:76: [invalid-assignment] invalid-assignment: Object of type `None` is not assignable to attribute `_credits_session_start_micros` of type `int`
Unchanged: 6093 pre-existing issues carried over.
Diagnostics are surfaced as warnings — this check never fails the build.
There was a problem hiding this comment.
Code Review
This pull request introduces a mechanism to prevent babysit, review, or re-verify tasks anchored to pre-existing PRs from being incorrectly stranded by the active PR respawn guard. It does this by checking if the task's title or body contains a PR reference, skipping the guard if a reference is found. The review feedback highlights a potential issue where the new regular expression could match file paths with line numbers (e.g., src/main.py#10), which would bypass the guard for implementation tasks. The reviewer suggests refining the regex to exclude common file extensions and adding a test case to verify this behavior.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| _RESPAWN_GUARD_PR_REF_RE = re.compile( | ||
| r"(?:\bPR\s*#\d+|[A-Za-z0-9._-]+/[A-Za-z0-9._-]+#\d+)", | ||
| re.IGNORECASE, |
There was a problem hiding this comment.
The regular expression _RESPAWN_GUARD_PR_REF_RE matches any string of the form owner/repo#number. However, this pattern also matches common file paths with line numbers or anchors, such as src/main.py#10 or utils/helper.js#42. If an implementation task's body contains such a file path, it will be incorrectly identified as anchored_to_pr, bypassing the active_pr guard and risking duplicate PR creation or worker storms.
To prevent this, we can use a negative lookahead to exclude common file extensions (like .py, .js, .ts, .go, .java, etc.) before the # character.
| _RESPAWN_GUARD_PR_REF_RE = re.compile( | |
| r"(?:\bPR\s*#\d+|[A-Za-z0-9._-]+/[A-Za-z0-9._-]+#\d+)", | |
| re.IGNORECASE, | |
| _RESPAWN_GUARD_PR_REF_RE = re.compile( | |
| r"(?:\\bPR\\s*#\\d+|[A-Za-z0-9._-]+/(?![A-Za-z0-9._-]*\\.(?:py|js|ts|go|rb|sh|h|c|cpp|java|cs|rs|php|kt|swift|html|css|yml|yaml|json|md|txt|sql)#)[A-Za-z0-9._-]+#\\d+)", | |
| re.IGNORECASE, | |
| ) |
| def test_respawn_guard_impl_task_still_guarded_by_pr_comment(kanban_home): | ||
| """An IMPLEMENTATION task (no PR reference in title/body) is STILL guarded | ||
| by active_pr when a PR URL appears in its comments — the duplicate-PR | ||
| protection the guard was built for must remain intact.""" | ||
| with kb.connect() as conn: | ||
| t = kb.create_task( | ||
| conn, title="Implement rate limiter", assignee="alice", | ||
| ) | ||
| kb.add_comment( | ||
| conn, t, "worker", | ||
| "PR created: https://github.com/totemx-AI/subsidysmart/pull/42", | ||
| ) | ||
| reason = kb.check_respawn_guard(conn, t) | ||
| assert reason == "active_pr" |
There was a problem hiding this comment.
Let's add a test case to verify that an implementation task containing a file path with a line number (e.g., src/main.py#42) in its body is still correctly guarded by active_pr and not incorrectly treated as anchored to a PR.
def test_respawn_guard_impl_task_still_guarded_by_pr_comment(kanban_home):
"""An IMPLEMENTATION task (no PR reference in title/body) is STILL guarded
by active_pr when a PR URL appears in its comments — the duplicate-PR
protection the guard was built for must remain intact."""
with kb.connect() as conn:
t = kb.create_task(
conn, title="Implement rate limiter", assignee="alice",
)
kb.add_comment(
conn, t, "worker",
"PR created: https://github.com/totemx-AI/subsidysmart/pull/42",
)
reason = kb.check_respawn_guard(conn, t)
assert reason == "active_pr"
def test_respawn_guard_impl_task_with_file_path_still_guarded(kanban_home):
"""An implementation task with a file path and line number in its body
(e.g., 'src/main.py#42') is STILL guarded by active_pr when a PR URL
appears in its comments."""
with kb.connect() as conn:
t = kb.create_task(
conn,
title="Fix bug in rate limiter",
body="See src/main.py#42 for details",
assignee="alice",
)
kb.add_comment(
conn, t, "worker",
"PR created: https://github.com/totemx-AI/subsidysmart/pull/42",
)
reason = kb.check_respawn_guard(conn, t)
assert reason == "active_pr"
Problem
Kanban babysit / re-verify tasks were stranding in
readyfor hours. Diagnostics flagged them asstranded_in_ready"no worker" even though the embedded dispatcher was healthy and ticking every 5s. The dispatcher was deliberately deferring them each tick withrespawn_guarded {"reason":"active_pr"}.Root cause
check_respawn_guard(added in 264e85b, "add respawn guard to block repeat worker storms") returnsactive_prwhen a GitHub PR URL appears in any recent task comment. Its purpose is to stop an implementation task from re-spawning and opening a duplicate PR.That signal misfires on babysit / review / re-verify tasks. Their entire job is to operate on a PR that already exists, so they legitimately post that PR's URL in their own comments AND must re-spawn to keep working it. The guard saw the URL, assumed duplicate-PR risk, and stranded them for the full 24h
_RESPAWN_GUARD_PR_WINDOW. The guard couldn't distinguish "worker created a new PR" from "task is about an existing PR."Fix
The discriminator is where the PR reference lives:
# noqa:isn't parsed as a directive #48", "cpe-research/research-agent#801").check_respawn_guardnow also reads the tasktitleandbody; if either contains a PR reference (full URL via_RESPAWN_GUARD_PR_URL_RE, or a barePR #N/owner/repo#Nref via the new_RESPAWN_GUARD_PR_REF_RE), the task is anchored to a pre-existing PR and the comment-basedactive_prguard is skipped. The duplicate-PR protection for implementation tasks (no PR ref in title/body) is unchanged. The bare-ref regex requires thePRkeyword or anowner/repoprefix so a plain#42issue mention doesn't exempt a task.Verification
Fail-before/pass-after, proven by stashing the source change:
test_respawn_guard_babysit_task_not_stranded_by_own_pr_commentandtest_respawn_guard_babysit_task_owner_repo_ref_not_strandedfail withassert 'active_pr' is Noneon old code, pass with the fix.test_respawn_guard_impl_task_still_guarded_by_pr_commentpasses both ways (duplicate-PR protection intact).Patch note:
~/.hermes/plans/hermes-patches/respawn-guard-babysit-anchored-pr.md