fix(doctor): stop promising --fix for working isolated shell-prompt cron jobs (#94655) (#94784)

Summary:
- Merged fix(doctor): stop promising --fix for working isolated shell-prompt cron jobs (#94655) after ClawSweeper review.

Automerge notes:
- PR branch already contained follow-up commit before automerge: fix(doctor): stop promising --fix for working isolated shell-prompt c…

Validation:
- ClawSweeper review passed for head 0d71970a16.
- Required merge gates passed before the squash merge.

Prepared head SHA: 0d71970a16
Review: https://github.com/openclaw/openclaw/pull/94784#issuecomment-4767423033

Co-authored-by: clawsweeper <274271284+clawsweeper[bot]@users.noreply.github.com>
Co-authored-by: ZengWen-DT <290981215+ZengWen-DT@users.noreply.github.com>
Co-authored-by: Altay <altay@hey.com>
Approved-by: altaywtf
This commit is contained in:
clawsweeper[bot] 2026-06-22 10:42:21 +00:00 committed by GitHub
parent e6f3912347
commit 6f17c4cc6d
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
8 changed files with 262 additions and 34 deletions

View file

@ -26,7 +26,7 @@ function parseJsonCommand(command, args, options = {}) {
} catch (error) {
lastError = error;
const message = error instanceof Error ? error.message : String(error);
const retryable = /HTTP 5\d\d|Server Error|ETIMEDOUT|ECONNRESET|EAI_AGAIN/u.test(message);
const retryable = isRetryableGhJsonErrorMessage(message);
if (!retryable || attempt === GH_JSON_RETRY_DELAYS_MS.length) {
throw error;
}
@ -36,6 +36,12 @@ function parseJsonCommand(command, args, options = {}) {
throw lastError;
}
export function isRetryableGhJsonErrorMessage(message) {
return /HTTP 5\d\d|HTTP 429|Server Error|secondary rate limit|abuse detection|ETIMEDOUT|ECONNRESET|EAI_AGAIN/iu.test(
message,
);
}
function normalizeRunJob(job) {
return {
completedAt: job.completedAt ?? job.completed_at ?? null,

View file

@ -600,6 +600,144 @@ describe("maybeRepairLegacyCronStore", () => {
expectNoteContaining("1 job still uses legacy", "Cron");
});
it("advises on isolated shell-prompt jobs without a non-actionable --fix repair note (#94655)", async () => {
const storePath = await makeTempStorePath();
const shellPromptJobs: Array<Record<string, unknown>> = [
createCurrentCronJob({
id: "shell-prompt-job-1",
name: "Shell prompt job 1",
schedule: { kind: "cron", expr: "*/30 * * * *", tz: "UTC" },
sessionTarget: "isolated",
payload: {
kind: "agentTurn",
message:
"Run python3 scripts/check_mail.py and send a compact summary if anything changed.",
toolsAllow: ["*"],
},
delivery: { mode: "announce" },
}),
createCurrentCronJob({
id: "shell-prompt-job-2",
name: "Shell prompt job 2",
schedule: { kind: "cron", expr: "15 * * * *", tz: "UTC" },
sessionTarget: "isolated",
payload: {
kind: "agentTurn",
message: "Run node scripts/check_mail.js and summarize any new messages.",
toolsAllow: ["bash"],
},
delivery: { mode: "announce" },
}),
createCurrentCronJob({
id: "shell-prompt-job-3",
name: "Shell prompt job 3",
schedule: { kind: "cron", expr: "45 * * * *", tz: "UTC" },
sessionTarget: "isolated",
payload: {
kind: "agentTurn",
message: "Execute ./scripts/check_mail.sh and report changed mailbox counts.",
toolsAllow: ["shell"],
},
delivery: { mode: "announce" },
}),
];
const shellPromptJob = requirePersistedJob(shellPromptJobs, 0);
await writeCurrentCronStore(storePath, shellPromptJobs);
const prompter = makePrompter(true);
await maybeRepairLegacyCronStore({
cfg: createCronConfig(storePath),
options: {},
prompter,
});
// The advisory is informational only: doctor --fix cannot rewrite a working
// isolated agentTurn job, so the misleading repair note must stay absent.
expectNoNoteContaining("Cron store issues detected", "Cron");
expectNoteContaining(
"3 isolated cron jobs drive shell/process tools from the agent prompt and keep running as-is: `Shell prompt job 1`, `Shell prompt job 2`, `Shell prompt job 3`.",
"Cron",
);
expectNoteContaining("informational only", "Cron");
expectNoteContaining("Shell prompt job 1", "Cron");
expectNoteContaining("Shell prompt job 2", "Cron");
expectNoteContaining("Shell prompt job 3", "Cron");
expectNoNoteContaining("openclaw doctor --fix", "Cron");
expectNoNoteContaining("jobs.json", "Cron");
expect(prompter.confirm).not.toHaveBeenCalled();
// No churn: the advisory does not rewrite the still-working jobs.
const persistedJobs = await readPersistedJobs(storePath);
expect(persistedJobs).toEqual(shellPromptJobs);
const job = requirePersistedJob(persistedJobs, 0);
expect(job).toEqual(shellPromptJob);
const reloaded = await loadCronJobsStoreWithConfigJobs(storePath);
expect(reloaded.configJobIndexes).toEqual([0, 1, 2]);
expect(reloaded.invalidConfigRows).toEqual([]);
const configJob = requirePersistedJob(reloaded.configJobs, 0);
expect(configJob).toEqual(
Object.fromEntries(Object.entries(shellPromptJob).filter(([key]) => key !== "updatedAtMs")),
);
expect(reloaded.configJobRuntimeEntries[0]).toEqual({
updatedAtMs: shellPromptJob.updatedAtMs,
state: {},
scheduleIdentity: JSON.stringify({
version: 1,
enabled: shellPromptJob.enabled,
schedule: shellPromptJob.schedule,
}),
});
const payload = requireRecord(job.payload, "cron payload");
expect(payload.kind).toBe("agentTurn");
expect(payload.message).toContain("python3 scripts/check_mail.py");
});
it("keeps restricted command prompts actionable without a --fix repair note", async () => {
const storePath = await makeTempStorePath();
const commandPromptJob = createCurrentCronJob({
id: "restricted-command-prompt",
name: "Restricted command prompt",
schedule: { kind: "cron", expr: "*/30 * * * *", tz: "UTC" },
sessionTarget: "isolated",
payload: {
kind: "agentTurn",
message: [
"Command to run:",
"- command: python3 scripts/check_mail.py",
"- workdir: /home/openclaw/.razor/clawd",
].join("\n"),
toolsAllow: ["read", "message"],
},
delivery: { mode: "announce" },
});
await writeCurrentCronStore(storePath, [commandPromptJob]);
const prompter = makePrompter(true);
await maybeRepairLegacyCronStore({
cfg: createCronConfig(storePath),
options: {},
prompter,
});
expectNoNoteContaining("Cron store issues detected", "Cron");
expectNoteContaining(
"1 isolated cron job describes a shell command in the agent prompt but lacks shell/process tool access: `Restricted command prompt`.",
"Cron",
);
expectNoteContaining("not the supported shell-tool prompt shape", "Cron");
expectNoteContaining("Recreate the job as a command cron job", "Cron");
expectNoNoteContaining("informational only", "Cron");
expectNoNoteContaining("keep running as-is", "Cron");
expectNoNoteContaining("openclaw doctor --fix", "Cron");
expect(prompter.confirm).not.toHaveBeenCalled();
const job = requirePersistedJob(await readPersistedJobs(storePath), 0);
const payload = requireRecord(job.payload, "cron payload");
expect(payload.kind).toBe("agentTurn");
expect(payload.message).toContain("python3 scripts/check_mail.py");
expect(payload.toolsAllow).toEqual(["read", "message"]);
});
it("repairs malformed persisted cron ids before list rendering sees them", async () => {
const storePath = await makeTempStorePath();
await writeCronStore(storePath, [

View file

@ -30,6 +30,8 @@ import {
} from "./legacy-store-migration.js";
import {
formatLegacyIssuePreview,
formatUnresolvedCommandPromptAdvisory,
formatUnresolvedShellPromptAdvisory,
mergeLegacyCronJobs,
mergeRuntimeEntryIntoConfigJob,
needsSqliteProjectionBackfill,
@ -336,9 +338,21 @@ export async function maybeRepairLegacyCronStore(params: {
const normalized = normalizeStoredCronJobs(rawJobs);
const notifyCount = rawJobs.filter((job) => job.notify === true).length;
const dreamingStaleCount = countStaleDreamingJobs(rawJobs);
const previewLines = formatLegacyIssuePreview(normalized.issues, {
unresolvedAgentTurnShellToolPrompt: normalized.unresolvedAgentTurnShellToolPromptJobs,
});
// Unresolved agentTurn command prompts are not auto-fixable; keep them out of the
// --fix preview so the repair note does not promise a fix that never lands (#94655).
const commandPromptAdvisory = formatUnresolvedCommandPromptAdvisory(
normalized.unresolvedAgentTurnCommandPromptJobs,
);
if (commandPromptAdvisory) {
note(commandPromptAdvisory, "Cron");
}
const shellPromptAdvisory = formatUnresolvedShellPromptAdvisory(
normalized.unresolvedAgentTurnShellToolPromptJobs,
);
if (shellPromptAdvisory) {
note(shellPromptAdvisory, "Cron");
}
const previewLines = formatLegacyIssuePreview(normalized.issues);
if (legacyStoreDetected) {
previewLines.unshift(
legacyImportCount > 0

View file

@ -12,6 +12,10 @@ type LegacyAgentTurnCommandPayload = {
timeoutSeconds?: number;
};
export type UnresolvedAgentTurnShellToolPromptKind =
| "commandPromptWithoutShellAccess"
| "shellToolPrompt";
const LEGACY_AGENT_TURN_COMMAND_MARKER_RE = /\bCommand to run\s*:/iu;
const LEGACY_AGENT_TURN_COMMAND_FIELD_RE = /^\s*-\s*(command|workdir|timeout)\s*:\s*(.*?)\s*$/iu;
const SHELL_TOOL_NAMES = new Set(["bash", "command", "exec", "process", "shell", "sh"]);
@ -217,17 +221,27 @@ export function migrateLegacyAgentTurnCommandPayload(payload: UnknownRecord): bo
return true;
}
export function hasUnresolvedAgentTurnShellToolPrompt(payload: UnknownRecord): boolean {
export function classifyUnresolvedAgentTurnShellToolPrompt(
payload: UnknownRecord,
): UnresolvedAgentTurnShellToolPromptKind | null {
if (payload.kind !== "agentTurn") {
return false;
return null;
}
const message = readString(payload.message);
if (typeof message !== "string") {
return false;
return null;
}
const parsed = parseLegacyAgentTurnCommandMessage(message);
return (
Boolean(parsed) ||
(hasShellToolAccess(payload.toolsAllow) && SHELL_COMMAND_MESSAGE_RE.test(message))
);
const shellToolAccess = hasShellToolAccess(payload.toolsAllow);
if (parsed && !shellToolAccess) {
return "commandPromptWithoutShellAccess";
}
if (shellToolAccess && SHELL_COMMAND_MESSAGE_RE.test(message)) {
return "shellToolPrompt";
}
return null;
}
export function hasUnresolvedAgentTurnShellToolPrompt(payload: UnknownRecord): boolean {
return classifyUnresolvedAgentTurnShellToolPrompt(payload) !== null;
}

View file

@ -5,28 +5,55 @@ import { normalizeCronJobInput } from "../../../cron/normalize.js";
import type { CronJob } from "../../../cron/types.js";
type CronLegacyIssueCounts = Partial<Record<string, number>>;
type CronLegacyIssueDetails = {
unresolvedAgentTurnShellToolPrompt?: string[];
};
function pluralize(count: number, noun: string) {
return `${count} ${noun}${count === 1 ? "" : "s"}`;
}
function formatJobNameList(names: string[] | undefined): string {
if (!names || names.length === 0) {
return "";
}
function formatJobNameList(names: string[]): string {
const preview = names.slice(0, 5).map((name) => `\`${name}\``);
const remaining = names.length - preview.length;
return remaining > 0 ? `: ${preview.join(", ")} (+${remaining} more)` : `: ${preview.join(", ")}`;
}
/**
* Advisory for isolated agentTurn cron jobs that describe a command but cannot access shell tools.
* These need operator attention, but `doctor --fix` cannot safely infer whether to grant tool
* access or recreate them as command cron jobs.
*/
export function formatUnresolvedCommandPromptAdvisory(names: string[]): string | null {
if (names.length === 0) {
return null;
}
const describeVerb = names.length === 1 ? "describes" : "describe";
const accessVerb = names.length === 1 ? "lacks" : "lack";
return [
`${pluralize(names.length, "isolated cron job")} ${describeVerb} a shell command in the agent prompt but ${accessVerb} shell/process tool access${formatJobNameList(names)}.`,
"- This is not the supported shell-tool prompt shape, so doctor cannot prove the job will execute the requested command.",
'- Recreate the job as a command cron job (`openclaw cron add ... --command "<shell>"`) or grant explicit shell/process tool access before relying on it.',
].join("\n");
}
/**
* Advisory for isolated agentTurn cron jobs that drive shell/process tools from the prompt.
* These keep running and are not a legacy store row, so `doctor --fix` cannot rewrite them;
* routing this through the auto-repair preview made the finding persist after every --fix.
*/
export function formatUnresolvedShellPromptAdvisory(names: string[]): string | null {
if (names.length === 0) {
return null;
}
const verb = names.length === 1 ? "drives" : "drive";
const keepVerb = names.length === 1 ? "keeps" : "keep";
return [
`${pluralize(names.length, "isolated cron job")} ${verb} shell/process tools from the agent prompt and ${keepVerb} running as-is${formatJobNameList(names)}.`,
"- This is a supported shape, not a legacy store row, so the doctor fix path cannot convert it and the finding is informational only.",
'- For a deterministic run, recreate the job as a command cron job (`openclaw cron add ... --command "<shell>"`).',
].join("\n");
}
/** Convert legacy cron issue counts into doctor preview lines. */
export function formatLegacyIssuePreview(
issues: CronLegacyIssueCounts,
details: CronLegacyIssueDetails = {},
): string[] {
export function formatLegacyIssuePreview(issues: CronLegacyIssueCounts): string[] {
const lines: string[] = [];
if (issues.jobId) {
lines.push(`- ${pluralize(issues.jobId, "job")} still uses legacy \`jobId\``);
@ -58,11 +85,6 @@ export function formatLegacyIssuePreview(
`- ${pluralize(issues.legacyAgentTurnCommandPayload, "job")} uses an agent prompt to run a shell command`,
);
}
if (issues.unresolvedAgentTurnShellToolPrompt) {
lines.push(
`- ${pluralize(issues.unresolvedAgentTurnShellToolPrompt, "job")} asks an isolated agent for shell/process tools and needs manual command conversion${formatJobNameList(details.unresolvedAgentTurnShellToolPrompt)}`,
);
}
if (issues.legacyPayloadProvider) {
lines.push(
`- ${pluralize(issues.legacyPayloadProvider, "job")} still uses payload \`provider\` as a delivery alias`,

View file

@ -190,6 +190,8 @@ describe("normalizeStoredCronJobs", () => {
expect(result.issues.legacyAgentTurnCommandPayload).toBeUndefined();
expect(result.issues.unresolvedAgentTurnShellToolPrompt).toBe(1);
expect(result.unresolvedAgentTurnCommandPromptJobs).toEqual(["Legacy job"]);
expect(result.unresolvedAgentTurnShellToolPromptJobs).toEqual([]);
const payload = job.payload as Record<string, unknown>;
expect(payload.kind).toBe("agentTurn");
expect(payload.message).toContain(command);

View file

@ -14,7 +14,7 @@ import { inferCronJobName } from "../../../cron/service/normalize.js";
import { normalizeCronStaggerMs, resolveDefaultCronStaggerMs } from "../../../cron/stagger.js";
import { normalizeLegacyDeliveryInput } from "./legacy-delivery.js";
import {
hasUnresolvedAgentTurnShellToolPrompt,
classifyUnresolvedAgentTurnShellToolPrompt,
hasLegacyOpenAICodexCronModelRef,
migrateLegacyAgentTurnCommandPayload,
migrateLegacyCronPayload,
@ -41,6 +41,7 @@ type CronStoreIssues = Partial<Record<CronStoreIssueKey, number>>;
type NormalizeCronStoreJobsResult = {
issues: CronStoreIssues;
unresolvedAgentTurnCommandPromptJobs: string[];
unresolvedAgentTurnShellToolPromptJobs: string[];
jobs: Array<Record<string, unknown>>;
mutated: boolean;
@ -246,7 +247,12 @@ export function normalizeStoredCronJobs(
jobs: Array<Record<string, unknown>>,
): NormalizeCronStoreJobsResult {
const issues: CronStoreIssues = {};
const unresolvedAgentTurnCommandPromptJobs: string[] = [];
const unresolvedAgentTurnShellToolPromptJobs: string[] = [];
const unresolvedAgentTurnPromptJobsByKind = {
commandPromptWithoutShellAccess: unresolvedAgentTurnCommandPromptJobs,
shellToolPrompt: unresolvedAgentTurnShellToolPromptJobs,
};
let mutated = false;
const keptJobs: Array<Record<string, unknown>> = [];
const removedJobs: NormalizeCronStoreJobsResult["removedJobs"] = [];
@ -421,11 +427,14 @@ export function normalizeStoredCronJobs(
if (migrateLegacyAgentTurnCommandPayload(payloadRecord)) {
mutated = true;
trackIssue("legacyAgentTurnCommandPayload");
} else if (hasUnresolvedAgentTurnShellToolPrompt(payloadRecord)) {
trackIssue("unresolvedAgentTurnShellToolPrompt");
const name = normalizeOptionalString(raw.name) ?? normalizeOptionalString(raw.id);
if (name) {
unresolvedAgentTurnShellToolPromptJobs.push(name);
} else {
const unresolvedPromptKind = classifyUnresolvedAgentTurnShellToolPrompt(payloadRecord);
if (unresolvedPromptKind) {
trackIssue("unresolvedAgentTurnShellToolPrompt");
const name = normalizeOptionalString(raw.name) ?? normalizeOptionalString(raw.id);
if (name) {
unresolvedAgentTurnPromptJobsByKind[unresolvedPromptKind].push(name);
}
}
}
}
@ -626,5 +635,12 @@ export function normalizeStoredCronJobs(
jobs.splice(0, jobs.length, ...keptJobs);
}
return { issues, unresolvedAgentTurnShellToolPromptJobs, jobs, mutated, removedJobs };
return {
issues,
unresolvedAgentTurnCommandPromptJobs,
unresolvedAgentTurnShellToolPromptJobs,
jobs,
mutated,
removedJobs,
};
}

View file

@ -2,6 +2,7 @@
import { describe, expect, it } from "vitest";
import {
collectRunJobsFromPages,
isRetryableGhJsonErrorMessage,
parseRunTimingArgs,
selectLatestMainPushCiRun,
summarizePnpmStoreWarmupBarrier,
@ -143,6 +144,21 @@ describe("scripts/ci-run-timings.mjs", () => {
]);
});
it("retries transient GitHub API failures while preserving auth failures", () => {
for (const message of [
"gh: API secondary rate limit exceeded (HTTP 403)",
"gh: HTTP 429: too many requests",
"Command failed: gh api repos/openclaw/openclaw/actions/runs/1/jobs\nHTTP 502",
"read ECONNRESET",
]) {
expect(isRetryableGhJsonErrorMessage(message)).toBe(true);
}
expect(
isRetryableGhJsonErrorMessage("gh: Resource not accessible by integration (HTTP 403)"),
).toBe(false);
});
it("summarizes the pnpm store warmup fanout barrier", () => {
expect(
summarizePnpmStoreWarmupBarrier({