Greptile Summary

This PR refactors the cron and agent task-lifecycle wiring in two complementary directions: (1) it introduces a task-executor.ts façade that wraps task-registry.ts with intent-revealing helpers (createRunningTaskRun, completeTaskRunByRunId, failTaskRunByRunId, …), forcing callers to encode success/failure semantics at the call site rather than passing raw status strings; and (2) it extracts message-formatting and delivery-policy predicates into a standalone task-executor-policy.ts that can be unit-tested independently. The executeJobCore function in timer.ts is also decomposed into executeMainSessionCronJob and executeDetachedCronJob to make the two dispatch paths explicit.

Key points:

• All behavioral logic in the extracted helpers is identical to what was previously inlined in task-registry.ts — this is a pure restructuring.
• The success/failure branch split in markBackgroundTaskTerminal (manager.core.ts) and subagent-registry-lifecycle.ts is correct; terminalOutcome is forwarded only on the success branch where it is semantically meaningful.
• The abort-signal handling in executeMainSessionCronJob inlines { status: \"error\", error: timeoutErrorMessage() } directly instead of delegating to resolveAbortError() — the two are identical in value.
• createCronTaskRunId is defined identically in both timer.ts (line 127) and ops.ts (line 389) — a duplication that should be extracted to a shared location to avoid future divergence.
• The startedAt field that was previously forwarded through markTaskTerminalByRunId in subagent-registry-lifecycle.ts is not exposed by completeTaskRunByRunId/failTaskRunByRunId; in practice this is a no-op since startedAt is set correctly at creation time, but the API narrowing is worth documenting.

Confidence Score: 5/5

Safe to merge — all findings are P2 style/observability concerns with no impact on runtime correctness.

All detected issues are P2: one duplicated helper function and one silently-dropped field that is a no-op in practice. No logic regressions, security issues, or data-integrity problems were found. The refactoring is well-structured and backed by new unit and integration tests.

src/cron/service/timer.ts and src/cron/service/ops.ts share a duplicated createCronTaskRunId helper that should be extracted.

This is a comment left during a code review.
Path: src/cron/service/timer.ts
Line: 127-129

Comment:
**Duplicated helper in `ops.ts`**

`createCronTaskRunId` is defined identically in both `src/cron/service/timer.ts` and `src/cron/service/ops.ts` (line 389). If the run-ID format ever needs to change (e.g. adding a counter for jobs that fire more than once per millisecond), both copies must be updated in lockstep, creating a silent divergence risk.

Consider extracting this to a shared module — e.g. a `cron-run-id.ts` utility or placing it in `jobs.ts` — and importing it in both files.

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/subagent-registry-lifecycle.ts
Line: 226-244

Comment:
**`startedAt` silently dropped on terminal update**

The previous call to `markTaskTerminalByRunId` included `startedAt: entry.startedAt`, which allowed the terminal write to backfill the `startedAt` field if it differed from what was stored at creation time. Neither `completeTaskRunByRunId` nor `failTaskRunByRunId` exposes a `startedAt` parameter, so this field is now silently dropped.

In practice the task record is created with the same `startedAt` value in `subagent-registry-run-manager.ts`, so there should be no observable delta today. But this is a silent narrowing of the API: callers that previously relied on terminal-time correction of `startedAt` can no longer do so through the new helpers. Worth noting in a comment or ensuring neither helper needs to accept `startedAt` for future use cases.

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

_{Reviews (1): Last reviewed commit: "Merge branch 'main' into pr5/cron-target..." | Re-trigger Greptile}