Skip to content

approval-queue: an expired action can never be re-staged — the unique index is not status-partial (absorbing state) #9481

Description

@JSONbored

Summary

sweepStaleApprovalQueue (#9032) expires a pending approval after 7 days — but the uniqueness constraint it collides with is not status-partial, and the staging path uses onConflictDoNothing. So an expired row blocks its own replacement forever: that PR + action class can never be staged for approval again. The module's own documentation promises the opposite.

This is strictly worse than the pre-#9032 behaviour, where rows waited indefinitely but stayed acceptable.

Mechanism (verified at HEAD, 776c414)

The promisesrc/services/agent-approval-queue.ts:566-570 (and agent-approval-staleness.ts:17-19):

a later pass that re-plans the same action stages a fresh row with a fresh notification

The constraintmigrations/0045_agent_pending_actions.sql:20:

CREATE UNIQUE INDEX IF NOT EXISTS agent_pending_actions_target_unique
  ON agent_pending_actions (repo_full_name, pull_number, action_class);

No WHERE status = 'pending'. It covers every status, including expired.

The collisioncreatePendingAgentActionIfAbsent (src/db/repositories.ts:6511-6543) uses onConflictDoNothing against that index. The expired row conflicts ⇒ created: falsestageForApproval (src/services/agent-action-executor.ts:1353-1363) returns before notifying. Meanwhile decidePendingAgentAction (agent-approval-queue.ts:50) returns already_decided for any non-pending row, and nothing anywhere deletes or reopens expired rows (the only delete in the codebase is src/db/repo-identity-rename.ts:419).

Net effect: any auto_with_approval merge or close whose maintainer was away for 7 days becomes permanently unexecutable through the queue for that PR and action class.

Compounding: the audit trail actively lies

Executor step 4 (src/services/agent-action-executor.ts:439-442) still audits "queued — awaiting maintainer approval" on every subsequent sweep, even when the staging silently no-op'd. An operator reading the audit log sees a queued action that does not exist.

Adjacent starvation risk

listPendingAgentActions orders by createdAt DESC (src/db/repositories.ts ~6350) and the sweep caps at 500 newest rows. Above 500 pending fleet-wide, the oldest rows — exactly the ones due to expire or be reminded — are the ones never examined.

Deliverables

  • Make re-staging work. Either: make the unique index partial (WHERE status = 'pending') via a migration, or on conflict conditionally UPDATE a row whose status is expired back to pending with a fresh createdAt. State which and why.
  • Make the step-4 "queued — awaiting maintainer approval" audit conditional on created, so the trail cannot claim a staging that did not happen.
  • Change the sweep's selection to oldest-first (or make it unbounded with a cursor), so the rows nearest expiry are the ones processed.
  • Sweep existing expired rows on edge-nl-01 after the fix so currently-stuck PR+class pairs are recoverable.

Tests (must fail against current main)

  • Stage → expire → re-plan the same action ⇒ a fresh pending row exists and a notification is sent.
  • The step-4 audit is not written when staging no-op'd.
  • With >500 pending rows, the oldest are still examined by the sweep.
  • Accept/reject on a re-staged row behaves normally.

Expected outcome

Approval expiry becomes what it was designed to be — a prompt to re-ask — instead of an absorbing state that permanently disables an action class for a PR.

Metadata

Metadata

Assignees

Labels

gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.maintainer-onlyOwner-only work — yields no Gittensor points.orbGittensory Orb related - maintainer self-hosting analytics.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions