feat(cron): manage scheduled jobs from the command line

[v01dstar]

Part of InsForge/instacloud#178 — platform-level cron, which tracks all five PRs, the merge order, and the decisions left open.

insta cron — schedules from the command line. Nine verbs over the platform's cron API: list, create, show, edit, pause, resume, delete, run, runs, preview.

Expressions are five-field and UTC, and every time this CLI prints is UTC. A schedule pinned to UTC does not keep a fixed local time, so a localised column would quietly lie across a DST boundary.

Three behaviours worth a reviewer's attention

The request is replaced, not merged — and edit says what that costs. The API cannot merge (a deep merge cannot express "remove this header") and the CLI could not either: header values are write-only, so the stored ones are unreadable here. Anything the new flags do not re-supply is gone, and the command names all three ways that happens — the headers it drops, a method that changes, and a stored body it is about to discard. --header alone on a POST job would otherwise turn it into a GET with no payload, and the job would keep running, answer 200, and do nothing.

One Idempotency-Key per invocation, minted inside run rather than at module load. That is what makes a trigger safe to retry: the platform dedupes a repeated key onto the same run, so a transport retry cannot fire the job twice — while a key captured once and shared would make the second deliberate cron run a duplicate of the first and silently swallow it. The tests assert both halves: one POST per invocation, and a different key on the next one.

Edits are conditioned on the revision just read. A concurrent edit fails and says so rather than clobbering, and the command does not re-read and retry behind the operator's back — the point of the guard is that the person decides. The test pins the GET count as well as the PATCH count, because a command that resolved the conflict itself would still show one PATCH per attempt.

Output

run reports what it did and no more: a manual run is an extra execution, so the next scheduled run still happens — except on a paused schedule, which accepts a manual run and stays paused, and is told that instead of being promised a scheduled run that is not coming.

show lists header names only; values are write-only on the platform and this command never pretends otherwise.

Verified

Typecheck clean, 85 files / 1771 tests pass, insta --help starts and lists cron between secrets and domain (the top-level surface test pins that list and its order).

🤖 Generated with Claude Code

[cubic-dev-ai]

1 issue found across 6 files

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="src/commands/cron.ts">

<violation number="1" location="src/commands/cron.ts:286">
P2: When editing a job that currently has a POST body, supplying only `--header` or `--method` silently replaces it with a GET or body-less request. Reject incomplete request replacements or require an explicit body/clear-body choice, and warn that the existing payload will be discarded.</violation>
</file>

<sub>Reply with feedback, questions, or to request a fix.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: When editing a job that currently has a POST body, supplying only --header or --method silently replaces it with a GET or body-less request. Reject incomplete request replacements or require an explicit body/clear-body choice, and warn that the existing payload will be discarded.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At src/commands/cron.ts, line 286:

<comment>When editing a job that currently has a POST body, supplying only `--header` or `--method` silently replaces it with a GET or body-less request. Reject incomplete request replacements or require an explicit body/clear-body choice, and warn that the existing payload will be discarded.</comment>

