fix(agent org): improving agent org efficiency and fixing edge cases

[ShiboSheng]

Background

PR #257 (fix/issue-159-wake-unassigned-tasks, commit 051f7d1) introduced the Agent Org
watchdog (agent_org_watchdog.rs), the eligible_member_ids claim whitelist, and the
member-failure requeue flow. A full review confirmed the design direction is correct and the
PR is a net improvement over baseline (no watchdog at all), but it left a set of known gaps.
This issue is the single tracking point for all of them.

---

🔴 E1 — Budget-exhausted unowned tasks stall silently (safety-net hole)

delayed_rewake_allowed (agent_org_watchdog.rs:52) caps rewakes of failed members at 3
attempts (1/5/15 min backoff). The budget is only cleared on a successful member turn
(lifecycle.rs clear_rewake_budget call).

Problem: for an unowned pending task whose eligible members are all Failed with an
exhausted budget, inspect_stalled_run produces:

• eligible_wake_members = empty (budget blocks them), and
• needs_repair = empty (that list only collects owned tasks with terminal owners and
  unowned tasks without an eligibility list — agent_org_watchdog.rs:152-217),

so the run is classified NotStalled forever. The coordinator is never told. A broken
provider/API key reproduces this within ~21 minutes.

Fix: when an unowned claimable task ends up with zero wakeable eligible members, push a
needs_repair entry (escalate to coordinator) instead of returning silence.

🔴 E2 — Unread-inbox gate blocks exactly the recovery the watchdog exists for

agent_org_watchdog.rs:219: HasEligibleClaimableWork is only returned if none of the
candidate members has an unread inbox row (has_any_unread_inbox). Two defects:

1. Wrong direction. "Idle + unread inbox" is the canonical missed wake state (wake
   tokio::spawn dropped at shutdown, app crash between insert and wake, etc.). The rest of
   the system already treats it as a first-class wake reason
   (AgentOrgWakeReason::UnreadInbox, org_tasks.rs:586). The watchdog — the safety net for
   missed wakes — is the one place that refuses to wake these members.
2. Collective punishment. One member with an unread row suppresses the wake batch for
   all eligible members.

Waking a member that has unread mail is idempotent and safe: should_dispatch_wake
(inbox_wake.rs:70) blocks in-flight sessions, and DrainGuard marks rows read
transactionally.

Fix: evaluate per member; treat idle+unread as a wake reason (reuse the
AgentOrgWakeReason semantics), do not gate the whole batch on one member's inbox.

🟡 E4 — Failure path wakes peers that cannot claim (pure token burn)

On member failure, finalize_agent_org_member_turn:

• requeues in-progress tasks via requeue_in_progress_for_owner (store.rs:581), which sets
  status=pending but keeps owner, then
• wakes every eligible_member_ids entry of each requeued task (lifecycle.rs:390).

But find_available_for_member (store.rs:277) and try_claim both require owner IS NULL,
so the woken peers find nothing claimable and burn one full LLM turn each (an empty wake
still reaches the provider — processor/mod.rs:734 nudge path). M eligible members ⇒ M−1
wasted LLM calls per failure.

Fix (decided: option b, self-healing): on failure, if the task's eligibility list
contains members other than the failed owner, release ownership (unassign_for_owner
semantics: owner=NULL, metadata/eligibility preserved) so woken peers can actually claim
it. Keep the owner (current behavior, coordinator repairs) only when the failed member is
the sole eligible member or the list is empty. The failed member remains in the eligibility
list and can reclaim after recovery, rate-limited by the existing rewake budget.

🟡 E5 — Coordinator repair notices have no budget

Failed-member rewakes are budgeted (3 attempts), but NeedsCoordinatorRepair
(agent_org_watchdog.rs:117) is not: as long as needs_repair is non-empty and the
coordinator has read the previous notice, a new stall notice + coordinator wake fires every
tick (60s). A coordinator that cannot repair (or is itself misbehaving) loops LLM turns
indefinitely. The only current damper is the coordinator-unread gate.

Fix: per-run notice budget keyed on a hash of the reason payload with the same
1/5/15-min backoff; reset when the reason content changes or the run status transitions
(a successful repair changes task state ⇒ reason changes ⇒ auto-reset).

🟡 E7 — O(T²·M) scan cost per watchdog tick

inspect_stalled_run calls AgentOrgTaskStore::find_available_for_member for every
(task × eligible member) pair (agent_org_watchdog.rs:203), and that function does a full
AgentOrgTaskStore::list(run_id) on every call because the eligibility whitelist lives in
metadata JSON and cannot be filtered in SQL (store.rs:277-296). Per tick per stalled run:
T × M full-table loads of T rows ⇒ O(T²·M) row scans. collect_run_progress_wake_targets
(org_tasks.rs:609) has the same per-member pattern (O(T·M)).

Fine at desktop scale (T≈20), hot at T≈200.

Fix (decided scope):

1. Single-pass set computation (do now): add a pure
   claimable_member_ids(tasks: &[Task]) -> HashSet<String> in agent_org_tasks — one pass
   collects completed-task ids, second pass collects eligible members of
   pending/unowned/unblocked tasks. Watchdog and progress-wake callers compute it once per
   run and do O(1) set lookups. Per-tick cost drops to one task-list load + in-memory work.
2. SQL-side JSON filter (do now, one query): switch find_available_for_member's query
   to filter via json_each(metadata,'$.eligible_member_ids') so the single-member claim
   path (inbox_drain) stops loading full lists.
3. Schema normalization (DEFERRED — recorded as debt, do NOT implement now): move the
   whitelist to an agent_org_task_eligibility(org_run_id, task_id, member_id) join table
   with an (org_run_id, member_id) index + backfill migration. Only if real-world scale
   demands it.

🟢 E6 — Small debt items

Item	Location	Fix
REWAKE_BUDGETS never pruned for finished runs (unbounded static HashMap growth)	agent_org_watchdog.rs:33	prune entries for non-Running runs during each tick (run list is already loaded)
Two independent 15-min stale thresholds with different semantics (auto-release vs. coordinator escalation)	STALE_IN_PROGRESS_MINUTES (watchdog.rs:20) vs STALE_WORKER_TASK_RELEASE_TIMEOUT_SECS (drain.rs:25)	share one constant; note in the stall notice that the task may already have been auto-released
Unparseable updated_at silently treated as not stale	agent_org_watchdog.rs:263 (is_stale_in_progress)	log warn + treat as stale (prefer false positive escalation over permanent silence)
list_runs(500) loads all runs then filters in Rust; >500 runs can hide an old Running run	agent_org_runs/store.rs:316 + watchdog.rs:96	add WHERE status='running'
Default MissedTickBehavior::Burst back-to-back ticks after a slow scan	agent_org_watchdog.rs:82	interval.set_missed_tick_behavior(Skip)
find_available is production-dead after #257 (only its own tests reference it)	store.rs:304	delete
has_unread_for_member loads full unread rows to test emptiness	watchdog.rs:286	EXISTS/COUNT query (minor)
Instant-based backoff does not advance across macOS sleep (windows stretch, direction is safe)	watchdog.rs:56	optional: switch to Utc timestamps; low priority

🟢 Maintainability

• inspect_stalled_run / recover_stalled_run have zero test coverage — the PR added
  300+ test lines but only for the two small helpers (is_wakeable_status, budget). The
  4-state decision core (the part with the E1/E2 bugs) is untested. Add fixtures per state,
  explicitly locking the E1 (budget-exhausted ⇒ NeedsCoordinatorRepair) and E2
  (idle+unread ⇒ wake) semantics.
• AgentOrgStallState decision priorities are implicit in early-return order; document them.

⚪ Known limitation (deliberately NOT fixed here) — E3

inspect_stalled_run returns NotStalled if any worker in the run is active
(watchdog.rs:136), so one long-running member masks stalls of every other member. Fixing it
means member-level stall detection (a rewrite of the watchdog's core premise). After E1/E2/E4
land, its practical impact is small (member failures already notify the coordinator
synchronously via the MemberIdle(failed) hook). Track separately if it bites in practice.

---

[chsimonpan]

While syncing our fork with latest develop (ac69bd1), we found that cargo test -p agent_core --lib currently fails on a clean checkout of official develop — 5 failures, 4 of which trace back to the eligible_member_ids claim-whitelist changes from #257 that this issue tracks:

test core::session::turn::processor::inbox_drain::tests::autonomous_claim_picks_up_unowned_task_for_idle_member ... FAILED
test core::session::turn::processor::inbox_drain::tests::autonomous_claim_skips_when_member_already_busy ... FAILED
test core::session::turn::processor::inbox_drain::tests::drain_releases_member_tasks_on_accepted_shutdown ... FAILED
test core::session::turn::processor::inbox_drain::tests::drain_releases_stale_worker_tasks_before_autonomous_claim ... FAILED
test core::session::turn::entry::tests::skill_slash_command_accepts_newline_after_name ... FAILED

Representative panics:

thread '...::autonomous_claim_skips_when_member_already_busy' panicked at
crates/agent-core/src/core/session/turn/processor/inbox_drain/mod.rs:1501:14:
called `Result::unwrap()` on an `Err` value: NotEligible

thread '...::autonomous_claim_picks_up_unowned_task_for_idle_member' panicked at
crates/agent-core/src/core/session/turn/processor/inbox_drain/mod.rs:1451:9:
assertion `left == right` failed: self-claim row should be the only injected row
  left: 0
 right: 1

The test fixtures create ownerless tasks without populating eligible_member_ids, so after the hard-whitelist change every autonomous claim path now returns NotEligible — consistent with the E1/E2 concerns described above. The 5th failure (skill_slash_command_accepts_newline_after_name, entry.rs:113) looks unrelated: the bundled e2e-testing skill content doesn't match the expected prefix.

Environment: Linux x86_64, stable toolchain, cargo test -p agent_core --lib at ac69bd1.

It may be worth a CI gate on cargo test -p agent_core --lib for develop pushes so fixture drift like this gets caught at merge time. Happy to help with either the fixture updates or the CI wiring.

[ShiboSheng]

Hi @chsimonpan — thanks for the detailed report; really appreciate you tracing the panics back to the root cause instead of just flagging the surface failures.

Confirmed on our end. We hit the same 5 failures when auditing develop at ca9bb57 (a few commits ahead of your ac69bd1 base, but same root cause). The 4 inbox_drain failures are exactly the fixture drift you described: those tests still create ownerless tasks without eligible_member_ids, so the hard whitelist gate from #257 (ClaimError::NotEligible) rejects every autonomous-claim path they exercise. The 5th (skill_slash_command_accepts_newline_after_name, entry.rs:113) is indeed unrelated — it's a bundled e2e-testing skill-content mismatch that we also reproduce on a clean origin/develop checkout and which predates the #257 work.

Fix for the 4 fixture failures is already staged. This issue (#272) tracks all of the #257 follow-up work, and the branch fix/issue-272-agent-org-watchdog-hardening carries a dedicated commit (37bfc0de, test(agent-org): repair inbox_drain fixtures for eligibility gating) that gives each affected fixture an explicit eligible_member_ids matching its claimant. cargo test -p agent_core inbox_drain goes from 26/30 to 30/30 on that branch. The PR will be opened against develop shortly and will reference Fixes #272, so once it lands a develop pull will resolve all 4 for your fork too.

On the CI gate. Strong +1 — a cargo test -p agent_core --lib step on develop pushes (and ideally on PRs targeting develop) would have caught this drift at #257's merge time rather than letting it surface via a downstream sync. That's a separate concern from the #272 fix scope, though. If you'd like to pursue it, opening a dedicated issue to track the CI hardening would be the right next step so it can be triaged on its own merits; keeping this issue scoped to the Agent Org watchdog/eligibility work.

On the skill_slash_command_accepts_newline_after_name failure. Worth tracking separately since it's a different subsystem. If you haven't already, opening a small issue with the expected vs. actual skill prefix would help whoever owns that area pick it up. We'll keep this issue scoped to the Agent Org watchdog/eligibility work.

Thanks again for the reproduce details and the env info — made verification painless.