fix(tests): hermetic kanban dispatch tests + default_assignee spawn regression - #43
Merged
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📥 CommitsReviewing files that changed from the base of the PR and between 3c3ce6da405c723497bfaee8e3ad1c754137b605 and 4081853. 📒 Files selected for processing (5)
💤 Files with no reviewable changes (5)
📝 WalkthroughWalkthroughdispatch_once now uses the task's effective assignee (row_assignee) for spawnability and quarantine checks; quarantine bookkeeping calls were updated to use row_assignee. Tests were refactored to rely on lazy HERMES_HOME resolution (removing sys.modules purging) and a new integration test verifies quarantine behavior for auto-assigned tasks. ChangesDispatcher Fix and Test Hermetic Isolation
Sequence Diagram(s)(omitted — changes are small and already summarized above) Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🔎 Lint report:
|
sahilm-ti
force-pushed
the
kanban/t_259adc02
branch
from
June 2, 2026 17:45
c118b49 to
3c3ce6d
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@hermes_cli/kanban_db.py`:
- Around line 8321-8327: The crash-breaker path still reads the stale snapshot
row["assignee"] for quarantining and telemetry; replace those usages with the
effective assignee variable row_assignee so the same resolved fallback is used
everywhere: change calls to is_assignee_quarantined(conn, row["assignee"]) to
is_assignee_quarantined(conn, row_assignee), replace
quarantine_blocked.append(..., row["assignee"]) to use row_assignee, and pass
row_assignee into mark_quarantine_probe_task(..., row["assignee"], ...) so
quarantine checks and probe/blocked telemetry reflect the actual assignee; keep
profile_exists(row_assignee) as-is.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: e1cc8c4c-1eda-4da9-9e98-50155bac76c3
📥 Commits
Reviewing files that changed from the base of the PR and between 62a2d5886d1b0dd0b76d3589066dd560dd03df3f and 3c3ce6da405c723497bfaee8e3ad1c754137b605.
📒 Files selected for processing (4)
hermes_cli/kanban_db.pytests/hermes_cli/test_kanban_cli_dispatch_passthrough.pytests/hermes_cli/test_kanban_default_assignee.pytests/hermes_cli/test_kanban_per_profile_cap.py
Comment on lines
+8321
to
+8327
| # Check the EFFECTIVE assignee (row_assignee), not the stale DB | ||
| # snapshot (row["assignee"]). After default_assignee auto-assignment | ||
| # row_assignee holds the fallback profile while row["assignee"] is | ||
| # still None — using the snapshot here would route every | ||
| # auto-assigned task into skipped_nonspawnable and never spawn it | ||
| # (#27145 regression reintroduced by #34). | ||
| if profile_exists is not None and not profile_exists(row_assignee): |
There was a problem hiding this comment.
Use the effective assignee for the rest of the dispatch path too.
This fixes the profile_exists(...) gate, but the same stale-snapshot problem still exists a few lines below in the crash-breaker path: is_assignee_quarantined(conn, row["assignee"]), quarantine_blocked.append(..., row["assignee"]), and mark_quarantine_probe_task(..., row["assignee"], ...) still read row["assignee"]. For tasks auto-assigned this tick, that value is still None, so quarantined default assignees can bypass the breaker and the probe/blocked telemetry is wrong.
Suggested fix
- if crash_breaker_enabled and not dry_run:
- blocked_q, is_probe = is_assignee_quarantined(conn, row["assignee"])
+ if crash_breaker_enabled and not dry_run:
+ blocked_q, is_probe = is_assignee_quarantined(conn, row_assignee)
if blocked_q:
- result.quarantine_blocked.append((row["id"], row["assignee"]))
+ result.quarantine_blocked.append((row["id"], row_assignee))
continue
@@
if is_probe:
try:
- mark_quarantine_probe_task(conn, row["assignee"], claimed.id)
+ mark_quarantine_probe_task(
+ conn, claimed.assignee or row_assignee, claimed.id
+ )
except Exception:
pass
- result.quarantine_probes.append((claimed.id, row["assignee"]))
+ result.quarantine_probes.append(
+ (claimed.id, claimed.assignee or row_assignee)
+ )🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hermes_cli/kanban_db.py` around lines 8321 - 8327, The crash-breaker path
still reads the stale snapshot row["assignee"] for quarantining and telemetry;
replace those usages with the effective assignee variable row_assignee so the
same resolved fallback is used everywhere: change calls to
is_assignee_quarantined(conn, row["assignee"]) to is_assignee_quarantined(conn,
row_assignee), replace quarantine_blocked.append(..., row["assignee"]) to use
row_assignee, and pass row_assignee into mark_quarantine_probe_task(...,
row["assignee"], ...) so quarantine checks and probe/blocked telemetry reflect
the actual assignee; keep profile_exists(row_assignee) as-is.
Owner
|
auto-review: approved, awaiting human merge + kanban_approve. Rebase verified. 39 to 1 commit ahead; 4 files (+42/-18); MERGEABLE. The core Two red CI shards are pre-existing flakes, NOT caused by this diff:
All blocking gates green (ruff enforcement, ty diff, nix ubuntu+macos, check-attribution, check-common-ancestor, e2e, Windows footguns, supply-chain, CodeRabbit). Safe to merge; the UNSTABLE mergeStateStatus is from unrelated infra flakes, not this PR. |
sahilm-ti
force-pushed
the
kanban/t_259adc02
branch
from
June 3, 2026 15:34
3c3ce6d to
a271d80
Compare
sahilm-ai
added a commit
that referenced
this pull request
Jun 3, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
…e spawn regression Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order.
sahilm-ti
force-pushed
the
kanban/t_259adc02
branch
from
June 3, 2026 17:49
a271d80 to
4081853
Compare
Owner
|
auto-review: approved, awaiting human merge + kanban_approve. Matrix checks (U1–U5, C1–C6): all pass. In-scope (kanban_db.py + 4 kanban test files), no deletions, no secrets, mergeStateStatus CLEAN, all CI shards green, no new type-ignore/cast, regression test added, commit identity sahilm-ai. Code-quality (role-reviewer): APPROVED, no violations. The 4-line quarantine-path change routes is_assignee_quarantined / quarantine_blocked / mark_quarantine_probe_task / quarantine_probes through the effective row_assignee (in scope at 8402–8425, defaulted at 8324) — consistent with the spawn gate. New test asserts at the dispatch_once public boundary that a default_assignee auto-routed task is blocked under its effective assignee, not spawned. |
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 5, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 5, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 15, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 15, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 17, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 17, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 22, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jun 22, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 3, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 3, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 13, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 15, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 15, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 17, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 17, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 21, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 21, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 23, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 23, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 28, 2026
…artbeat enforcement (#46) test_dispatch_once_stale_disabled_when_timeout_zero stored os.getpid() as worker_pid with a 5h-old started_at and no heartbeat, then called dispatch_once(stale_timeout_seconds=0). dispatch_once runs enforce_missing_heartbeat independently of stale_timeout_seconds, which os.kill(SIGTERM)'d the stored PID = the pytest process itself, killing pytest before it printed its summary (raw RC=143). The parallel harness then scraped 0 passed/0 failed and bucketed the file as 'no tests ran', turning test(4) red on #43/#44/#45 — broken-main from the upstream rebase. Fix: set a recent last_heartbeat_at on the run so enforce_missing_heartbeat skips the task. This test isolates STALE detection, not heartbeat enforcement. Also hardened test_enforce_max_runtime_integrates_with_dispatch (same os.getpid() footgun on its real-os.kill dispatch_once call). Test-only change. Verified: raw pytest RC=0, 166 passed 1 skipped. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Jul 28, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Aug 24, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
sahilm-ti
pushed a commit
that referenced
this pull request
Sep 2, 2026
…e spawn regression (#43) Two distinct bugs surfaced as the test(4) CI shard failing on every PR: 1. Real source regression (not pollution): dispatch_once checked profile_exists(row["assignee"]) — the stale DB snapshot, still None after default_assignee auto-assignment — instead of the effective row_assignee. Every auto-assigned task fell into skipped_nonspawnable and never spawned. #34 (3146a5e) reverted the correct NousResearch#27145 fix. Fixes test_unassigned_task_auto_assigned_with_default_assignee and test_dry_run_with_default_assignee_reports_without_mutating (these fail standalone, proving it's a logic bug, not ordering). 2. Non-hermetic fixtures: three kanban test files (default_assignee, cli_dispatch_passthrough, per_profile_cap) did del sys.modules[...] + reimport in their isolation fixture. kanban_db resolves HERMES_HOME lazily at call time, so the reimport was unnecessary — and it created a SECOND hermes_cli.kanban_db module object. Sibling files that captured the module at import time (test_kanban_core_functionality's top-level 'import ... as kb') then read process-global state (_recent_worker_exits) from the OLD module while 'import ... as _kb' inside the test resolved the NEW one. This split flipped test_detect_crashed_workers_protocol_violation_auto_blocks depending on file order. Removing the reimport restores hermeticity. Verified: all tests/hermes_cli/test_kanban_*.py pass per-file (CI model) and the previously-polluting in-process combinations now pass in any order. Co-authored-by: Sahil (AI) <266772320+sahilm-ai@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
CI
test (4)shard failed on every PR (confirmed on #41, #42) with deterministic failures intests/hermes_cli/test_kanban_default_assignee.py. Two independent root causes, both surfacing as order/shard-dependent failures.Root cause 1 — real source regression (
dispatch_once)dispatch_oncecheckedprofile_exists(row["assignee"])— the stale DB snapshot, stillNoneafterdefault_assigneeauto-assignment — instead of the effectiverow_assignee. Every auto-assigned task therefore fell intoskipped_nonspawnableand never spawned.This was the correct
row_assigneeform in NousResearch#27145, accidentally reverted torow["assignee"]by #34 (3146a5e81). The twodefault_assigneetests fail standalone, proving this is a logic bug, not test ordering. Fix: checkrow_assignee.Root cause 2 — non-hermetic fixtures (
del sys.modulesreimport)Three kanban test files (
test_kanban_default_assignee.py,test_kanban_cli_dispatch_passthrough.py,test_kanban_per_profile_cap.py) diddel sys.modules[...]+ reimport in their isolation fixture.kanban_dbresolvesHERMES_HOMElazily at call time (viakanban_home()→get_default_hermes_root()), so the reimport was unnecessary — and harmful. It created a secondhermes_cli.kanban_dbmodule object. Sibling files that captured the module at import time (e.g.test_kanban_core_functionality.py's top-levelimport ... as kb) then read process-global state (_recent_worker_exits) from the OLD module, while an in-testimport ... as _kbresolved the NEW one. That split flippedtest_detect_crashed_workers_protocol_violation_auto_blocks(_record_worker_exitwrote one global,detect_crashed_workersread the other) depending on test/shard order.Fix: drop the reimport in all three fixtures. The fresh
HERMES_HOMEmonkeypatch is sufficient.Verification
tests/hermes_cli/test_kanban_*.pyfile passes per-file (CI'srun_tests_parallel.pymodel).core_functionality) now pass in any order.ruff check .green.Assertions were left untouched — they encode the real dispatcher contract; only the isolation and the source bug were fixed.
skip-review: false
Summary by CodeRabbit
Bug Fixes
Tests