Skip to content

fix(cron): treat launches from a pruned live install as retryable skips - #9038

Open
rubencu wants to merge 2 commits into
kirodotdev:mainfrom
rubencu:fix/cron-stale-runtime
Open

fix(cron): treat launches from a pruned live install as retryable skips#9038
rubencu wants to merge 2 commits into
kirodotdev:mainfrom
rubencu:fix/cron-stale-runtime

Conversation

@rubencu

@rubencu rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Problem / Motivation

When a managed-install auto-update promotes a new version, the updater unlinks the old version's entire tree — including its Python interpreter — while the old gateway is still running from mapped memory. Every cron child launch from that drained gateway then fails with FileNotFoundError: [Errno 2] No such file or directory: '<install>/python3.12/bin/python3.12', logged as a hard cron ERROR and consuming an auto-pause failure strike against the job.

Observed twice in the wild on the same host (two consecutive updates): the failing jobs were healthy, the runtime under them had simply been replaced. The strikes risk auto-pausing healthy jobs for an environmental race that resolves itself at the next gateway restart.

Why it matters

Cron jobs are the unattended backbone — a monitoring job that gets auto-paused because its runtime vanished mid-update silently stops watching. The update race is guaranteed to recur on every auto-update that lands while a gateway is live.

What changed (motivation → approach → change)

  • Added _running_install_was_pruned(): detects the handoff by requiring BOTH sys.executable and the module file itself to be absent — distinguishing a replaced install from an unrelated missing working directory, launcher, or user binary, which must remain real failures.
  • Script and agent cron launches that raise FileNotFoundError while the running install is pruned are booked as never-started via a shared _record_pruned_launch_skip() helper: last_status = "error" keeps the scheduler from recording a success, run_never_started = True is the retention marker that stops _merge_job_result deleting a due one-shot delete_after_run job, and record_failure() is deliberately not called so no auto-pause strike is spent. The replacement gateway launches a version-consistent child on its next tick.
  • Unrelated FileNotFoundError failures (bad cwd, missing user script interpreter) keep their existing hard-failure semantics.

Tests

  • test/test_cron_gateway_integration.py: coverage for the pruned-runtime never-started contract (script + agent + agent-sequence paths), both-paths-absent detector semantics, strike preservation for unrelated FileNotFoundError, reaper state cleanup, and a dedicated one-shot delete_after_run retention regression (test_pruned_install_retains_a_due_one_shot).
  • Behavioral mutation proofs: forcing the detector to the pre-fix False fails test_pruned_install_requires_both_runtime_paths_to_be_absent; removing the never-started bookkeeping fails 4 retention-contract tests.
  • Neighbors green: test_cron_script_more_coverage.py + test_cron.py (334 total pass, including the owed-fire persistence and cancellation regressions). flake8/isort/mypy clean on touched files.

Manual verification

Reproduced the race on a live host across two real auto-updates; confirmed the failing launches match the exact guarded path (interpreter file gone, module tree gone, gateway still serving).

Screenshots / video

N/A (backend behavior).

Related Issues

None open; failure class first captured 2026-09-04, recurred 2026-09-06.

Pattern harvest

Rule candidate: when a scheduled task is skipped for an environmental handoff (not a task defect), it must be booked as never-started — a bare skip that reads as success advances scheduler state and can irrecoverably delete due one-shot jobs.

Also: a live-updated daemon must treat "my own install vanished" as an environment handoff, not a task failure; requiring two independent paths to be absent avoids false positives from single missing files.

Checklist

  • Tests added/updated
  • Lint/type gates pass
  • Two focused commits (base fix + review-finding fix)

Contribution License Agreement

I confirm my contribution is made under the project's contribution license terms.

@rubencu
rubencu requested a review from a team as a code owner September 6, 2026 13:58
@rubencu
rubencu requested a review from patrigao September 6, 2026 13:58
@github-actions github-actions Bot added fork Pull request from a fork (external contributor) readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of ab756251916242688dc947a150587e04176fc09c via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

