Greptile Summary

This PR fixes two related bugs in cron-tool.ts that caused cron add to fail with "job: must be object" when LLMs sent flat parameters: the job/patch schema is relaxed from Type.Object to Type.Unsafe to survive provider-level validation, and schedule shorthand keys (every, everyMs, cron, at, tz, stagger, staggerMs, exact) are added to the flat-params recovery sets so LLM-generated flat calls are correctly reconstructed. The same fix is applied symmetrically to both the add and update code paths, and six new test cases verify the new behaviour.

Key findings:

• staggerMs silently dropped in cron reconstruction – staggerMs is listed in JOB_KEYS (meaning it is expected as a flat param), but the cron-shorthand reconstruction block only spreads tz into the synthetic schedule. The cleanup loop then deletes staggerMs, so { cron: "0 0 * * *", staggerMs: 5000, ... } loses the stagger value. The same gap exists in the update path. A one-line spread would fix it.

• Misleading null-coercion comment – The comment (and the test name) claim that null is coerced to undefined, but typeof null === "object" means the guard params.job != null && typeof params.job !== "object" never fires for null. Null is handled correctly downstream by !params.job being truthy, but the comment misrepresents the code, which could confuse future maintainers.

No test covers staggerMs as a flat param; adding one would document the expected behaviour (or surface the gap).

Confidence Score: 5/5

Safe to merge; the root-cause fix is correct and all remaining findings are P2 edge-case gaps that do not affect the primary bug being addressed.

Both open findings are P2: the staggerMs data-loss only occurs on a combination (cron shorthand + staggerMs flat param) that no current LLM is known to produce, and the misleading comment does not affect runtime behaviour. The core fix — schema relaxation, null/string coercion, and every/cron/at reconstruction — is correct and well-tested.

src/agents/tools/cron-tool.ts — specifically the two duplicate schedule-reconstruction blocks (add and update paths) where staggerMs should be spread into the cron schedule object.

This is a comment left during a code review.
Path: src/agents/tools/cron-tool.ts
Line: 363-386

Comment:
**`staggerMs` silently dropped after cron-shorthand reconstruction**

`staggerMs` is now captured in `JOB_KEYS` (signalling that LLMs may send it as a flat param), but the `cron` shorthand reconstruction only spreads `tz` into the synthetic schedule — `staggerMs` is never added. The cleanup loop then deletes it, so a flat call like `{ cron: "0 0 * * *", staggerMs: 5000, message: "ping" }` produces `{ kind: "cron", expr: "0 0 * * *" }` with `staggerMs` silently discarded.

The same gap exists in the `update` code path (the duplicate reconstruction block around lines 556-600).

Consider spreading `staggerMs` into the cron schedule when present:

```typescript
} else if (typeof synthetic.cron === "string" && synthetic.cron.trim()) {
  synthetic.schedule = {
    kind: "cron",
    expr: synthetic.cron.trim(),
    ...(typeof synthetic.tz === "string" && synthetic.tz.trim()
      ? { tz: synthetic.tz.trim() }
      : {}),
    ...(typeof synthetic.staggerMs === "number" ? { staggerMs: synthetic.staggerMs } : {}),
  };
```

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

---

This is a comment left during a code review.
Path: src/agents/tools/cron-tool.ts
Line: 308-312

Comment:
**Misleading comment — `null` is not coerced here**

The comment says "Coerce non-object job values **(null**, string) to undefined", but because `typeof null === "object"` in JavaScript, the condition `params.job != null && typeof params.job !== "object"` evaluates as:

- `null`: `null != null` → `false` → the branch is **never entered**; `params.job` stays `null`
- `"some string"`: both checks pass → correctly coerced to `undefined` ✓

`null` still works because the immediately-following `!params.job` check is truthy for `null`, so flat-params recovery does trigger. But the comment and the test name (`"coerces null job to undefined"`) misrepresent what the code actually does, which could mislead future readers.

```suggestion
          // Coerce non-null non-object job values (e.g. a bare string) to undefined so
          // flat-params recovery triggers instead of failing schema validation (#56996).
          // Note: null is handled implicitly below by `!params.job` being truthy.
          if (params.job != null && typeof params.job !== "object") {
            params.job = undefined;
          }
```

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

_{Reviews (1): Last reviewed commit: "fix(cron): recover flat schedule params ..." | Re-trigger Greptile}

This is related to the legacy cron shape cleanup, but I’m leaving it open as a separate track.

#71534 fixed persisted legacy jobs.json rows that had flat cron/tz/session fields. This PR is about agent tool calls sending flat cron params into cron-tool.ts, including schedule shorthand recovery and flat-param validation. That is a different input boundary, so it should not be closed as a duplicate of the store-load fix.

Next useful step here is resolving the existing review comments, especially preserving flat staggerMs during cron shorthand reconstruction.

Thanks for the useful flat-schedule recovery work here. I reworked the safe subset into #71547 and merged it to main as 9242713.

That version preserves flat cron/tz/staggerMs add/update calls before gateway validation and also resolves the remaining cron restart interruption issues in the same cleanup pass. Closing this PR as superseded so we do not keep two competing cron-tool fixes open. If there is a separate non-object coercion case still reproducible on current main, please reopen with the exact payload and we can route it as a focused follow-up.