Skip to content

feat(delegations): stuck-task sweeper with deadline + heartbeat-staleness rules (RFC #2829 PR-3) - #2836

Merged
HongmingWang-Rabbit merged 1 commit into
stagingfrom
feat/rfc2829-pr3-stuck-task-sweeper
May 5, 2026
Merged

feat(delegations): stuck-task sweeper with deadline + heartbeat-staleness rules (RFC #2829 PR-3)#2836
HongmingWang-Rabbit merged 1 commit into
stagingfrom
feat/rfc2829-pr3-stuck-task-sweeper

Conversation

@HongmingWang-Rabbit

Copy link
Copy Markdown
Contributor

Third of 4 PRs implementing RFC #2829. Built on PR-1 (#2832, schema, merged).

What this PR adds

A periodic sweeper (DelegationSweeper) over the delegations table:

  1. Deadline-exceeded rows → marked failed ("deadline exceeded by sweeper")
  2. Heartbeat-stale rows (no beat for >10min default) → marked stuck

Why both rules

  • Deadline catches forever-heartbeating wedged agents (alive-but-not-advancing class — agent loops on heartbeat call inside its main loop).
  • Heartbeat-staleness catches OOM-killed and crashed agents that stop cold without graceful shutdown.

Either rule alone misses one of these classes.

Order matters

Deadline checked first. A deadline-exceeded AND stale row is marked failed (give up), not stuck (operator can retry). Semantic difference matters.

NULL heartbeat is a free pass

A delegation that's just been inserted but hasn't emitted its first heartbeat yet is NOT stuck-marked — gives the agent its first beat window. The deadline naturally catches true never-started rows.

Concurrent-completion safety

When sweep races with UpdateStatus on a delegation that just completed, the ledger's terminal forward-only protection (PR-1's ErrInvalidTransition) holds. Sweeper logs + counts in Errors; the row stays correctly in completed.

Configuration

  • DELEGATION_SWEEPER_INTERVAL_S — tick cadence (default 5min)
  • DELEGATION_STUCK_THRESHOLD_S — heartbeat-staleness threshold (default 10min)

Invalid input falls back to default — typo'd env shouldn't crash startup.

Wiring

NOT wired into main.go in this PR — ships separately so the sweeper can be enabled/disabled independently of binary upgrade.

Coverage

13 unit tests:

Sweep semantics (8):

  • empty in-flight set → clean no-op
  • deadline → failed
  • heartbeat-stale → stuck
  • NULL heartbeat is left alone (first-beat free pass)
  • healthy row → no-op
  • both-rule row → marked failed (deadline wins)
  • mixed set → both rules fire on the right rows
  • concurrent-completion race → forward-only protection holds

Env override parsing (5):

  • default on missing env
  • parses positive seconds
  • falls back on garbage
  • falls back on negative
  • constructor picks up overrides

Test plan

  • 13 unit tests pass
  • Existing delegation tests untouched
  • PR wiring sweeper into main.go (follow-up)
  • Staging E2E once sweeper is wired

Refs RFC #2829.

…ness rules (RFC #2829 PR-3)

Periodically scans the `delegations` table (PR-1 schema) for in-flight
rows that need terminal action:

  1. Deadline-exceeded → marked `failed` with "deadline exceeded by sweeper"
  2. Heartbeat-stale (no beat for >10× heartbeat interval) → marked `stuck`

## Why both rules

Deadline catches forever-heartbeating wedged agents (the alive-but-not-
advancing class — agent loops on heartbeat call inside its main loop).
Heartbeat-staleness catches OOM-killed and crashed agents that stop cold
without graceful shutdown. Either rule alone misses one of these classes.

## Order matters

Deadline is checked first. A deadline-exceeded AND stale row is marked
`failed` (operator action: investigate + give up), not `stuck` (operator
action: investigate + retry). The semantic difference matters.

## NULL heartbeat is a free pass

A delegation that's just been inserted but hasn't emitted its first
heartbeat yet is NOT stuck-marked — gives the agent its first beat
window. Lets the deadline catch true never-started rows naturally.

## Concurrent-completion safety

Sweep races with UpdateStatus on a delegation that just completed: the
ledger's terminal forward-only protection (PR-1) returns ErrInvalidTransition,
sweeper logs + counts in Errors, the row stays correctly in completed.

## Configuration

  - DELEGATION_SWEEPER_INTERVAL_S — tick cadence (default 5min)
  - DELEGATION_STUCK_THRESHOLD_S  — heartbeat-staleness threshold (default 10min)

Both fall back gracefully on invalid input (typo'd env shouldn't crash
startup). Both read at construction time so a long-running process
picks up overrides via restart.

## Wiring

NOT wired into main.go in this PR — that ships separately so the
sweeper can be enabled/disabled independently of the binary upgrade.
The sweeper is a standalone Sweep(ctx) callable + Start(ctx) ticker
loop, both with panic recovery, both indexed-scan-cheap on the
partial idx_delegations_inflight_heartbeat from PR-1.

## Coverage

13 unit tests against sqlmock-backed *sql.DB:

Sweep semantics (8 tests):
  - empty in-flight set → clean no-op
  - deadline → failed
  - heartbeat-stale → stuck
  - NULL heartbeat is left alone (first-beat free pass)
  - healthy row → no-op
  - both-rule row → marked failed (deadline wins)
  - mixed set → both rules fire on the right rows
  - concurrent-completion race → forward-only protection holds

Env override parsing (5 tests):
  - default on missing env
  - parses positive seconds
  - falls back on garbage
  - falls back on negative
  - constructor picks up overrides; defaults when env unset

Refs RFC #2829.
@HongmingWang-Rabbit
HongmingWang-Rabbit added this pull request to the merge queue May 5, 2026
Merged via the queue into staging with commit b5c0b4d May 5, 2026
22 checks passed
@HongmingWang-Rabbit
HongmingWang-Rabbit deleted the feat/rfc2829-pr3-stuck-task-sweeper branch May 5, 2026 04:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant