Skip to content

fix(kanban): honor explicit same-PR requeues - #76227

Open
FreedomFrog wants to merge 1 commit into
NousResearch:mainfrom
FreedomFrog:fix/kanban-active-pr-requeue
Open

fix(kanban): honor explicit same-PR requeues#76227
FreedomFrog wants to merge 1 commit into
NousResearch:mainfrom
FreedomFrog:fix/kanban-active-pr-requeue

Conversation

@FreedomFrog

Copy link
Copy Markdown

Summary

  • preserve the active-PR duplicate-spawn guard until a deliberate requeue occurs
  • allow promoted, unblocked, reclaimed, or status-requeued tasks to continue on an existing PR
  • add direct guard and dispatcher regression coverage for same-PR review cycles

Why

Kanban documents blocked → unblock → re-run and dependency promotion as supported workflows. The active-PR guard currently treats any recent PR URL as an unconditional 24-hour lock, stranding deliberate same-PR re-reviews even after remediation dependencies complete.

Test plan

  • RED confirmed: new dependency-promotion regression returned active_pr before the patch
  • scripts/run_tests.sh tests/hermes_cli/test_kanban_db.py -q — 33 passed
  • focused Kanban group — 63 passed
  • ruff check hermes_cli/kanban_db.py tests/hermes_cli/test_kanban_db.py — passed
  • live read-only probe against two currently stranded tasks returns no guard with patched code
  • broader tests/hermes_cli run: 3,889 passed; 5 unrelated pre-existing failures in test_update_eol_churn.py reproduced when run alone

@FreedomFrog

Copy link
Copy Markdown
Author

Live end-to-end verification on an affected multi-board installation:

  • Before patch: both deliberate same-PR re-review tasks returned active_pr; dispatcher repeatedly spawned 0.
  • Patched code, read-only guard probe: both tasks returned None.
  • Bounded real dispatch (--max 2) spawned both tasks.
  • Board readback showed both running; process readback showed two live gpt-5.6-sol/openai-codex worker PIDs.

This verifies the production dispatcher path, not only the unit helper. The installation still preserves the guard when the latest PR comment has no later explicit requeue.

@alt-glitch alt-glitch added type/bug Something isn't working P3 Low — cosmetic, nice to have comp/cron Cron scheduler and job management duplicate This issue or pull request already exists sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation labels Aug 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Duplicate of #74432: both patches use the same later requeue-event versus latest PR-comment ordering rule in check_respawn_guard().

@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused Kanban fix. The premise is confirmed on current main: check_respawn_guard() returns "active_pr" immediately after finding recent PR evidence at hermes_cli/kanban_db.py:8041-8048, and dispatch_once() applies that guard before claiming at hermes_cli/kanban_db.py:8413.

Problems

  • The tests do not cover timestamp equality. Comments and events are both recorded at integer-second resolution (hermes_cli/kanban_db.py:3551, hermes_cli/kanban_db.py:3875), while the new condition intentionally requires a strictly later event. Add an equal-timestamp regression asserting the PR guard remains active.
  • The member discussion identifies this as duplicating open fix(kanban): honor explicit requeue after PR evidence #74432; that PR's diff already covers equal timestamps and the broader ordering cases.

Suggested changes

Automated hermes-sweeper review.

@teknium1 teknium1 added the sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform label Aug 1, 2026

@GottZ GottZ left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was generated by AI during triage.

Summary

No member issue is listed for this complex; two open PRs address the same respawn-guard defect. Both make a later explicit requeue override recent PR evidence, while #76640 additionally updates the recent-success path and covers the production manual-promotion event and timestamp-equality boundary; #76227 uniquely contributes a dispatcher-path regression test in the supplied evidence.

Related pull requests

  • #76227 related — (+99/-1) — duplicate core with dispatcher-test salvage: The implementation applies the later-requeue ordering rule also attributed by a contributor to #74432, but it omits promoted_manual and equal-timestamp coverage. Despite the keep_open verdict on #76227, closing it after salvaging its dispatcher-path regression is supported by the contributor duplicate finding and by #76640's broader event handling; the supplied evidence does not establish that #74432 already has equivalent dispatcher coverage.
  • #76640 related — (+115/-3) — keep open with a salvage path: Its current diff includes promoted_manual in both respawn-guard allowlists, tests the production promote_task() API, preserves the guard on equal timestamps, and extends the related recent-success path. This addresses the contributor's blocking manual-promotion finding and the keep_open review's requested equality coverage; a contributor still identifies the core active-PR change as a duplicate of #74432, so reconciliation with #74432 remains necessary.

Duplicates

#76227 and #76640 substantially duplicate each other on the active-PR later-requeue bypass, and contributor comments identify each as duplicating #74432. Their supplied test scope is not identical: #76227 has dispatcher-path coverage, while #76640 has production manual-promotion, equal-timestamp, and recent-success coverage.

Suggested consolidation

Keep #76640 open with a salvage path: reconcile its shared active-PR change against #74432, retain the demonstrated promoted_manual, promote_task(), equal-timestamp, and recent-success coverage, and import #76227's dispatcher-path regression if #74432 does not already cover that path. Then close #76227 as a duplicate of #74432, superseded for consolidation purposes by the broader salvage in #76640; this departs from #76227's keep_open verdict because its implementation misses the documented production event and timestamp boundary, while preserving its one evidenced distinct test contribution.

Cross-PR triage: Reviewed 2 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 13 kB of PR diffs, 2 kB of issue/PR text, 6 kB of discussion (8 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

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

Labels

comp/cron Cron scheduler and job management duplicate This issue or pull request already exists P3 Low — cosmetic, nice to have sweeper:blast-moderate Sweeper blast radius: moderate — a subsystem or single platform sweeper:risk-automation Sweeper risk: may affect CI, automerge, label sync, or maintainer automation sweeper:risk-session-state Sweeper risk: may lose/corrupt/mis-associate session or context state type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants