From f5cacfe528f2c9ded83a9502ecfa18acb36f5540 Mon Sep 17 00:00:00 2001
From: scottgl <scottgl@gmail.com>
Date: Tue, 17 Mar 2026 20:22:58 -0500
Subject: [PATCH] fix: address review comments (settled guard + static import)

---
 src/cron/pre-check.ts     |  9 +++++
 src/cron/service/timer.ts | 71 +++++++++++++++++----------------------
 2 files changed, 39 insertions(+), 41 deletions(-)

diff --git a/src/cron/pre-check.ts b/src/cron/pre-check.ts
index 49b036db6c9..974db5fd87d 100644
--- a/src/cron/pre-check.ts
+++ b/src/cron/pre-check.ts
@@ -27,6 +27,7 @@ export function runPreCheck(
   const timeoutMs = (preCheck.timeoutSeconds ?? DEFAULT_TIMEOUT_SECONDS) * 1_000;
 
   return new Promise((resolve) => {
+    let settled = false;
     const child = exec(preCheck.command, {
       timeout: timeoutMs,
       maxBuffer: MAX_OUTPUT_BYTES,
@@ -46,10 +47,18 @@ export function runPreCheck(
     });
 
     child.on("error", (err) => {
+      if (settled) {
+        return;
+      }
+      settled = true;
       resolve({ passed: false, reason: `preCheck error: ${err.message}` });
     });
 
     child.on("close", (code, signal) => {
+      if (settled) {
+        return;
+      }
+      settled = true;
       if (signal === "SIGTERM") {
         resolve({
           passed: false,
diff --git a/src/cron/service/timer.ts b/src/cron/service/timer.ts
index 388ad28ac58..3978ac34570 100644
--- a/src/cron/service/timer.ts
+++ b/src/cron/service/timer.ts
@@ -1,10 +1,6 @@
-import { resolveFailoverReasonFromError } from "../../agents/failover-error.js";
+import { dirname } from "node:path";
 import type { CronConfig, CronRetryOn } from "../../config/types.cron.js";
 import type { HeartbeatRunResult } from "../../infra/heartbeat-wake.js";
-import { DEFAULT_AGENT_ID } from "../../routing/session-key.js";
-import { resolveCronDeliveryPlan } from "../delivery.js";
-import { runPreCheck, applyPreCheckOutput } from "../pre-check.js";
-import { sweepCronRunSessions } from "../session-reaper.js";
 import type {
   CronDeliveryStatus,
   CronJob,
@@ -13,6 +9,12 @@ import type {
   CronRunStatus,
   CronRunTelemetry,
 } from "../types.js";
+import type { CronEvent, CronServiceState } from "./state.js";
+import { resolveFailoverReasonFromError } from "../../agents/failover-error.js";
+import { DEFAULT_AGENT_ID } from "../../routing/session-key.js";
+import { resolveCronDeliveryPlan } from "../delivery.js";
+import { runPreCheck, applyPreCheckOutput } from "../pre-check.js";
+import { sweepCronRunSessions } from "../session-reaper.js";
 import {
   computeJobPreviousRunAtMs,
   computeJobNextRunAtMs,
@@ -22,7 +24,6 @@ import {
   resolveJobPayloadTextForMain,
 } from "./jobs.js";
 import { locked } from "./locked.js";
-import type { CronEvent, CronServiceState } from "./state.js";
 import { ensureLoaded, persist } from "./store.js";
 import { DEFAULT_JOB_TIMEOUT_MS, resolveCronJobTimeoutMs } from "./timeout-policy.js";
 
@@ -1006,11 +1007,33 @@ export async function runDueJobs(state: CronServiceState) {
 export async function executeJobCore(
   state: CronServiceState,
   job: CronJob,
-<<<<<<< HEAD
   abortSignal?: AbortSignal,
 ): Promise<
   CronRunOutcome & CronRunTelemetry & { delivered?: boolean; deliveryAttempted?: boolean }
 > {
+  // ── Pre-check gate ─────────────────────────────────────────────────
+  // If the job has a preCheck, run the lightweight shell command first.
+  // Skip the entire agent turn (no tokens spent) if the gate fails.
+  let preCheckOutput: string | undefined;
+  if (job.preCheck?.command) {
+    // Run pre-check from the session store directory (near the agent workspace)
+    // or fall back to the gateway's working directory.
+    const cwd = state.deps.storePath ? dirname(state.deps.storePath) : undefined;
+    const result = await runPreCheck(job.preCheck, { cwd });
+    if (!result.passed) {
+      state.deps.log.debug(
+        { jobId: job.id, jobName: job.name, reason: result.reason },
+        "cron: preCheck gate failed, skipping job",
+      );
+      return { status: "skipped", error: result.reason };
+    }
+    preCheckOutput = result.output;
+    state.deps.log.debug(
+      { jobId: job.id, jobName: job.name, outputLen: preCheckOutput.length },
+      "cron: preCheck gate passed",
+    );
+  }
+
   const resolveAbortError = () => ({
     status: "error" as const,
     error: timeoutErrorMessage(),
@@ -1040,40 +1063,6 @@ export async function executeJobCore(
   if (abortSignal?.aborted) {
     return resolveAbortError();
   }
-=======
-): Promise<{
-  status: "ok" | "error" | "skipped";
-  error?: string;
-  summary?: string;
-  sessionId?: string;
-  sessionKey?: string;
-}> {
-  // ── Pre-check gate ─────────────────────────────────────────────────
-  // If the job has a preCheck, run the lightweight shell command first.
-  // Skip the entire agent turn (no tokens spent) if the gate fails.
-  let preCheckOutput: string | undefined;
-  if (job.preCheck?.command) {
-    // Run pre-check from the session store directory (near the agent workspace)
-    // or fall back to the gateway's working directory.
-    const cwd = state.deps.storePath
-      ? (await import("node:path")).dirname(state.deps.storePath)
-      : undefined;
-    const result = await runPreCheck(job.preCheck, { cwd });
-    if (!result.passed) {
-      state.deps.log.debug(
-        { jobId: job.id, jobName: job.name, reason: result.reason },
-        "cron: preCheck gate failed, skipping job",
-      );
-      return { status: "skipped", error: result.reason };
-    }
-    preCheckOutput = result.output;
-    state.deps.log.debug(
-      { jobId: job.id, jobName: job.name, outputLen: preCheckOutput.length },
-      "cron: preCheck gate passed",
-    );
-  }
-
->>>>>>> 063e2bec7b (feat(cron): add preCheck gate to skip jobs when nothing changed)
   if (job.sessionTarget === "main") {
     let text = resolveJobPayloadTextForMain(job);
     if (!text) {