The no-strike skip is right, but it tolerates an updater invariant violation with a disproportionate exactly-once debt engine bolted into the scheduler.

Watch

  • The stated cause — "the updater unlinks the old version's entire tree" — is exactly what wheel_engine.py forbids ("No tree a live process might be using is ever moved or deleted"; pruning is deliberately absent pending a liveness protocol). The root cause is an updater-side invariant breach (or an out-of-contract external update), and this PR hardens only cron: agent chat, subagents, and every other child-spawning surface on the same drained gateway still hard-fails. Chase the updater; otherwise each subsystem grows its own pruned-install patch.
  • The owed_fire machinery (persisted field, two-direction _pending_owed_fires drains, dispatch-confirmation hooks, cancellation-window semantics) buys exactly-once for one cron-expression occurrence in the handoff window, while the same occurrence is still silently lost in the far commoner case — the gateway simply being down across the matching minute during the restart itself. High permanent scheduler complexity for a sliver of the missed-occurrence space; the debt also has no expiry, so one persisted before a long pause fires arbitrarily later at resume.
  • "The replacement gateway owns the next wake" is assumed, never enforced: the quiesce is process-lifetime with no escalation, so if no supervisor respawns the gateway, cron on that host is silently dead until a manual restart.

Suggestions

  • Simpler, more general shape: on detecting the prune, gracefully self-terminate and let the service manager (KeepAlive/systemd) respawn the promoted version — fixes every subsystem and reduces the owed-debt machinery to the ordinary restart path.
  • The ~700-line test_cron.py reformat/baseline removal buries the scheduler change; land baseline burn-down separately.

[DESIGN-REVIEWED] ab75625

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

GPT 5.6 Review (fork) — ✅ no blocking findings

Reviewed ab756251916242688dc947a150587e04176fc09c via the fork AI-review pipeline; updated in place on each push.

Review details

No findings.
[GPT-REVIEWED] ab75625

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed ab756251916242688dc947a150587e04176fc09c via the fork AI-review pipeline; updated in place on each push.

Review details

The working tree sits at the base commit, so the PR changes live only in the diff — I've validated against it.

Candidate 1 is a real logic error in an added line: inside for agent in agents:, the interrupted-sequence path builds last_error from agents[agents.index(agent) - 1], and list.index() returns the first occurrence. A duplicate agent name is a valid, unvalidated configuration (no dedup in add_job/update_job/_job_from_record). It's a misleading operator-facing string only — advisory, not blocking.

Candidate 2 dies under falsification: its outcome is stated as "can make the containment test disagree" and requires assuming a symlinked/.. install layout that neither the diff nor the code establishes. (c) never resolves past "could", so it does not clear the bar.

No blocking issues; one advisory finding.

FINDING — src/kiro_crew/slack/gateway.py:4645 — the interrupted-sequence error uses f"'{agents[agents.index(agent) - 1]}' completed", but list.index() returns the FIRST match, so a repeated agent name (e.g. ["research","write","research"] pruned on the 3rd element) makes index return 0 and names agents[-1] ("research") as the predecessor instead of the real one ("write") in the operator-visible last_error → Fix: iterate with for idx, agent in enumerate(agents) and use agents[idx - 1].

[OPUS-REVIEWED] ab75625

@github-actions

github-actions Bot commented Sep 6, 2026

Copy link
Copy Markdown
Contributor

First Principles Review (Fable 5, fork) — 🟡 CONCERNS

Premise-level review of ab756251916242688dc947a150587e04176fc09c via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: CONCERNS

The strike/one-shot fix is real, but half the diff is an undeclared make-up-run machine riding along, while the nameable cause — a drained gateway the repo already knows how to restart — stays unfixed.

What this change ships

