mirror of
https://github.com/openclaw/openclaw.git
synced 2026-08-22 10:25:20 -06:00
Peter Steinberger
58127fa73e
* fix(agents): remove a deleted agent's cron jobs on the offline delete path Follow-up to #127037, which fixed the exec-approvals half of the same gap and named this one explicitly. `agents delete` tries the Gateway first and falls back to a local path. The Gateway handler nests two transactional cleanups around the roster commit -- cron wrapping approvals wrapping the config write. After #127037 the offline path did the inner one; it still skipped cron. So deleting an agent without a Gateway left its scheduled jobs enabled: $ openclaw agents delete cronprobe --force Deleted agent: cronprobe <- no mention of cron $ sqlite3 <state>/state/openclaw.sqlite "select job_id, name, agent_id, enabled from cron_jobs" 975cb750-... | cronprobe-job | cronprobe | 1 $ openclaw cron list cronprobe-job every 1h Next: in 59m idle To be accurate about severity: this is not silent. Each firing records `error: "cron job agent is unavailable: cronprobe"` and `cron list` flips to `error`. The defect is that the job keeps its schedule forever, and that recreating an agent with the same id points it at the new agent. The fallback had collapsed two different reasons into one `null` return, which is what made the fix look unsafe at first: credential failures happen *before* transport, so a live scheduler may still own the cron store, while an unreachable Gateway means nothing else is holding it. `maybeDeleteAgentThroughGateway` now returns a discriminated union, and only the unreachable branch mutates the store directly. The credentials branch commits the roster, warns, and sets `cronCleanupSkipped: true` in JSON. The local `CronService` construction already existed inside `local-request-context.ts`; it moves to `src/cron/local-service.ts` and both callers share it rather than growing a second cron mutation path. That extraction also switches the default-owner resolver from `tryResolveLegacyCompatibilityAgentId` to `tryResolveAmbientOwnerAgentId`, which is a superset -- it honors an explicitly configured `agents.defaults.systemAgent.agentId` and otherwise falls back to exactly the previous function. Live testing showed agentless memory-dreaming jobs need it to load under explicit agent ownership. Production +89/-62. * test(agents): split the delete suite so the new cron coverage stays under the cap The 40-line cron regression test added in the previous commit pushed `src/commands/agents.delete.test.ts` to 1018 code lines, over the 1000 cap, and `check-lint-core-3` went red. Repo policy forbids a `max-lines` suppression. Unlike the earlier `cron/view.test.ts` split there was no describe-level seam -- 23 flat tests in a single describe -- so the split follows subject instead. The seven workspace-lifecycle tests (trashing, sharing, overlap, symlink reachability, workspace-state cleanup) move to `agents.delete.workspace.test.ts`. `vi.mock` and `vi.hoisted` are per-file and cannot be imported, so the mock preamble and the shared `beforeEach` are declared in both files; the helper block above them is unchanged in each. Each file then imports only what it uses, which is why the import lists differ. Trimming to a hair under the cap by moving only the new test was possible and rejected: it would have left the file at ~978 code lines, back at the cap within a couple of changes. This leaves 749 and 603 physical lines. No test content changed: 27 passed before, 27 after.