Only restore env vars when we actually modified them (noProxyDidModify
flag). Prevents silently deleting a user's NO_PROXY that already
contains loopback entries. Added regression test.
Fix Greptile review: when call A exits before call B, the isFirst flag
on B is false, so the restore condition (refCount===0 && isFirst) was
never true and NO_PROXY leaked permanently.
Remove '&& isFirst' so any last exiter (refCount===0) restores the
original env vars. Added explicit reverse-exit-order regression test.
Address Greptile review feedback:
- Replace snapshot/restore pattern with reference counter to prevent
permanent NO_PROXY env-var leak under concurrent async calls
- Include [::1] in the alreadyCoversLocalhost guard
- Add concurrency regression test
When HTTP_PROXY / HTTPS_PROXY / ALL_PROXY environment variables are set,
CDP connections to localhost/127.0.0.1 can be incorrectly routed through
the proxy (e.g. via global-agent or undici proxy dispatcher), causing
browser control to fail.
Fix:
- New cdp-proxy-bypass module with utilities for direct localhost connections
- WebSocket (ws) CDP connections: pass explicit http.Agent to bypass any
global proxy agent patching
- fetch-based CDP probes: wrap in withNoProxyForLocalhost() to temporarily
set NO_PROXY for the duration of the call
- Playwright connectOverCDP: wrap in withNoProxyForLocalhost() since
Playwright reads env vars internally
- 13 new tests covering getDirectAgentForCdp, hasProxyEnv, and
withNoProxyForLocalhost (env save/restore, error recovery)
When a Chrome relay targetId becomes stale between snapshot and action,
the browser tool now retries once without targetId so the relay falls
back to the currently attached tab.
Drop the unknown recovered field from the test mock return value
to satisfy tsc strict checking against BrowserActResponse.
Add recomputeNextRunsForMaintenance() call in run() so that stale
runningAtMs markers (from a crashed Phase-1 persist) are cleared by the
existing normalizeJobTickState logic before the already-running guard.
Without this, a manual cron.run() could be blocked for up to
STUCK_RUN_MS (2 hours) even though no job was actually running.
Fixes#17554
Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* fix(cron): wrap computeJobNextRunAtMs in try-catch inside applyJobResult
Without this guard, if the croner library throws during schedule
computation (timezone/expression edge cases), the exception propagates
out of applyJobResult and the entire state update is lost — runningAtMs
never clears, lastRunAtMs never advances, nextRunAtMs never recomputes.
After STUCK_RUN_MS (2h), stuck detection clears runningAtMs and the job
re-fires, creating a ~2h repeat cycle instead of the intended schedule.
The sibling function recomputeJobNextRunAtMs in jobs.ts already wraps
computeJobNextRunAtMs in try-catch; this was an oversight in the
applyJobResult call sites.
Changes:
- Error-backoff path: catch and fall back to backoff-only schedule
- Success path: catch and fall through to the MIN_REFIRE_GAP_MS safety net
- applyOutcomeToStoredJob: log a warning when job not found after forceReload
* fix(cron): use recordScheduleComputeError in applyJobResult catch blocks
Address review feedback: the original catch blocks only logged a warning,
which meant a persistent computeJobNextRunAtMs throw would cause a
MIN_REFIRE_GAP_MS (2s) hot loop on cron-kind jobs.
Now both catch blocks call recordScheduleComputeError (exported from
jobs.ts), which tracks consecutive schedule errors and auto-disables the
job after 3 failures — matching the existing behavior in
recomputeJobNextRunAtMs.
* test(cron): cover applyJobResult schedule-throw fallback paths
---------
Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
* fix(cron): skip isError payloads when picking summary/delivery content
buildEmbeddedRunPayloads appends isError warnings as the last payload.
Three functions in helpers.ts iterate last-to-first and pick the error
over real agent output. Use two-pass selection: prefer non-error payloads,
fall back to error-only when no real content exists.
Fixes: pickSummaryFromPayloads, pickLastNonEmptyTextFromPayloads,
pickLastDeliverablePayload — all now accept and filter isError.
* Changelog: note cron payload isError filtering (#21454)
---------
Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
* cron: treat announce delivery failure as ok when agent execution succeeded
* fix: set delivered:false and error on announce delivery failure paths
* Changelog: note cron announce delivery status handling (#31082)
---------
Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
* fix(cron): use longer default timeout for cron run --expect-final
* test(cron-cli): stabilize cron run timeout assertions with explicit run exits
---------
Co-authored-by: Kelly Baker <kelly@users.noreply.github.com>
Co-authored-by: Tak Hoffman <781889+Takhoffman@users.noreply.github.com>