<file context>
@@ -0,0 +1,567 @@
+/** The stored request, from the flags alone. `undefined` when no flag shaped one. */
+export function buildRequest(o: RequestOpts): { method: CronMethod; headers?: Record<string, string>; body?: string } | undefined {
+  if (!namesRequest(o)) return undefined
+  const method = o.method ? parseMethod(o.method) : o.body !== undefined ? 'POST' : 'GET'
+  // A body on a GET is a typo with a plausible-looking outcome: the platform would send it and most
+  // targets would ignore it, so the job would run "fine" and do nothing. `--body` alone implies
</file context>

[v01dstar]

Reviewed the current head and API/header contracts for InsForge/instacloud#178. The 105 cron/help-surface tests and TypeScript checking passed. I did not find an additional functional CLI defect in the reviewed paths; one documentation regression is noted inline. The earlier request-replacement warning finding was checked against the current implementation rather than reposted. This does not validate a deployed backend.

[v01dstar]

[P3] Remove the duplicate command rows restored from the old CLI surface

The existing rows above already document insta service (including the services alias) and the complete insta secrets surface. This added second services row advertises set-access, scale, upgrade, and secrets subcommands that do not exist under services; running src/index.ts services --help on this head lists only add/list/remove/rename. The second secrets row also drops supported verbs from its duplicate description. Keep the new cron row and remove these stale duplicates; the parallel command-table additions in .claude/skills/developing-insta-cli/SKILL.md should likewise retain one current definition instead of reintroducing the old insta observe surface.

[v01dstar]

Addressed, plus the same mistake in a second file.

P3 · duplicate command rows — confirmed: the rows advertised services set-access/scale/upgrade/secrets, which do not exist (services --help lists add/list/remove/rename, and retired-policy.test.ts asserts those verbs are rejected). They were not authored here — a merge resolution that kept both sides re-introduced retired content. Removed; only the insta cron row remains.

The same resolution left a stale duplicate pair in .claude/skills/developing-insta-cli/SKILL.md (commands/ and observe/, the older wording, directly under main's current rows). Removed too, and cron folded into the row main actually maintains — so the docs diff against main is now exactly one added word and one added row.

Earlier findings from the first review round are also in: cron edit now names the method change and the dropped body, not only dropped headers (--header alone on a POST job silently turned it into a body-less GET); cron run no longer claims a scheduled run still happens when the job is paused; and the two tests that did not exercise the guarantees in their titles now count the trigger POSTs and the listing GET.

85 files / 1771 tests pass.

[v01dstar]

Follow-up review at e54b3ab9422f52a8a3027d173919dbdc9b1b1a2a: the duplicate/stale README command rows and duplicate developer-guide command/observe rows are removed. The new cron documentation remains, and there are no executable-code changes since the head tested in the previous review. This finding is addressed; I found no additional issue in the documentation-only delta. The previous 105-test result applies to the unchanged implementation; those tests were not rerun for this documentation edit.

[jwfing]

Summary

insta cron is a careful, well-reasoned addition — the three behaviours the body singles out (replace-not-merge with named losses, one Idempotency-Key per invocation, If-Match-conditioned edits with no silent retry) are all implemented as described and I confirmed each is pinned by a test that goes red when the behaviour is removed; no Critical findings.

Reviewed at head e54b3ab.

Requirements context

This repo has no /docs/superpowers/ and no plans/ — no matching spec/plan document exists. The requirements live in the tracking issue InsForge/instacloud#178, which this PR's body links and which carries the objective, the branch-semantics table, the merge order and the deliberately-open items. I graded against that issue plus the platform contract in the sibling PR InsForge/instacloud-platform#480 (src/cron/routes.ts, src/openapi/schemas/cron.ts, src/cron/service.ts at ef894d2), which is the API this CLI calls.

Against that contract the CLI is faithful: partial PATCH semantics (request/retry replace wholesale, omitted fields untouched) match UpdateCronJobBody; preview correctly treats valid:false as a 200 answer rather than a transport error, matching PreviewCronResult's own rationale; the --timeout 1000..300000 and --limit 1..100 bounds match the schemas exactly; and the paused-job branch of cron run matches service.ts:198-206, which has no enabled check by explicit design.

Verification I ran

Full suite in a clean clone, Node 20: 85 files / 1770 passed | 1 skipped, tsc --noEmit clean — the body's claim reproduces.

Four negative controls, to check the claims are actually bound rather than merely true:

sabotage	result
hoist idempotencyKey = randomUUID() to module scope (cron.ts:535)	2 red ✅
replace the If-Match header with {} (cron.ts:396-398)	1 red ✅
delete the POST-body-drop warning (cron.ts:325-327)	2 red ✅
delete if (scope.headers) … from api.ts:111	1770 passed — green ❌ (see S1)

Findings

Critical

(none)

Suggestion

S1 — software engineering / test coverage. src/api.ts:111 is the seam that actually puts If-Match and Idempotency-Key on the wire, and it is unbound: deleting that line leaves the entire 1771-test suite green. Both headline safety properties are asserted only against the hand-stubbed structural CronApi (cron.ts:379-382), which records what cronRun/patchJob pass, never what ApiClient sends. If that Object.assign were ever dropped in a refactor, every edit would go unconditioned (clobbering the concurrent editor the guard exists to protect) and every trigger would lose its key — with no test failing. The injection point already exists: ApiClient's fetchImpl constructor argument, used exactly this way in test/user-agent.test.ts. One test asserting a custom header reaches fetchImpl would close it — and ideally a second covering the 401-refresh replay, since api.ts:35-37 makes a specific claim about it (raw() re-invokes fetch with the same scope) that nothing currently asserts.

S2 — functionality. cronRun (cron.ts:529-541) does not handle the 409 the platform documents for this exact route: routes.ts:201 — "a run with this Idempotency-Key is still being created — retry". patchJob (cron.ts:400-403) translates its documented 409 into conflictLines with actionable advice; the trigger path lets the same status class surface as a bare ApiError. Given the key is already minted once per invocation, retrying the same request with the same key is precisely the safe action here — either do it once internally, or say so, rather than printing HTTP 409 at an operator who has no way to know a bare re-run mints a new key.

S3 — functionality. edit --path alone is refused even when the job already has a service target. Measured: assertTargetFlags({ path: '/new' }, true) throws --path applies to --service (an external target carries its path in the --url) (cron.ts:277). But the edit help text for the flag reads "request path on the --service target (default: the path it already had)", which implies exactly the call that fails — and targetOf's fallback at cron.ts:483 is written to preserve the existing path, so the intent looks present. Repointing an existing service job at a new path requires re-typing --service <name>. Loosening the guard in the partial branch when the current target is a service would match the documented behaviour.

S4 — functionality. No CLI flag sets the retry policy, though cron show renders it (retryLine, cron.ts:168-172) and the platform accepts retry on both create (CreateCronJobBody) and update (UpdateCronJobBody). The surface is read-only for a field the API makes writable, so a tenant who needs retryableStatuses: [429, 503] has to leave the CLI. Not blocking — the defaults are sensible and show names them — but worth either adding or stating as a deliberate v1 omission.

S5 — security. --header name=value is the only way to attach a credential to a cron job, and it lives on argv — visible in ps to every user on the box and written to shell history. The module's own comment at cron.ts:121-125 is explicit that this flag is the credential carrier ("silently dropping one of two credentials an agent passed"). This repo already has the escape hatch and the precedent: insta secrets set <name> [value] takes the value from stdin when omitted (index.ts:165). A --header-stdin / name@- form would match it. (The platform's own schema also steers credentials to secretRefs rather than literal headers — but per instacloud#178 that field is unwired today and would fail blocked on every fire, so omitting a --secret-ref flag is the right call for now; worth a comment saying so, since the omission otherwise reads as an oversight.)

S6 — software engineering / test coverage. Command-level coverage stops at run, edit and delete. Untested: cronRuns' fan-out (mapLimit, cron.ts:408-420) including the attempts-degrade-to-null path that runListLine's counter fallback exists to serve, and cronPreview's process.exitCode = 1 under --json (cron.ts:593) — which the comment calls out as the thing a piping script branches on, i.e. the highest-value assertion in that function. mapLimit is also not exported, so it cannot be unit-tested for the concurrency bound it claims.

Information

I1. The body and instacloud#178 both say "nine verbs"; there are ten (grep -c over the cron.command( registrations in index.ts returns 10). list create show edit pause resume delete run runs preview.

I2 — belongs to platform#480, not here. routes.ts:196 documents the trigger's 400 as "missing Idempotency-Key, or the job is disabled", which contradicts service.ts:200-205 ("A PAUSED schedule is triggerable, deliberately, and there is no enabled check here for that reason") and therefore contradicts this CLI's paused branch at cron.ts:548-550. The CLI is correct and the platform's schema description is stale; flagging only so the two PRs don't merge with a contract doc that says the opposite of both implementations.

I3. api.ts:111 applies scope.headers before Authorization and agentHeaders, so neither can be overridden — the ordering the comment claims, and correct. It can, however, override Content-Type, Insta-Hints and User-Agent. No caller does, and both current keys are literals; noting it because the type is an open Record<string, string>.

I4. The agent signature covers method, path and body — not headers. If-Match and Idempotency-Key are the first semantically load-bearing protocol headers to ride outside that signed surface. Nothing is weakened relative to today (no header was ever signed), but the property is new.

I5. No client-side length check for name (API: ≤64) or expression (≤256), where --timeout and --limit are both pre-validated against the same schema file. The API 400s cleanly on both, so this is consistency only, not a gap.

I6. cronEdit writes the drop warnings to stderr before the PATCH (cron.ts:481), so a subsequent 409 leaves the operator having read "these headers are dropped" for a write that never happened. conflictLines says "nothing was written", which resolves it — ordering is fine, just worth knowing the sequence is intentional.

Performance

cron runs is 1 + N requests, N ≤ 100 — a deliberate N+1, argued in the comment at cron.ts:562-565, bounded to 5 concurrent by mapLimit, and degrading to counters rather than failing the page when a detail read errors. That is the right trade for this surface. serviceNames adds one extra request to list/show and is skipped entirely when no job has a service target (cron.ts:453). No other hot paths, no blocking I/O on the event loop, no per-request allocations of note. No database work in this repo — no index or migration concerns.

Verdict

approved — zero Critical findings. Six Suggestions and six Information notes, none blocking; S1 (the unbound api.ts header seam) is the one I'd most like to see closed before this lands, since it is the only thing standing between a future refactor and the silent loss of both safety properties this PR is built around.

(Informational verdict from the review bot — the GitHub approval is a separate human action.)

[jwfing]

LGTM - approved.