Flaky test: exact Date.now() equality

wallNow is captured before the async function call, while deadlineAtMs inside executeJobCoreWithTimeout is set to a second Date.now() invocation. If even 1 ms elapses between the two calls (very possible under CI load), the .toBe() assertion fails. A range check or fake timers would make this deterministic:

Alternatively, mock Date.now with vi.spyOn(Date, "now").mockReturnValue(wallNow) to make both calls return the same value and keep the exact .toBe check.

This is a comment left during a code review.
Path: src/cron/service.issue-regressions.test.ts
Line: 1115-1119

Comment:
**Flaky test: exact `Date.now()` equality**

`wallNow` is captured before the async function call, while `deadlineAtMs` inside `executeJobCoreWithTimeout` is set to a second `Date.now()` invocation. If even 1 ms elapses between the two calls (very possible under CI load), the `.toBe()` assertion fails. A range check or fake timers would make this deterministic:

```suggestion
    const wallNow = Date.now();
    const result = await executeJobCoreWithTimeout(state, cronJob);

    expect(result).toMatchObject({ status: "ok", summary: "done" });
    // Allow a small window for the two Date.now() calls to differ
    expect(observedDeadlineAtMs).toBeGreaterThanOrEqual(wallNow + timeoutSeconds * 1000);
    expect(observedDeadlineAtMs).toBeLessThan(wallNow + timeoutSeconds * 1000 + 500);
```

Alternatively, mock `Date.now` with `vi.spyOn(Date, "now").mockReturnValue(wallNow)` to make both calls return the same value and keep the exact `.toBe` check.

How can I resolve this? If you propose a fix, please make it concise.

Preserve the last real error when a budget stop follows skips

When beforeAttempt returns stop here, the loop just breaks and falls through to throwFallbackFailureSummary(). That only rethrows the prior error when attempts.length <= 1, so any skipped candidate recorded earlier in this same run (for example the existing cooldown/auth skip branches) turns the result into a synthesized All models failed ... summary instead of the original primary failure. In a chain like “primary fails, next fallback is skipped, later fallback is stopped for low cron budget”, this change still masks the real error the commit is trying to preserve.

Useful? React with 👍 / 👎.

Propagate stop errors when all prior candidates were skipped

When beforeAttempt stops here after only skip-only entries have accumulated (for example, the requested model is skipped by the cooldown/auth branch earlier in runWithModelFallback, and the cron budget gate fires on the next runnable fallback), lastError is still undefined. This break then falls through to throwFallbackFailureSummary(), so callers see a synthesized All models failed ... error instead of the explicit stop message from beforeAttempt. In that cron path the new budget-stop behavior is still masked by a misleading summary.

Useful? React with 👍 / 👎.

Run the stop hook before marking cooldown probes

When beforeAttempt stops a near-deadline cron run on a cooldowned provider, the code above has already called markProbeAttempt() for that candidate before any request is sent. Because lastProbeAttempt is global and throttles probes for 30 seconds, the next fallback run can incorrectly skip a real cooldown-recovery probe even though this run never actually probed the provider. Moving the stop check ahead of the cooldown probe bookkeeping would avoid burning probe slots on aborted attempts.

Useful? React with 👍 / 👎.

Base the fallback guard on the actual agent timeout

For cron jobs that rely on agents.defaults.timeoutSeconds instead of payload.timeoutSeconds, timeoutMs in this function can be much smaller than the cron safety timeout. The new guard uses cronTimeoutMs here, so those jobs still require 30s remaining before any fallback, even when each model attempt is capped to 5–10s. A primary failure in the last 20–25s will now skip fallbacks that still had enough budget to complete under the configured agent timeout.

Useful? React with 👍 / 👎.

Remove the unconditional remote branch deletion

This helper always deletes fix/subagent-timeout-partial-results at the end, even though that branch is not in BRANCHES and the script never verifies that it was merged. Running the script to rebase the listed branches will also remove that remote branch from origin, which is destructive if the PR is still open or another maintainer is still using it.

Useful? React with 👍 / 👎.

Base the fallback guard on the actual agent timeout

For cron jobs that rely on agents.defaults.timeoutSeconds instead of payload.timeoutSeconds, timeoutMs in this function can be much smaller than the cron safety timeout. The new guard uses cronTimeoutMs here, so those jobs still require 30s remaining before any fallback, even when each model attempt is capped to 5–10s. A primary failure in the last 20–25s will now skip fallbacks that still had enough budget to complete under the configured agent timeout.

Useful? React with 👍 / 👎.

Remove the unconditional remote branch deletion

This helper always deletes fix/subagent-timeout-partial-results at the end, even though that branch is not in BRANCHES and the script never verifies that it was merged. Running the script to rebase the listed branches will also remove that remote branch from origin, which is destructive if the PR is still open or another maintainer is still using it.

Useful? React with 👍 / 👎.