Skip to content

fix(kanban): reclaim pid-less claim when local claimer is dead - #983

Merged
Kyzcreig merged 5 commits into
mainfrom
fix/reclaim-dead-claimer
Sep 25, 2026
Merged

Kyzcreig merged 5 commits into
mainfrom
fix/reclaim-dead-claimer

Conversation

@Kyzcreig

@Kyzcreig Kyzcreig commented Sep 24, 2026 •

Copy link
Copy Markdown
Collaborator

Problem

A gateway may die after claiming a task but before stamping worker_pid. The worker sweep repeatedly treats the missing PID as unprovable, extends the claim, and pages every tick; the card cannot age out.

Fix

For a host-local claim with no worker PID, probe the PID embedded in claim_lock with psutil.Process(pid) (the portable equivalent of os.kill(pid, 0); signal 0 kills console groups on Windows). NoSuchProcess proves the claimer is gone, so normal reclaim releases the card and records claimer_pid_dead. Live, inaccessible, and malformed claimers remain fail-closed. Foreign-host reclaim policy is unchanged. Both manual and expired-TTL paths share this termination probe.

Verification

  • New regression: manual and TTL dead-claimer cases failed on fork main before the fix; live-claimer probe also failed.
  • nice -n 19 .../venv/bin/python -m pytest tests/hermes_cli/test_kanban_reclaim_unprovable_liveness.py tests/hermes_cli/test_kanban_second_claim_class.py tests/hermes_cli/test_kanban_db.py -o addopts= -q: 205 passed, 1 skipped.
  • Ruff and git diff --check: pass.

Task: hermes-fork/t_9d7e0142. Upstream contribution follows fork-first verification.

Upstream port (split dispatcher layout): NousResearch#121730

Apollo added 2 commits September 24, 2026 08:39
Probe the local claim owner with kill(pid, 0) before treating a missing worker PID as unknown. An absent claimer cannot finish its launch; preserve fail-closed grace for live or inaccessible claimers. Cover manual and TTL reclaim plus foreign-host behavior.
Apollo added 2 commits September 24, 2026 09:07
A dead claimer does not prove no worker was spawned: _default_spawn's
Popen(start_new_session=True) precedes the _set_worker_pid commit, so a
gateway killed in that window leaves a detached, heartbeating worker with
no stamped pid. Releasing that claim hands the card to a second worker
(t_09180e10 double-worker shape).

The no-pid branch now looks for heartbeat/spawned events bound to the
CURRENT run before probing the claimer; any such evidence keeps the claim
liveness_unprovable. Prior-run evidence is ignored so a never-spawned
retry still releases. Applies to reclaim_task, release_stale_claims and
detect_stale_running (all three pass conn/task_id).

Regression: real-subprocess orphan arm (manual/ttl/stale) is RED on
5ad2916 and GREEN here.
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

Round 2 (addresses review B1: dead claimer != no worker spawned).

The no-pid branch now checks for heartbeat/spawned events bound to the CURRENT run before it probes the claimer. Any such evidence keeps the claim liveness_unprovable, so it stays held and no second worker is spawned. Evidence from a prior run is ignored, so a never-spawned retry still releases.

Class sweep over every caller of _terminate_reclaimed_worker / _worker_survived_termination:

  • reclaim_task, release_stale_claims, detect_stale_running: these can reach the no-pid branch. All three now pass conn/task_id and are covered by the orphan test (manual/ttl/stale).
  • progress-stall reclaim: dismissed. It only iterates rows with worker_pid set.
  • ancestor-reopen invalidation: dismissed. It is a post-commit best-effort kill whose result is never gated.
  • _abort_lost_claim_spawn: dismissed. It is always called with the concrete pid Popen returned.
  • upstream archive and terminal-row reap: dismissed. The pid is known and the claim is already released or terminal.

Tests: the real-subprocess orphan arm is RED on the previous head (3 failed) and GREEN on this head. The incident, alive-claimer and foreign-host arms are still green. Adjacent suites are green.

… context

Argus r2 (t_9d7e0142):
- B2: dashboard _set_status_direct now passes conn/task_id; the no-pid branch
  fails closed when either is omitted, and an AST guard asserts every gated
  _terminate_reclaimed_worker call site passes them.
- B3: a dead claimer with no current-run worker evidence is held until
  DEAD_CLAIMER_LAUNCH_BOUND_SECONDS (= claim TTL, 900 s) after the claim;
  live ledger first-heartbeat max is 708 s.
- B5: with evidence, release once the newest heartbeat/spawned event on the
  current run is older than DEFAULT_CLAIM_HEARTBEAT_MAX_STALE_SECONDS.
- B4: set-model test fakes accept the new kwargs.
@Kyzcreig

Copy link
Copy Markdown
Collaborator Author

🤖 merged-by: apollo · lane: kanban-merge-pass · gate: BYPASS: FleetReview paused by Ace 2026-09-22 (state/fleetreview-pause marker present) · why: t_9d7e0142: reclaim: dead claimer pid w/o stamped worker pid = proven death (r3, Argus r2 B2-B5 addressed); Argus off card review (Ace 13:08), CI green

@Kyzcreig
Kyzcreig added this pull request to the merge queue Sep 24, 2026
Merged via the queue into main with commit a33a28a Sep 25, 2026
57 checks passed
@Kyzcreig
Kyzcreig deleted the fix/reclaim-dead-claimer branch September 25, 2026 00:14
@Kyzcreig Kyzcreig added the fleetreview:post-merge Ask FleetReview to review this MERGED pull (merge commit vs first parent) label Sep 25, 2026
@ang-prism

ang-prism Bot commented Sep 27, 2026

Copy link
Copy Markdown

FleetReview

Review: post-merge · head a33a28aa9b51 · duration 17m 08s
Profile: light (merit: default light: lines 450<800, files 4<1000000, hunks 16<1000000, no hot path) · policy: changed-lines>400
Roster: B-state → gpt-6-sol (openai), C-assert-xhigh → claude-code-opus-5-5 (anthropic), 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.

profile: light (rule: default light: lines 450<800, files 4<1000000, hunks 16<1000000, no hot path) · round 0 · members: B-state, L6, C-assert-xhigh, G · families: anthropic,openai,xai

Confidence: 2/5

Findings

  • P1 hermes_cli/kanban_db.py:13980 — Duplicate workers · agreed: B-state,L6,C-assert-xhigh (openai, anthropic)
  • P0 tests/hermes_cli/test_kanban_reclaim_unprovable_liveness.py:217 — Conflicting expectations · agreed: B-state,L6,C-assert-xhigh,G (openai, anthropic, xai)

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

ang-fleet-workers Bot added a commit that referenced this pull request Oct 2, 2026
…ins, launch bound follows claim TTL, cron catch-up opt-out

Proved on ace-ai (Linux; these files skip on darwin):
- tests/e2e/core/kanban/*: _helpers.Board writes KANBAN_FAST_CONFIG
  (kanban.rate_limit_cooldown_seconds: 0 — the fork's config beats the
  FAST_ENV env bridge and load_config fills the 300 s default; receipt_gate:
  false — #1621 refused the rigs' prose-only kanban_complete). dispatcher
  restart: body check strips the fork's `origin:` provenance line.
  rate_limit_review 3 passed; worker_contract 2 passed/2 xfailed;
  decompose_billing 1 passed/3 xfailed; dispatcher_restart 2 passed.
- hermes_cli/kanban_db.py: _dead_claimer_launch_bound_seconds() — the #983
  pid-less dead-claimer hold (900 s) follows HERMES_KANBAN_CLAIM_TTL_SECONDS
  when set; the SIGKILL-mid-tick rig (3 s TTL) otherwise strands claims
  15 min. Default unchanged; tests/hermes_cli kanban_db +
  reclaim_unprovable_liveness + quota_exit: 255 passed.
- tests/e2e/core/delivery/test_cron_virtual_clock_soak.py: fixtures pin
  cron.oneshot_catchup_s: 0 (fork #1087 fires a missed one-shot late; the
  oracle models the upstream never-fires contract). 5 passed/1 xfailed.
- tests/e2e/core/providers/test_openai_codex_pool.py: two cells xfail
  (strict=False) — they pin upstream's dead-row-on-disk + stdout re-login
  model that fork #673 (codex_owner receipts) replaced. 1 passed/2 xfailed.
Ledger updated: upgrade/git reds are the fork remote lacking release tags
newer than v2026.5.7 (N-1 installer predates --non-interactive) -> FOLLOWUP.
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