Intent: stop auto-update install pruning from spending auto-pause strikes on (and deleting one-shots of) healthy cron jobs — a FIX.

  1. Pruned-install script-launch ENOENT costs no auto-pause strike — justified
  2. Same skip for agent-cron launches (single + sequence) — justified
  3. A due delete_after_run one-shot survives the skip — justified
  4. Affected jobs never fire again on the drained process (process-lifetime quiesce) — justified, symptom-level
  5. Missed cron-expression occurrence re-runs once on the next gateway (owed_fire, new persisted store key) — rides along, undeclared in "What changed"
  6. Mid-sequence pruning books a strike instead of a skip — undeclared, justified
  7. Manual triggers persist no make-up debt (run_is_manual) — one consumer, owed-machinery only
  8. Unreadable-store merge now queues the one-shot's removal — pre-existing-bug rider, undeclared
  9. Cancellation of an owed run restores/keeps debt via run_dispatched closures — owed-machinery only
  10. test_cron.py fully reformatted, dropped from black baseline (~30 cosmetic hunks) — rides along

Watch

  • Symptom level, counted siblings. The guard covers 2 of 20 real sessions.get_or_create call sites (grepped sessions\.get_or_create); chat_runner, slack/handler, channel, telegram, workflows, subagent_manager and apps hit the identical pruned-install ENOENT unguarded. The description's "The replacement gateway launches a version-consistent child on its next tick" names a process nothing creates: the quiesce is "never cleared — this process can never launch again", so absent a human restart the monitoring job still "silently stops watching." The cause-level mechanism exists in-tree and is used on the self-applied-update path: _restart_after_update + respawn_executable (wheel_engine.py:334), pre-imported precisely because "the apply may have deleted the venv this process imports from" (gateway.py:9458-9464).
  • owed_fire contradicts a recorded position and is partial. cron.py:3544-3549 already accepts cron-expression occurrence loss ("persisted deferral markers are a possible follow-up"). This marker recovers only the FIRST drain-window occurrence — every later one is quiesced un-owed — at the permanent price of a persisted schema key, _pending_owed_fires + two drains + a stop() hook, owed_consumed, two dispatch-confirmation closures, and a cancellation matrix.

Subtractions

  • Drop the owed-fire machinery: owed_fire (field, serialization, string-"false" guard), _pending_owed_fires, _drain_pending_owed_fires_locked/_drain_owed_fires_with_lock, the stop() drain, run_dispatched/owed_consumed, both _confirm_dispatch_gate closures, and run_is_manual (1 consumer: _record_pruned_launch_skip). Keep detector + never-started + keep_overdue + quiesce; the one-occurrence delay is the loss cron.py:3546 already accepts.
  • Replace wait-for-a-replacement with the existing restart: on _running_install_was_pruned(), route into _restart_after_update(respawn_executable) — this deletes _pruned_quiesced and its due-scan/wake checks and heals the 18 unguarded launch surfaces at once.

[FIRST-PRINCIPLES-REVIEWED] ab75625

@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Addressed the GPT 5.6 blocking finding (F1, pruned-runtime skips consuming scheduled executions) in 9cd287e: all three pruned-launch sites (script cron, single-agent, agent-sequence acquisition) now book through a shared _record_pruned_launch_skip() helper that mirrors the overlap-skip retention pattern — last_status = "error" keeps CronScheduler._execute from recording a success, run_never_started = True is the retention marker that stops _merge_job_result deleting a due one-shot delete_after_run job, and record_failure() is still not called so the update handoff costs no auto-pause strike. Regression tests updated to the never-started contract plus a dedicated test for the one-shot retention shape (test_pruned_install_retains_a_due_one_shot); verified the updated tests fail against the previous commit's behavior. 318 tests pass, lint/type gates clean.

@rubencu
rubencu force-pushed the fix/cron-stale-runtime branch from 9cd287e to 805fed0 Compare September 6, 2026 15:26
@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Round-2 findings addressed in 805fed0 (amended into the review-fix commit to stay within the two-commit gate):

F1 (at-job refire loop): _record_pruned_launch_skip now disables a pruned-skipped at-job IN MEMORY only. _merge_job_result's at-job enabled propagation is gated on not delete_after_run or fire_time_denied, so for the retained delete_after_run shape the flag never reaches disk — the drained gateway stops the zero-delay refire loop while the replacement gateway still sees an enabled job to retry. Regression: test_pruned_at_job_is_quiesced_in_memory + test_pruned_recurring_job_stays_enabled.

F2 (partial sequences): the sequence-loop acquisition now checks _prompt_dispatched; when a prior agent already ran, it records a normal failed run (strike + explanatory last_error, no retention marker) so the one-shot cannot replay completed side effects. Regression: test_mid_sequence_pruning_records_a_failure_not_never_started. The single-agent path acquires before any dispatch and its transient retry is gated on not _prompt_dispatched, so it needs no equivalent guard.

F3 (ENOENT scope): both handlers bind the exception and consult _enoent_names_this_install() — the excuse only applies when the missing path is inside this install's own tree (or exec-level ENOENT with no path); a deleted user script or provider binary stays a real failure. Regression: test_unrelated_missing_path_still_fails_even_when_pruned.

322 targeted tests pass; flake8/isort/mypy clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@rubencu
rubencu force-pushed the fix/cron-stale-runtime branch from 805fed0 to 39723b0 Compare September 6, 2026 16:21
@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Round-3 findings addressed in 39723b0:

F1 (never-started launches still consume their schedule): new CronJob.keep_overdue marker (in-memory only, reset at the start of every run, never merged). When set by _record_pruned_launch_skip, _execute returns before advancing last_run_ts and before the at-job fired/parked disable, and _merge_job_result skips its at-job enabled propagation — so on disk the job remains exactly as due/enabled as before the drained process touched it, and the replacement gateway retries immediately. The drained process itself is still quiesced by the in-memory enabled = False. This deliberately does NOT change run_never_started semantics for the overlap/starvation paths, where the tick was genuinely spent. Scheduler-level regressions: test_execute_keep_overdue_leaves_the_schedule_owed, test_execute_keep_overdue_does_not_park_a_plain_at_job.

F2 (pathless ENOENT): _enoent_names_this_install now rejects pathless ENOENT (a resolve_script_path()-style missing user script stays a real failure). On containment: limiting to Path(sys.prefix) alone would miss the package tree, which lives beside the prefix in versioned installs — so containment is the union of two concrete roots, sys.prefix and this module's own package tree (Path(__file__).resolve().parents[2]), never their common parent (which degrades to / on non-versioned layouts). Regression: test_pathless_enoent_is_a_real_failure_even_when_pruned.

Also brought the touched files to black formatting (the Backend Lint failure). The Backend Tests shard-4 test_snapshot.py failures are timing-sensitive tests untouched by this diff and pass locally (26/26); flagging as suspected base/host flake pending this head's re-run.

325 targeted tests pass; black/flake8/isort/mypy clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Round-4 finding addressed in 99431b4: the outer _run_job_isolated drift-correction (last_run_ts = started_at for 'every' jobs) is now guarded with not job.keep_overdue, so a pruned-install skip survives outer finalization and the replacement gateway retries the owed run immediately. Regression: test_run_job_isolated_keeps_a_pruned_every_job_overdue (mutation-verified: un-guarding the assignment fails it). The reaper/timeout stamp sites are untouched — they only apply to runs that genuinely dispatched, which a pruned launch never does. 326 targeted tests pass; black/flake8/isort/mypy clean.

@rubencu
rubencu force-pushed the fix/cron-stale-runtime branch from 39723b0 to 99431b4 Compare September 6, 2026 16:35
@github-actions github-actions Bot added readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running and removed readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention labels Sep 6, 2026
@rubencu
rubencu force-pushed the fix/cron-stale-runtime branch from 6c2b510 to 4623817 Compare September 6, 2026 21:07
@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Round-9 findings addressed in 4623817:

F1 (cancellation replays a dispatched owed run): correct — round 8's unconditional restore was the mirror error of the bug it fixed. The CancelledError handler now restores the debt ONLY when the callback had already recorded run_never_started (pre-dispatch refusals); a cancellation landing after dispatch keeps the debt consumed, erring toward no-replay exactly as the adjudication prescribed. Regressions: test_cancellation_during_the_callback_restores_the_debt (never-started path) and test_cancellation_after_dispatch_keeps_the_debt_consumed.

F2 (truthy string "false" on disk): deserialization now uses a strict identity check (j.get("owed_fire") is True). Regression: test_a_string_false_owed_fire_on_disk_loads_as_false (writes the corrupted store shape and asserts a fresh load yields False).

336 targeted tests pass; black/flake8/isort/mypy clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@rubencu
rubencu force-pushed the fix/cron-stale-runtime branch from 4623817 to 3c014db Compare September 6, 2026 22:05
@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Round-10 findings addressed in 3c014db:

F1 (swallowed store errors bypass owed-state recovery): confirmed — _sync degrades an unreadable store to an empty job list without raising, so _merge_job_result silently no-ops (job.id not in by_id) and returns normally, never reaching the caller's queueing hook. Fix respects the module's existing raise-reaches-user-mutations-only contract (a first attempt that raised CronStoreUnreadable from the merge correctly failed test_a_background_writer_degrades_instead_of_crashing): the merge now degrades as before but queues the desired owed state itself (owed_fire/owed_consumed_pending_owed_fires) before returning, and the warning names the queued recovery. The drain already refuses under _load_failed, so recovery correctly waits for a readable store. Regression: test_merge_queues_owed_state_when_the_store_is_unreadable.

F2 (stale docstring): removed the "an operator re-arms it" clause — retained one-shots are retried automatically by the replacement gateway via keep_overdue; no operator step exists.

337 targeted tests pass; black/flake8/isort/mypy clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
Treat script and agent cron launches from a fully replaced live install as retryable skips instead of spending an auto-pause failure strike. Clear pre-acquisition reaper state, preserve unrelated FileNotFoundError failures, and let the replacement gateway launch a version-consistent child on the next tick.
@rubencu
rubencu force-pushed the fix/cron-stale-runtime branch 2 times, most recently from 9f7c058 to ff6b051 Compare September 6, 2026 23:01
@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Round-11 findings addressed in ff6b051 (both lanes converged on the same regression):

Unreadable-store branch dropped the completed one-shot deletion: correct — my round-10 early return bypassed the base path's delete_owed bookkeeping, so a completed delete_after_run job would re-fire after the store healed. The branch now owes the removal to the existing defer_removal queue (in-memory only, no lock re-entry; its drain also refuses under _load_failed, so the delete lands together with recovery), carrying the same retention guards as the base path (fire_time_denied/run_never_started retained, not consumed). Regressions: test_merge_queues_the_one_shot_removal_when_the_store_is_unreadable + test_merge_does_not_queue_removal_for_a_never_started_one_shot.

Also rebased onto current main and validated against the new upstream test_cron_store_unreadable_boundaries.py contract suite — all 33 pass against this implementation (372 targeted tests total). black/flake8/isort/mypy clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running readiness: action required A blocking check or review needs attention and removed readiness: action required A blocking check or review needs attention readiness: checking Automated validation is still running labels Sep 6, 2026
@rubencu
rubencu force-pushed the fix/cron-stale-runtime branch from ff6b051 to 46cb5ac Compare September 6, 2026 23:56
@rubencu

rubencu commented Sep 6, 2026

Copy link
Copy Markdown
Contributor Author

Round-12 findings addressed in 46cb5ac:

F1 (owed policy denials refire every poll): correct — restoring the debt on fire_time_denied recreated the unbounded-refire shape one layer up: a policy denial can persist indefinitely and an owed job is due on every poll. Per the prescribed fix, the restore now keys on run_never_started only (a self-clearing state, so the retry is bounded); a policy-denied owed occurrence is dropped and the job resumes at its next scheduled slot, with the denial itself remaining operator-visible. Regression: test_a_policy_denied_owed_run_drops_the_debt.

F2 (reaped make-up runs retain consumed debt): correct — the reaper's terminal merge does not carry owed_fire, so the in-memory consume never reached disk and the stale True would replay the occurrence. The post-dispatch CancelledError path now queues the durable clear (_pending_owed_fires[job.id] = False, drained by tick/stop) before re-raising. Regression: test_cancellation_after_dispatch_keeps_the_debt_consumed extended to assert the queued clear.

373 targeted tests pass (incl. the 33 upstream store-boundary contracts); black/flake8/isort/mypy clean.

@github-actions github-actions Bot added readiness: checking Automated validation is still running and removed readiness: action required A blocking check or review needs attention labels Sep 7, 2026
@rubencu

rubencu commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Round-13 finding addressed in a18100d: correct, and the fix goes further than the suggested merge-side reapply — reapplying enabled=False on the merged copy would still die at the NEXT tick's _sync(), which wholesale-replaces self._jobs with fresh disk copies (enabled on disk by design, for the replacement gateway). The durable in-process guard is a new service-level pruned-quiesce registry: CronService.quiesce_pruned(job_id) records the id for the process lifetime (never persisted, never cleared — a drained process can never launch again), and the timer's due-scan excludes registered ids regardless of store reloads. _record_pruned_launch_skip registers through it (all three launch sites pass the service), keeping the per-object disable only for the current tick's snapshot; user_paused is untouched throughout. Regression: test_quiesced_pruned_job_survives_a_store_reload — simulates the resurrection (reload hands back an enabled copy, _is_due is True) and proves the registry is the operative guard keeping the due-scan empty.

374 targeted tests pass (incl. the 33 upstream store-boundary contracts); black/flake8/isort/mypy clean.

@rubencu

rubencu commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Round-14 findings addressed in f6a4a4c:

F1 (manual triggers create scheduled debt): correct — a manual trigger (run_job / cron trigger) failing on the pruned install persisted an owed occurrence the schedule never owed, so resuming a paused job later would execute it unscheduled. The service's existing _job_run_meta trigger provenance is now exposed as CronService.run_is_manual(job_id), and _record_pruned_launch_skip sets owed_fire only for scheduled runs (quiesce and never-started bookkeeping still apply to manual runs — the drained process can't launch either way). Regression: test_manual_trigger_during_pruning_persists_no_debt.

F2 (stale comment): the _execute debt-snapshot comment now states the round-12 semantics precisely — only self-clearing never-started states restore; a fire-time policy denial deliberately drops the debt.

375 targeted tests pass (incl. the 33 upstream store-boundary contracts); black/flake8/isort/mypy clean.

@rubencu

rubencu commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Round-15 findings addressed in 8835ae9:

F1 (quiesced overdue jobs hot-loop the timer): correct — the due-scan skipped quiesced ids but _next_wake_secs() still computed 0 for an overdue quiesced job, so every empty scan re-armed the timer immediately. _next_wake_secs() now excludes _pruned_quiesced ids with the reasoning documented inline. Regression: the reload-survival test now also asserts _next_wake_secs() is None with a quiesced overdue job as the only candidate.

F2 (docstring precision): the drain's save condition is now stated as "once iff a queued id is still present".

The Backend (Windows) shard failures on the previous head are in files this diff never touches (test_mcp_gateway_rewriter, test_mcp_quarantine, test_notification_settings, test_override_expiry_notice, test_security, test_session_cleanup) — flagging as base/host breakage; this push re-rolls them.

375 targeted tests pass (incl. the 33 upstream store-boundary contracts); black/flake8/isort/mypy clean.

@rubencu

rubencu commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Round-16 finding addressed in 7f4eece: correct — a cancellation landing after the callback starts but before the prompt/launch goes out (session/context setup, the agent path's longest window) set neither signal, and the handler wrongly kept the debt consumed. New in-memory CronJob.run_dispatched marker (reset per run, never merged): the gateway callbacks set it at the moment side effects become possible (script-pool launch await; both agent _prompt_dispatched points). The cancellation handler now restores the debt when the run provably never happened — no dispatch confirmation yet, OR a guaranteed-nothing-ran path recorded run_never_started (these can fire after the script-path marker is set while still queued/vetting, which is why both signals are consulted) — and keeps it consumed with the durable queued clear otherwise. Regressions: test_cancellation_during_setup_restores_the_debt (the exact reported window) plus the updated pre/post-dispatch pair.

The recurring Backend shard-3/Windows-3 + Coverage Gate failures remain in files this diff does not touch (base breakage, flagged in round 15); this push re-rolls them.

376 targeted tests pass (incl. the 33 upstream store-boundary contracts); black/flake8/isort/mypy clean.

…ng runs

The pruned-runtime skip reused completed-Skip semantics, so the
scheduler advanced last_run_ts as a success and _merge_job_result
deleted a due one-shot delete_after_run job irrecoverably. Book all
pruned-launch sites through a shared never-started helper: last_status
'error' blocks the success branch, run_never_started retains the
one-shot, and no auto-pause strike is spent.

Review-round hardening:
- New CronJob.keep_overdue marker (in-memory, reset per run): a pruned
  skip neither advances last_run_ts nor fires _execute's at-job
  disable, and the merge skips its at-job enabled propagation — the
  replacement gateway sees the job exactly as due as before the
  drained process touched it.
- Quiesce past-due at-jobs in memory only, killing the zero-delay
  refire loop on the drained gateway without persisting disabled.
- A mid-sequence pruning after a prior agent completed records a
  normal failed run instead of never-started, so the retained one-shot
  cannot replay finished side effects.
- Scope the ENOENT excuse to this install's own trees (interpreter
  prefix + package tree; never their common parent, which degrades to
  '/' on non-versioned layouts); pathless ENOENT stays a real failure.

Regression tests cover the retention contract, the one-shot data-loss
shape, schedule-owing semantics at the scheduler (_execute) level,
at-job quiescence without durable parking, unrelated-path and pathless
strictness, and the partial-sequence failure recording.
@rubencu

rubencu commented Sep 7, 2026

Copy link
Copy Markdown
Contributor Author

Round-17 findings addressed in ab75625:

F1 (dispatch marked before prompt submission): correct — the marker sat before the stream await, so a cancellation inside session warmup (ensure_ready()) consumed the debt with nothing sent. Dispatch confirmation now rides the tool-gate callback (on_tool_gate wrapper at both agent sites): a tool call reaching the gate is the earliest possible side-effect signal, so the debt stays consumed from that moment; a cancelled turn that produced no tool calls delivered nothing and is safe to replay, which is why text-chunk confirmation is deliberately not used (that also keeps on_chunk free for the post-token resume helper, whose kwarg collision surfaced immediately in the harness when I tried the chunk hook — 13 failures, reverted). The script path keeps its pre-launch marker: its pre-launch cancel paths (queue wait, vet deny) all record run_never_started, which the handler consults alongside run_dispatched.

F2 (drain failure aborts the tick): the tick's locked transaction now contains the drain's re-raise (log + continue) — the drain already requeued its claim, and a due job must not miss its minute over an unrelated persistence failure. Regression: test_a_drain_save_failure_does_not_abort_the_tick_scan (asserts the scan returns the snapshot AND the claim survives).

377 targeted tests pass (incl. the 33 upstream store-boundary contracts); black/flake8/isort/mypy clean.

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

Labels

fork Pull request from a fork (external contributor) readiness: passed Eligible automated validation passed for the current revision

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant