Skip to content

fix(pull): close the pilot-scoped gaps left by #2391 — schedule context, RETRY-001 stacking, _dispatch_and_record_outcome, breaker test #2514

Description

@vybe

Follow-up from the pre-merge structural review of #2513 (feat(pull): let scheduled work reach the durable queue, #2391). None of these blocked the merge: every one is scoped to agents listed in PULL_MODE_PILOT_AGENTS (unset by default), and none can corrupt state on the push path. They should land before the pull pilot is widened past a soak agent.

1. Pulled cron turns lose their schedule context

build_pull_queue_payload (src/backend/services/task_execution_service.py) carries no schedule_context, PersistentTaskPayload / backlog_metadata have no field for it, and pull_coordination_service._compose_pull_system_prompt builds ExecutionContext without schedule_name / schedule_cron / schedule_next_run — which the push path injects (task_execution_service.py ~L1647). A pulled scheduled run therefore executes without the schedule identity in the platform prompt (#171), i.e. the #2317 "pulled turn silently ran on defaults" class. The #2391 e2e test asserts model/tools/timeout/session but not this.

2. RETRY-001 retries stack on, and defeat, the pull recovery story

_execute_retry (src/scheduler/service.py ~L1777) creates a NEW row with triggered_by="retry", which is not in _AUTONOMOUS_TRIGGERS, so pull_owns_dispatch is False and the retry is pushed — a slot is ZADDed on a pilot agent, breaking the #1766 pull-exclusive property on exactly the failure path. Worse, a lease-reaper poison-parked row (FAILED with the poison_lease tag, parked precisely so it is NOT re-run, #1402) is a retry candidate: _maybe_schedule_retry (~L1619) skips only SKIPPED/CANCELLED, so with max_retries > 0 the scheduler re-runs what the reaper parked. retry rows are also absent from the M1 tables in docs/testing/PULL_MIGRATION_TESTING.md and from note_unreachable_pull_trigger (non-autonomous ⇒ silent).

3. _dispatch_and_record_outcome was not given the same fix as its sibling

#2513 widened _poll_execution_completion and _poll_and_finalize to _NON_TERMINAL_POLL_STATES but left _dispatch_and_record_outcome (src/scheduler/service.py ~L1217) at != ExecutionStatus.RUNNING. On a dispatch timeout after the backend's background task already enqueued, a queued row reads as "already finalized": the scheduler publishes schedule_execution_completed(status='queued') and never spawns the poll task, so the worker's real terminal is never reported and retry/validation hooks are lost. Safe direction (no FAILED write), but the else-branch write just below is still the pre-existing non-CAS writer (#1022 D4) and on the pull path it can flip a just-queued row to FAILED before a claim.

4. No test for the breaker on the pull path

No #2391 test exercises breaker_enabled=True when the pull payload is set: the producer harness mocks acquire and patches dispatch_breaker_active=False, and the e2e class never enables it. The no-enqueue rule (D2) holds only because CircuitOpen is raised in capacity_manager.acquire before the pull_exclusive branch — unchanged code, but unasserted for this producer.

Also observed (documented, not defects)

  • Pulled scheduled runs create no agent_activities dispatch row (execute_task exits before step 3; the claim path creates none), so the Timeline shows nothing for them. Pre-existing pull property, now covering cron.
  • Scheduler post-completion hooks (WS event, VALIDATE-001, RETRY-001) are lost whenever queue-wait + run exceeds timeout + 60s; on a busy cron pilot this will be routine. The soak should count it.
  • _execute_reminder's stale-firing reclaim keys on == RUNNING; a queued row is not FAILed but the reminder is still re-armed (duplicate delivery in the crash-mid-fire window — the same at-least-once envelope as push).

Parent: #1081. Surfaced by /validate-pr + structural review of #2513.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    complexity-mediumComplexity: medium (board points 5-8)priority-p2Importantstatus-readyGreenlit and ready for development (vetted; counterpart to status-incubating)theme-reliabilityTheme: Reliabilitytype-bugBug fix

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions