Skip to content

fix(kanban): reject recycled PIDs as prior worker owners - #956

Merged
Kyzcreig merged 4 commits into
mainfrom
fix/kanban-prior-pid-reuse
Sep 24, 2026
Merged

Kyzcreig merged 4 commits into
mainfrom
fix/kanban-prior-pid-reuse

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Stacked on #952 for its stuck-card notifier. Verify process start within the claim/spawn event window before refusing a new claim; skip explicitly runtime-bounded old runs. Page continuous prior_worker_still_alive claim refusals after 15 minutes through the existing guard-stuck #alerts lane, with prior PID and diagnostic command. Tests: 211 passed, 1 skipped across second-claim, reclaim, DB and watcher suites; ruff passed. Start-time mutant fails the real-PID claim test. No live board mutation or deployment.


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Kanban card: t_089fcb25 (Argus PASS r5 run 8950)

@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Upstream surface audit: NousResearch/main and upstream PR NousResearch#121177 do not contain _prior_worker_still_alive, _spawned_owner_alive, or this claim-rejection reason (git grep on both refs returned no hits). Upstream has a separate _pid_recycled guard for reclaim in kanban_db_dispatch.py, so porting this fork-specific claim guard would create a new subsystem rather than fix an existing path. The generic stuck-card notifier being extended here is linked to the upstream port at NousResearch#121177. This PR is intentionally stacked on fork #952; land #952 first, then rebase this PR onto main.

Kyzcreig added a commit that referenced this pull request Sep 24, 2026
Minimum of #952 (t_7d7ff489) that the prior_worker >15-min page needs,
so #956 lands standalone on main: respawn_guard_stuck_tasks + constants,
requeue_task + 'requeued' operator kind + kanban requeue CLI (the
active_pr clear verb), and the watcher probe/notifier/sender/loop wiring
with their tests. #952's dependency_wait resume and PR-intent
consumption changes are NOT carried; #952 rebases on top.

Verified: focused kanban db/cli/second-claim/reclaim/watchers suites
205 passed, 1 skipped; ruff clean.
@Kyzcreig
Kyzcreig force-pushed the fix/kanban-prior-pid-reuse branch from d36f28b to 8ccf5b7 Compare September 24, 2026 10:23
Kyzcreig added a commit that referenced this pull request Sep 24, 2026
…r review skip

Argus r2 caveats on #956:
C1: the prior_worker_still_alive page hardcoded 'READY card'. The probe
now carries the card status and the page says '<STATUS> card' (REVIEW
for the review claim door). Dispatcher STUCK log no longer says READY.
C2: new test_active_pr_stuck_page_is_ready_lane_only[ready|review]
gates the status != 'ready' skip for the active_pr page.

Verified: removing the skip -> [review] RED; hardcoding READY in the
sender -> test_guard_stuck_sender_routes_to_alerts RED; dropping status
from the probe -> both prior_worker parametrizations RED. Focused
db/second-claim/reclaim/watchers/cli + stdin-guard tests: 213 passed,
1 skipped; ruff clean; scripts/check_subprocess_stdin.py clean.

Upstream ref: NousResearch#121177
@Kyzcreig
Kyzcreig changed the base branch from fix/t_7d7ff489-respawn-guard-depwait to main September 24, 2026 10:23
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Unstacked onto main (t_4a427d98). Head 8ccf5b7, base main. Land order: #956 first, then #952 rebases on top.

Carried from #952, in commit 876e124 only: respawn_guard_stuck_tasks and its constants, requeue_task with the 'requeued' operator kind and the kanban requeue CLI (the clear verb for active_pr), and the watcher plumbing: _guard_stuck_cards, _GuardStuckNotifier, _send_guard_stuck_alert, the guard_stuck arg on _stall_streak_is_bad, and the dispatcher-loop wiring. Tests came with it, including d55f4e7, the corruption-probe call-count fix that the added watcher call requires. NOT carried: #952's dependency_wait resume and its PR-intent consumption changes.

The code Argus verified in r1/r2 is byte-identical to d36f28b: claim_task, _prior_worker_still_alive, _spawned_owner_alive, _real_pid_started_in_claim, respawn_guard_stuck_tasks at the cherry-pick, plus the watcher functions. I checked this with an AST compare.

C1 and C2 are fixed in 8ccf5b7. The page now uses the card's status ('REVIEW card: …'). The active_pr ready-only skip is gated by test_active_pr_stuck_page_is_ready_lane_only, which goes RED if the skip is removed.

Upstream: NousResearch#121177

Verified 170 passed, 1 skipped in targeted kanban suites. Reused-PID mutant fails the real-PID claim test.
Verified focused kanban and watcher suites: 211 passed, 1 skipped. Ruff passed.
Verify ready/review and lane-transition tests: 4 passed; focused DB/reclaim/watchers: 213 passed, 1 skipped; ruff and subprocess stdin guard passed. Review arm failed against ready-only baseline.
…r review skip

Argus r2 caveats on #956:
C1: the prior_worker_still_alive page hardcoded 'READY card'. The probe
now carries the card status and the page says '<STATUS> card' (REVIEW
for the review claim door). Dispatcher STUCK log no longer says READY.
C2: new test_active_pr_stuck_page_is_ready_lane_only[ready|review]
gates the status != 'ready' skip for the active_pr page.

Verified: removing the skip -> [review] RED; hardcoding READY in the
sender -> test_guard_stuck_sender_routes_to_alerts RED; dropping status
from the probe -> both prior_worker parametrizations RED. Focused
db/second-claim/reclaim/watchers/cli + stdin-guard tests: 213 passed,
1 skipped; ruff clean; scripts/check_subprocess_stdin.py clean.

Upstream ref: NousResearch#121177
@Kyzcreig
Kyzcreig force-pushed the fix/kanban-prior-pid-reuse branch from 8ccf5b7 to 9124c85 Compare September 24, 2026 16:17
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: discord · gate: BYPASS: FR paused by Ace ruling 2026-09-22; gate = kanban Argus PASS run 8950 · why: dispatcher: prior-worker PID-reuse check by process start time + >15m claim-refusal page on ready AND review doors (t_089fcb25): Argus PASS-WITH-CAVEATS r5 run 8950 at 9124c85 (rebased onto main after #952, unstacked); CI green

@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit 7defc24 Sep 24, 2026
57 checks passed
@Kyzcreig
Kyzcreig deleted the fix/kanban-prior-pid-reuse branch September 24, 2026 18:29
@Kyzcreig Kyzcreig added the fleetreview:post-merge Ask FleetReview to review this MERGED pull (merge commit vs first parent) label Sep 24, 2026
Kyzcreig pushed a commit that referenced this pull request Sep 25, 2026
…ecycled PID (t_0ae83825)

Rebased onto main after #946/#956/t_21dfa673. The owner window now carries
the spawned event's start_token, so termination and liveness paths use the
same clock-step-immune identity as the claim guard; legacy rows keep the
causal window. enforce_max_runtime passes conn/task_id per main's run-context
class guard.
@ang-prism

ang-prism Bot commented Sep 27, 2026

Copy link
Copy Markdown

FleetReview

Review: post-merge · head 7defc243150f · duration 19m 11s
Profile: full recipe · policy: below-size-and-path-gates
Roster: B-assert-ctx → gpt-6-sol (openai), B-state → gpt-6-sol (openai), C-assert-xhigh → claude-code-opus-5-5 (anthropic), F → gpt-6-sol (openai), G → grok-4.6 (xai), L6 → gpt-6-sol (openai)

Post-merge review (fleetreview:post-merge override): this reviewed the merge commit against its first parent — the bytes that already shipped. It is not a pre-merge gate pass.

Confidence: 2/5

Findings

  • P1 hermes_cli/kanban_db.py:7319 — Unverified owner · agreed: B-assert-ctx,B-state,C-assert-xhigh,F,G (openai, anthropic, xai)
  • P2 hermes_cli/kanban_db.py:16055 — Wrong PID · agreed: B-assert-ctx,C-assert-xhigh,G (openai, anthropic, xai)
  • P1 hermes_cli/kanban_db.py:7362 — Live owner bypass · agreed: B-assert-ctx,B-state,L6,C-assert-xhigh,F (openai, anthropic)
  • P1 hermes_cli/kanban_db.py:7409 — Late fallback event · agreed: B-state,L6 (openai)
  • P1 hermes_cli/kanban_db.py:16062 — New guard reason shares the existing alert deduplication key · agreed: F (openai)

FleetReview provenance · models: B=gpt-6-sol, C=claude-code-opus-5-5, D=grok-4.6, F=gpt-6-sol · cost: $1.96 · duration: 20m 17s · rounds: 1 · files examined: 6

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

Labels

fleetreview:post-merge Ask FleetReview to review this MERGED pull (merge commit vs first parent)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant