diff --git a/docs/cron-discovery.md b/docs/cron-discovery.md index c8164dcd..d93aeee8 100644 --- a/docs/cron-discovery.md +++ b/docs/cron-discovery.md @@ -71,6 +71,10 @@ records), kept distinct from the existing `ScheduledRun`. `session.stop(killMux)` at `src/session.ts:2498-2585`. - The cron does **not** kill sessions it launches (the brief wants them visible in the normal session UI); cleanup stays user-driven. + _Superseded post-review:_ recurring jobs now default to + `autoClosePreviousSession: true` — the previous run's still-open session is + closed via `cleanupSession` when the next run fires (see + `docs/cron-guide.md` §8); opt out per job for fully user-driven cleanup. ## 6. How session state is stored / 7. Existing persistence diff --git a/docs/cron-guide.md b/docs/cron-guide.md index 023c27d7..8b56a391 100644 --- a/docs/cron-guide.md +++ b/docs/cron-guide.md @@ -67,13 +67,13 @@ curl -s "$API/api/cron/jobs//runs" | jq ## 2. Concepts -| Term | Meaning | -|------|---------| -| **Cron job** (`CronJob`) | A saved, named definition: what agent to launch, where, with what prompt, on what schedule. | -| **Run** (`CronJobRun`) | One execution of a job — a history record with a status and a link to the session it created. | -| **Schedule type** | How fire times are computed: `once`, `interval`, `daily`, or `weekly`. | +| Term | Meaning | +| -------------------------- | ------------------------------------------------------------------------------------------------ | +| **Cron job** (`CronJob`) | A saved, named definition: what agent to launch, where, with what prompt, on what schedule. | +| **Run** (`CronJobRun`) | One execution of a job — a history record with a status and a link to the session it created. | +| **Schedule type** | How fire times are computed: `once`, `interval`, `daily`, or `weekly`. | | **Next run** (`nextRunAt`) | Server-computed epoch-ms of the next fire. `null` when the job is disabled or has no future run. | -| **Due tick** | A background loop (every 30s) that launches any enabled job whose `nextRunAt` has passed. | +| **Due tick** | A background loop (every 30s) that launches any enabled job whose `nextRunAt` has passed. | A job is essentially a **trigger + persistence + history layer on top of the existing session primitives**. When a job fires, the cron service does exactly @@ -88,25 +88,26 @@ prompt. It does **not** reimplement any tmux/PTY logic. These map 1:1 to `CronJobSchema` (`src/web/schemas.ts`) and the `CronJob` type (`src/types/cron.ts`). -| Field | Required | Values / limits | Notes | -|-------|----------|-----------------|-------| -| `name` | ✅ | 1–200 chars | Display name; also used as the created session's name. | -| `agentType` | ✅ | `claude` \| `shell` \| `opencode` \| `codex` \| `gemini` | Reuses Codeman's `SessionMode`. `shell` = a plain terminal. | -| `workingDir` | ✅ | valid path (allowlist-validated) | Must exist and be a directory **at fire time** or the run fails. | -| `launchCommand` | — | ≤ 2000 chars | Only meaningful for `shell` mode (custom launch command). | -| `promptMode` | ✅ | `inline_text` \| `prompt_file_path` | See §5. | -| `promptText` | conditional | ≤ 100000 chars | Required when `promptMode = inline_text`. | -| `promptFilePath` | conditional | valid path | Required when `promptMode = prompt_file_path`. Confined to `workingDir` (see §5). | -| `inputMode` | ✅ | `paste` \| `typed` | How the prompt is delivered. See §6. | -| `scheduleType` | ✅ | `once` \| `interval` \| `daily` \| `weekly` | See §4. | -| `runAt` | conditional | epoch-ms (positive int) | Required for `once`. | -| `intervalMinutes` | conditional | 1–525600 (≤ 1 year) | Required for `interval`. | -| `dailyTime` | conditional | `HH:MM` (24h) | Required for `daily`. Server-local time. | -| `weeklyDays` | conditional | array of 1–7 ints, each 0–6 (0 = Sunday) | Required for `weekly`. | -| `weeklyTime` | conditional | `HH:MM` (24h) | Required for `weekly`. Server-local time. | -| `enabled` | ✅ | boolean | Disabled jobs never auto-fire (but **Run Now** still works). | -| `notes` | — | ≤ 2000 chars | Free-form. | -| `concurrencyPolicy` | ✅ | `warn_only` \| `skip_if_same_agent_running` | Applies to **automatic** runs only. See §7. | +| Field | Required | Values / limits | Notes | +| -------------------------- | ----------- | -------------------------------------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `name` | ✅ | 1–200 chars | Display name; also used as the created session's name. | +| `agentType` | ✅ | `claude` \| `shell` \| `opencode` \| `codex` \| `gemini` | Reuses Codeman's `SessionMode`. `shell` = a plain terminal. | +| `workingDir` | ✅ | valid path (allowlist-validated) | Validated at **create/update** (must exist, be a directory, and not resolve into a blocked tree — `/etc`, `/root`, `/proc`, `/sys`, `/dev`, or `/` itself) and again **at fire time**. | +| `launchCommand` | — | ≤ 2000 chars, single line | `shell` mode only: sent as the **first input line** once the shell is up, before the prompt. Ignored for other agent types. | +| `promptMode` | ✅ | `inline_text` \| `prompt_file_path` | See §5. | +| `promptText` | conditional | ≤ 100000 chars, **single line** | Required when `promptMode = inline_text`. Newlines are rejected (see §6). | +| `promptFilePath` | conditional | valid path | Required when `promptMode = prompt_file_path`. Confined to `workingDir` (see §5). | +| `inputMode` | ✅ | `paste` \| `typed` | How the prompt is delivered. See §6. | +| `scheduleType` | ✅ | `once` \| `interval` \| `daily` \| `weekly` | See §4. | +| `runAt` | conditional | epoch-ms (positive int) | Required for `once`. | +| `intervalMinutes` | conditional | 1–525600 (≤ 1 year) | Required for `interval`. | +| `dailyTime` | conditional | `HH:MM` (24h) | Required for `daily`. Server-local time. | +| `weeklyDays` | conditional | array of 1–7 ints, each 0–6 (0 = Sunday) | Required for `weekly`. | +| `weeklyTime` | conditional | `HH:MM` (24h) | Required for `weekly`. Server-local time. | +| `enabled` | ✅ | boolean | Disabled jobs never auto-fire (but **Run Now** still works). | +| `notes` | — | ≤ 2000 chars | Free-form. | +| `concurrencyPolicy` | ✅ | `warn_only` \| `skip_if_same_agent_running` | Applies to **automatic** runs only. See §7. | +| `autoClosePreviousSession` | — | boolean (default **true**) | Recurring schedules only (ignored for `once`): when the next run fires, the still-open session created by this job's **previous** run is closed first via the normal cleanup path. See §8. | **Cross-field validation** (`refineCronJob` in `schemas.ts`): the conditional fields above are enforced by a Zod `superRefine` on create. A missing dependent @@ -128,24 +129,28 @@ Next-run math lives in `src/cron/cron-time.ts` (pure, unit-tested in timezone** (v0.1 decision). ### `once` + - Fires a single time at the absolute `runAt` epoch-ms. - A **missed** one-time job (server was down at `runAt`) **still fires once** on the next tick — `computeNextRunAt` returns `runAt` even if it's in the past, until the job has fired. - After firing, the job **self-disables**: `completedOnce = true`, `enabled = - false`, `nextRunAt = null`. +false`, `nextRunAt = null`. ### `interval` + - Fires every `intervalMinutes`, computed as `fireTime + intervalMinutes`. - ⚠️ **Drift**: the next run re-anchors to the actual fire time, not to an ideal cadence — a slow tick or restart shifts subsequent runs slightly later. This is an accepted limitation. ### `daily` + - Fires at `dailyTime` (`HH:MM`) every day, server-local. - If today's time has already passed, the next run is tomorrow at that time. ### `weekly` + - Fires at `weeklyTime` on each weekday in `weeklyDays` (0 = Sunday … 6 = Saturday), server-local. - The next run is the soonest upcoming matching weekday/time within the next 7 @@ -156,24 +161,36 @@ timezone** (v0.1 decision). ## 5. Prompt source (`promptMode`) ### `inline_text` + The prompt is the literal `promptText`. Simplest option. ### `prompt_file_path` + The prompt is read from a file at fire time. **This path is security-hardened** because a job config is attacker-controllable and the file's contents are injected into an agent session (an exfiltration sink over SSE/terminal). `resolveSafePromptPath()` enforces, in order: -1. **`realpath` resolution** — symlinks are resolved to their true target. -2. **Blocklist** (defense-in-depth) — sensitive trees (`/etc`, `/root`, known - secret locations) are rejected. -3. **Allowlist (primary gate)** — the resolved path **must live inside the job's - `workingDir`** (`validateSessionFilePath`). A symlink escaping the workspace - fails here. -4. **Regular-file check** — directories, FIFOs, and `/dev/*` character devices +1. **`realpath` resolution** — symlinks are resolved to their true target, for + the prompt file **and for `workingDir` itself**. +2. **`workingDir` is not a trust boundary** — because it is user-supplied, the + resolved `workingDir` is itself rejected if it is `/` or resolves into a + blocked tree (`/etc`, `/root`, operator extras) or a pseudo-filesystem + (`/proc`, `/sys`, `/dev`). This closes the `workingDir: '/proc'` + + `promptFilePath: '/proc/self/environ'` env-exfil trick. The same rule is + enforced earlier, at job create/update. +3. **Blocklist** (defense-in-depth) — sensitive trees (`/etc`, `/root`, + `/proc`, `/sys`, `/dev`, known secret locations) are rejected for the + resolved prompt file. +4. **Allowlist (primary gate)** — the resolved path **must live inside the job's + (resolved) `workingDir`** (`validateSessionFilePath`). A symlink escaping the + workspace fails here. +5. **Regular-file check** — directories, FIFOs, and `/dev/*` character devices are rejected (they would hang or OOM an unbounded read). -5. **Size cap** — files larger than **1 MiB** (`MAX_PROMPT_FILE_BYTES`) are +6. **Size cap** — files larger than **1 MiB** (`MAX_PROMPT_FILE_BYTES`) are rejected. +7. **Single-line check** — after trailing newlines are stripped, the file + content must be a single line (see §6). If any check fails, the run is recorded as **`failed`** with the reason; no session is created. @@ -185,15 +202,18 @@ session is created. Once the CLI is ready (see §8), the prompt is written to the session with a trailing carriage return: -| Mode | Mechanism | Use when | -|------|-----------|----------| +| Mode | Mechanism | Use when | +| ------- | --------------------------------------------------------------- | ------------------------------------------------ | | `typed` | `session.writeViaMux()` — tmux `send-keys -l` (literal) + Enter | Default; behaves like a human typing the prompt. | -| `paste` | `session.write()` — writes directly to the PTY/mux | Bulk paste-style delivery. | +| `paste` | `session.write()` — writes directly to the PTY/mux | Bulk paste-style delivery. | -> ⚠️ **Single-line only.** Like all programmatic input in Codeman, a multi-line -> `promptText` is delivered broken (Ink-based TUIs treat the first newline as -> submit). Keep prompts to a single line, or put multi-line instructions in a -> file the agent reads itself. +> ⚠️ **Single-line only — enforced.** Like all programmatic input in Codeman, +> multi-line delivery would be silently corrupted (Ink-based TUIs treat a +> newline as submit; typed mode fuses lines). So newlines are **rejected**: the +> schema and the form refuse a multi-line `promptText`, and at fire time a +> prompt file whose content is multi-line (after stripping trailing newlines) +> fails the run with a clear `errorMessage`. Put multi-line instructions in a +> file the agent is told to read itself (e.g. "read TASKS.md and do it"). --- @@ -202,13 +222,20 @@ trailing carriage return: `concurrencyPolicy` governs what happens when a **scheduled** run is due and sessions of the same `agentType` already exist: -| Policy | Behavior | -|--------|----------| -| `warn_only` | Always launch. (The count is surfaced but not blocking.) | -| `skip_if_same_agent_running` | If ≥ 1 session of that mode is active, **skip** this fire — record a `skipped` run and advance the schedule without launching. | +| Policy | Behavior | +| ---------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------ | +| `warn_only` | Always launch. (The count is surfaced but not blocking.) | +| `skip_if_same_agent_running` | If ≥ 1 **other, live** session of that mode is active, **skip** this fire — record a `skipped` run and (for recurring schedules) advance the schedule without launching. | Notes on `skip_if_same_agent_running`: -- It counts **all** sessions of that mode, not just sessions this job created. + +- Only **live** sessions block: a tab whose CLI already exited (status + `stopped`/`error`) does not count. +- Sessions created by **this job's own previous runs never block it** — + otherwise a recurring job would deadlock on the session it created last time + and fire exactly once. +- A skipped **`once`** job is **not consumed**: it stays armed and retries on + the next tick until the blocking session goes away, then fires its single run. - A skip is **not** a run: it sets `lastStatus = 'skipped'` but does **not** advance `lastRunAt`. - Consecutive skips are **coalesced** — a perpetually-skipped interval job writes @@ -227,27 +254,41 @@ Sequence in `CronService.launch()`: 1. A `CronJobRun` is created with status **`created`** and broadcast (`cron:runCreated`). -2. The prompt is resolved (inline or file). Failure → **`failed`**. +2. The prompt is resolved (inline or file, single-line enforced). Failure → + **`failed`**. 3. `workingDir` is checked (`statSync().isDirectory()`). Missing/not-a-dir → **`failed`**. -4. The global session cap is checked (`MAX_CONCURRENT_SESSIONS = 50`). At cap → +4. **Auto-close previous session** (recurring schedules, unless + `autoClosePreviousSession: false`): any still-open session created by this + job's previous runs is closed via the normal session-cleanup path. +5. The global session cap is checked (`MAX_CONCURRENT_SESSIONS = 50`). At cap → **`failed`**. -5. A `Session` is created **with `useMux: true`** (so it runs inside tmux), +6. A `Session` is created **with `useMux: true`** (so it runs inside tmux), registered, listeners attached, and started via `startInteractive()` (`startShell()` for `shell` mode). Model/claudeMode come from global config. Run status → **`session_started`**. -6. **Readiness wait** (async, non-blocking): for non-shell agents the service +7. **Readiness wait** (async, non-blocking): for non-shell agents the service polls the terminal buffer up to **60 × 500ms** for a `❯` prompt or the string - `tokens`, then settles **2000ms** (`CRON_READY_SETTLE_MS`). Shell mode just - waits 1000ms. -7. The prompt is delivered (`typed`/`paste`, trailing `\r`). Run status → - **`prompt_sent`**; `finishedAt` stamped. Delivery failure → **`failed`**. + `tokens`, then settles **2000ms** (`CRON_READY_SETTLE_MS`). Shell mode waits + 1000ms, then sends the optional `launchCommand` as the first input line + (+1000ms settle). +8. The prompt is delivered (`typed`/`paste`, trailing `\r`). Run status → + **`prompt_sent`**; `finishedAt` stamped. Delivery failure (e.g. the mux + session is gone) → **`failed`**. The created session is a **normal, persistent interactive session** — it appears as its own tab and keeps running after the prompt is sent. The run's `createdSessionUrl` is a deep link (`/?session=`); the UI focuses it automatically after **Run Now**. +> ⚠️ **Session-cap math if you disable auto-close.** With +> `autoClosePreviousSession: false`, nothing ever closes the sessions a +> recurring job creates — an interval job every 30 min creates 48 tabs/day and +> hits the global 50-session cap in ~25 hours (sooner with existing tabs), after +> which **every** fire of **every** job fails with "Maximum concurrent sessions +> reached" until you delete tabs by hand. Leave auto-close on for unattended +> recurring jobs, or clean up sessions yourself. + ### The background tick `tickDueJobs()` runs every **30s** (`CRON_TICK_INTERVAL`, registered in @@ -266,13 +307,13 @@ automatically after **Run Now**. Each job keeps a history of `CronJobRun` records. Statuses (`CronJobRunStatus`): -| Status | Meaning | -|--------|---------| -| `created` | Run record created; prompt/session not yet started. | -| `session_started` | Session launched successfully. | -| `prompt_sent` | Prompt delivered — the happy-path terminal state. | -| `failed` | Something went wrong (see `errorMessage`). | -| `skipped` | A scheduled fire was skipped by `skip_if_same_agent_running`. | +| Status | Meaning | +| ----------------- | ------------------------------------------------------------- | +| `created` | Run record created; prompt/session not yet started. | +| `session_started` | Session launched successfully. | +| `prompt_sent` | Prompt delivered — the happy-path terminal state. | +| `failed` | Something went wrong (see `errorMessage`). | +| `skipped` | A scheduled fire was skipped by `skip_if_same_agent_running`. | Each run also records `triggerType` (`scheduled` or `manual_run_now`), `sessionId`/`sessionName`, timestamps, and `createdSessionUrl`. @@ -287,17 +328,17 @@ oldest are pruned first. Deleting a job also deletes its run records. All responses use the standard `ApiResponse` envelope (`{success, data}` / `{success, error, errorCode}`). `/api/v1/*` is a stable alias. -| Method | Endpoint | Body | Returns | -|--------|----------|------|---------| -| `GET` | `/api/cron/jobs` | — | `CronJob[]` | -| `POST` | `/api/cron/jobs` | `CronJobSchema` | `{ job }` | -| `GET` | `/api/cron/jobs/:id` | — | `CronJob` (404 if missing) | -| `PUT` | `/api/cron/jobs/:id` | partial `CronJob` | `{ job }` (400 if merge invalid) | -| `DELETE` | `/api/cron/jobs/:id` | — | `{}` | -| `PUT` | `/api/cron/jobs/:id/enabled` | `{ enabled: boolean }` | `{ job }` | -| `POST` | `/api/cron/jobs/:id/run` | — | `{ run, activeAgents }` | -| `GET` | `/api/cron/jobs/:id/runs` | — | `CronJobRun[]` (newest first) | -| `GET` | `/api/cron/runs` | — | all `CronJobRun[]` (newest first) | +| Method | Endpoint | Body | Returns | +| -------- | ---------------------------- | ---------------------- | --------------------------------- | +| `GET` | `/api/cron/jobs` | — | `CronJob[]` | +| `POST` | `/api/cron/jobs` | `CronJobSchema` | `{ job }` | +| `GET` | `/api/cron/jobs/:id` | — | `CronJob` (404 if missing) | +| `PUT` | `/api/cron/jobs/:id` | partial `CronJob` | `{ job }` (400 if merge invalid) | +| `DELETE` | `/api/cron/jobs/:id` | — | `{}` | +| `PUT` | `/api/cron/jobs/:id/enabled` | `{ enabled: boolean }` | `{ job }` | +| `POST` | `/api/cron/jobs/:id/run` | — | `{ run, activeAgents }` | +| `GET` | `/api/cron/jobs/:id/runs` | — | `CronJobRun[]` (newest first) | +| `GET` | `/api/cron/runs` | — | all `CronJobRun[]` (newest first) | --- @@ -305,12 +346,12 @@ All responses use the standard `ApiResponse` envelope (`{success, data}` / Emitted on `/api/events`, mirrored in `SSE_EVENTS` (`constants.js`): -| Event | Payload | When | -|-------|---------|------| -| `cron:jobsChanged` | `{ jobs }` | Any job created / updated / enabled / status change. | -| `cron:jobDeleted` | `{ id }` | A job was deleted. | -| `cron:runCreated` | `CronJobRun` | A run (incl. skips) started. | -| `cron:runUpdated` | `CronJobRun` | A run advanced state (`session_started` / `prompt_sent` / `failed`). | +| Event | Payload | When | +| ------------------ | ------------ | -------------------------------------------------------------------- | +| `cron:jobsChanged` | `{ jobs }` | Any job created / updated / enabled / status change. | +| `cron:jobDeleted` | `{ id }` | A job was deleted. | +| `cron:runCreated` | `CronJobRun` | A run (incl. skips) started. | +| `cron:runUpdated` | `CronJobRun` | A run advanced state (`session_started` / `prompt_sent` / `failed`). | --- @@ -328,18 +369,19 @@ boot. Sessions the jobs create persist through the normal session-recovery path. ## 13. Limits & constants -| Constant | Value | Source | -|----------|-------|--------| -| Due-tick interval | 30s | `CRON_TICK_INTERVAL` (`config/server-timing.ts`) | -| Readiness poll | 60 × 500ms | `CRON_READY_MAX_ATTEMPTS` | -| Readiness settle | 2000ms | `CRON_READY_SETTLE_MS` | -| Run-history cap (global) | 500 | `MAX_CRON_RUN_HISTORY` (`config/map-limits.ts`) | -| Concurrent-session cap | 50 | `MAX_CONCURRENT_SESSIONS` | -| Prompt-file size cap | 1 MiB | `MAX_PROMPT_FILE_BYTES` (`cron-service.ts`) | -| `name` length | 1–200 | `CronJobSchema` | -| `promptText` length | ≤ 100000 | `CronJobSchema` | -| `intervalMinutes` | 1–525600 | `CronJobSchema` | -| `weeklyDays` | 1–7 entries, each 0–6 | `CronJobSchema` | +| Constant | Value | Source | +| ------------------------ | --------------------- | ------------------------------------------------ | +| Due-tick interval | 30s | `CRON_TICK_INTERVAL` (`config/server-timing.ts`) | +| Readiness poll | 60 × 500ms | `CRON_READY_MAX_ATTEMPTS` | +| Readiness settle | 2000ms | `CRON_READY_SETTLE_MS` | +| Run-history cap (global) | 500 | `MAX_CRON_RUN_HISTORY` (`config/map-limits.ts`) | +| Saved-jobs cap | 100 | `MAX_CRON_JOBS` (`config/map-limits.ts`) | +| Concurrent-session cap | 50 | `MAX_CONCURRENT_SESSIONS` | +| Prompt-file size cap | 1 MiB | `MAX_PROMPT_FILE_BYTES` (`cron-service.ts`) | +| `name` length | 1–200 | `CronJobSchema` | +| `promptText` length | ≤ 100000 | `CronJobSchema` | +| `intervalMinutes` | 1–525600 | `CronJobSchema` | +| `weeklyDays` | 1–7 entries, each 0–6 | `CronJobSchema` | --- @@ -349,8 +391,9 @@ boot. Sessions the jobs create persist through the normal session-recovery path. host's local time; there is no per-job timezone. - **Interval drift** — `interval` re-anchors to the actual fire time; long-running intervals slowly shift. -- **Single-line prompts** — multi-line `promptText` is delivered broken; use a - prompt file the agent reads itself for multi-line instructions. +- **Single-line prompts** — multi-line prompts are rejected (schema, form, and + at fire time for prompt files); tell the agent to read a file itself for + multi-line instructions. - **`runNow` / tick race** — a manual Run Now firing at the same instant as a scheduled tick is theoretically possible; benign (you may get two sessions). - **`{enabled:true}` on a dead `once` job** — re-enabling a fired one-time job @@ -361,14 +404,15 @@ boot. Sessions the jobs create persist through the normal session-recovery path. ## 15. Troubleshooting -| Symptom | Likely cause | Fix | -|---------|--------------|-----| -| Job never fires | Disabled, or `nextRunAt: null` | Check **Enabled**; verify the schedule fields are complete. | -| Run shows `failed` immediately | Bad `workingDir`, prompt-file rejected, or session cap hit | Read `errorMessage` on the run; confirm the dir exists and the prompt file is inside it and < 1 MiB. | -| Run shows `skipped` | `skip_if_same_agent_running` + an active same-type session | Switch to `warn_only`, or wait for the other session to end. | -| Prompt looks truncated | Multi-line `promptText` | Use a single line, or a prompt file. | -| Wrong fire time | Timezone assumption | Times are **server-local** — check the host clock/TZ. | -| One-time job won't re-fire | `completedOnce` set | Edit the schedule (any real schedule change re-arms it). | +| Symptom | Likely cause | Fix | +| ------------------------------ | ----------------------------------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------- | +| Job never fires | Disabled, or `nextRunAt: null` | Check **Enabled**; verify the schedule fields are complete. | +| Run shows `failed` immediately | Bad `workingDir`, prompt-file rejected, or session cap hit | Read `errorMessage` on the run; confirm the dir exists and the prompt file is inside it and < 1 MiB. | +| Run shows `skipped` | `skip_if_same_agent_running` + another live same-type session (this job's own sessions and dead tabs don't count) | Switch to `warn_only`, or wait for the other session to end. | +| Run fails with "single line" | Multi-line prompt text / prompt file | Keep the prompt to one line; point the agent at a file to read for long instructions. | +| Sessions pile up between runs | `autoClosePreviousSession: false` | Re-enable auto-close, or delete old tabs before the 50-session cap bites (see §8). | +| Wrong fire time | Timezone assumption | Times are **server-local** — check the host clock/TZ. | +| One-time job won't re-fire | `completedOnce` set | Edit the schedule (any real schedule change re-arms it). | --- diff --git a/src/config/map-limits.ts b/src/config/map-limits.ts index b8fc5529..c3e246c2 100644 --- a/src/config/map-limits.ts +++ b/src/config/map-limits.ts @@ -50,6 +50,12 @@ export const MAX_TODOS_PER_SESSION = 500; */ export const MAX_CRON_RUN_HISTORY = 500; +/** + * Maximum saved cron jobs. Jobs persist to state.json, so an unbounded count + * would grow it without limit; creation past the cap is rejected with 400. + */ +export const MAX_CRON_JOBS = 100; + // ============================================================================ // Pending Tool Calls Limits // ============================================================================ diff --git a/src/cron/cron-input.ts b/src/cron/cron-input.ts index c0783d6c..885f993f 100644 --- a/src/cron/cron-input.ts +++ b/src/cron/cron-input.ts @@ -27,4 +27,6 @@ export interface CronJobInput { enabled: boolean; notes?: string; concurrencyPolicy: ConcurrencyPolicy; + /** Default true. Ignored for 'once' schedules. */ + autoClosePreviousSession?: boolean; } diff --git a/src/cron/cron-service.ts b/src/cron/cron-service.ts index e274ee2b..e1fb64cc 100644 --- a/src/cron/cron-service.ts +++ b/src/cron/cron-service.ts @@ -14,9 +14,13 @@ import { Session } from '../session.js'; import { SseEvent } from '../web/sse-events.js'; import { CronJobSchema } from '../web/schemas.js'; import { getErrorMessage, createErrorResponse, ApiErrorCode } from '../types/api.js'; -import { MAX_CONCURRENT_SESSIONS, MAX_CRON_RUN_HISTORY } from '../config/map-limits.js'; +import { MAX_CONCURRENT_SESSIONS, MAX_CRON_JOBS, MAX_CRON_RUN_HISTORY } from '../config/map-limits.js'; import { CRON_READY_MAX_ATTEMPTS, CRON_READY_SETTLE_MS } from '../config/server-timing.js'; -import { isBlockedAttachmentPath, loadAttachmentGuardConfig } from '../config/attachment-guard.js'; +import { + DEFAULT_BLOCKED_TREES, + isBlockedAttachmentPath, + loadAttachmentGuardConfig, +} from '../config/attachment-guard.js'; import { validateSessionFilePath } from '../web/route-helpers.js'; import { computeNextRunAt, dueKeyFor } from './cron-time.js'; import type { SessionPort, EventPort, ConfigPort, InfraPort } from '../web/ports/index.js'; @@ -31,6 +35,20 @@ const delay = (ms: number): Promise => new Promise((r) => setTimeout(r, ms /** Hard ceiling on a prompt-file read (defends against unbounded-read DoS). */ const MAX_PROMPT_FILE_BYTES = 1024 * 1024; +/** + * Pseudo-filesystem trees a cron job may never touch, ON TOP of the shared + * attachment blocklist. `/proc` in particular defeats the workingDir + * confinement trick (`workingDir: '/proc'` + `promptFilePath: + * '/proc/self/environ'` would read the SERVER's own environment). + */ +const CRON_PSEUDO_FS_TREES: readonly string[] = ['/proc', '/sys', '/dev']; + +/** Sync blocklist for the create/update workingDir gate (no settings extras). */ +const CRON_WORKING_DIR_BLOCKED_TREES: readonly string[] = [...DEFAULT_BLOCKED_TREES, ...CRON_PSEUDO_FS_TREES]; + +/** Prompt delivery is single-line only (writeViaMux/Ink constraint). */ +const HAS_NEWLINE = /[\r\n]/; + /** Order-insensitive equality for the weekly-days arrays. */ function sameDays(a: number[] | undefined, b: number[] | undefined): boolean { const x = [...(a ?? [])].sort((p, q) => p - q); @@ -61,16 +79,40 @@ export class CronService { return filtered.sort((a, b) => b.startedAt - a.startedAt); } - /** Number of active sessions of a given agent type (for the multi-session warning). */ - countActiveAgents(agentType: string): number { + /** + * Number of LIVE sessions of a given agent type (for the multi-session + * warning and the skip_if_same_agent_running policy). Sessions whose CLI has + * exited (`stopped`/`error` — the tab is still open but nothing is running) + * don't count. When `excludeJobId` is given, sessions created by that job's + * own runs are also excluded — otherwise a recurring job with the skip + * policy would deadlock on its own previous (never-closed) session and fire + * exactly once, forever skipping after that. + */ + countActiveAgents(agentType: string, excludeJobId?: string): number { + const ownSessionIds = excludeJobId + ? new Set( + this.listRuns(excludeJobId) + .map((r) => r.sessionId) + .filter((id): id is string => id !== null) + ) + : null; let n = 0; - for (const s of this.deps.sessions.values()) if (s.mode === agentType) n++; + for (const [id, s] of this.deps.sessions.entries()) { + if (s.mode !== agentType) continue; + if (s.status === 'stopped' || s.status === 'error') continue; + if (ownSessionIds?.has(id)) continue; + n++; + } return n; } // ──────────────────────────── Mutations ─────────────────────────── createJob(input: CronJobInput): CronJob { + if (Object.keys(this.store.getCronJobs()).length >= MAX_CRON_JOBS) { + throw this.badRequest(`Maximum number of cron jobs (${MAX_CRON_JOBS}) reached`); + } + this.assertValidWorkingDir(input.workingDir); const now = Date.now(); const job: CronJob = { id: uuidv4(), @@ -91,6 +133,7 @@ export class CronService { enabled: input.enabled, notes: input.notes, concurrencyPolicy: input.concurrencyPolicy, + autoClosePreviousSession: input.autoClosePreviousSession ?? true, createdAt: now, updatedAt: now, lastRunAt: null, @@ -141,12 +184,9 @@ export class CronService { // (e.g. switching to `once` without a `runAt` → a dead `nextRunAt:null`). const check = CronJobSchema.safeParse(updated); if (!check.success) { - const msg = check.error.issues[0]?.message ?? 'Invalid cron job update'; - throw Object.assign(new Error(msg), { - statusCode: 400, - body: createErrorResponse(ApiErrorCode.INVALID_INPUT, msg), - }); + throw this.badRequest(check.error.issues[0]?.message ?? 'Invalid cron job update'); } + if (patch.workingDir !== undefined) this.assertValidWorkingDir(patch.workingDir); updated.nextRunAt = updated.enabled ? computeNextRunAt(updated, now) : null; this.store.setCronJob(updated.id, updated); @@ -199,12 +239,21 @@ export class CronService { continue; } - // Optional concurrency policy for AUTOMATIC runs. - if (job.concurrencyPolicy === 'skip_if_same_agent_running' && this.countActiveAgents(job.agentType) > 0) { - job.lastDueKey = key; + // Optional concurrency policy for AUTOMATIC runs. Only LIVE sessions + // block, and this job's own previous sessions never do (see + // countActiveAgents) — otherwise a recurring job would deadlock on the + // session it created last time. + if (job.concurrencyPolicy === 'skip_if_same_agent_running' && this.countActiveAgents(job.agentType, job.id) > 0) { // Record the skip so the job's run history isn't silently empty when it // keeps getting skipped (otherwise it looks like the job never ran). this.recordSkippedRun(job); + if (job.scheduleType === 'once') { + // A skipped one-time job is NOT consumed: leave nextRunAt armed (and + // the due key unconsumed) so the next tick retries once the blocking + // session goes away. + continue; + } + job.lastDueKey = key; this.advanceAfterFire(job, now); continue; } @@ -279,6 +328,14 @@ export class CronService { return this.failRun(job, run, 'workingDir does not exist'); } + // Recurring jobs: close the still-open session created by this job's + // previous run before launching the next (default ON, opt-out via + // autoClosePreviousSession:false) — otherwise an unattended interval/daily + // job accumulates a new tab per fire until the global session cap. + if (job.scheduleType !== 'once' && job.autoClosePreviousSession !== false) { + await this.closePreviousRunSessions(job, run.id); + } + // Respect the global session cap. if (this.deps.sessions.size >= MAX_CONCURRENT_SESSIONS) { return this.failRun(job, run, `Maximum concurrent sessions (${MAX_CONCURRENT_SESSIONS}) reached`); @@ -331,13 +388,31 @@ export class CronService { return run; } + /** + * Resolves the prompt text and enforces the single-line constraint: prompt + * delivery rides writeViaMux/PTY writes where a newline is Enter, so a + * multi-line prompt would be silently corrupted (typed mode fuses lines, + * paste mode submits the first line and dribbles the rest in as separate + * messages). Rather than mangle an unattended agent's instructions, fail the + * run with a clear error. A prompt FILE may end with trailing newline(s) + * (every editor writes one) — those are stripped before the check. + */ private async resolvePrompt(job: CronJob): Promise { if (job.promptMode === 'prompt_file_path') { if (!job.promptFilePath) throw new Error('prompt file path is empty'); const safePath = await this.resolveSafePromptPath(job.promptFilePath, job.workingDir); - return readFile(safePath, 'utf-8'); + const content = (await readFile(safePath, 'utf-8')).replace(/[\r\n]+$/, ''); + if (HAS_NEWLINE.test(content)) { + throw new Error('prompt file must contain a single line — multi-line prompts are not supported'); + } + return content; } - return job.promptText ?? ''; + const text = job.promptText ?? ''; + if (HAS_NEWLINE.test(text)) { + // Schema-rejected since this check was added; guards legacy persisted jobs. + throw new Error('promptText must be a single line — multi-line prompts are not supported'); + } + return text; } /** @@ -364,14 +439,29 @@ export class CronService { throw new Error('prompt file path could not be resolved'); } - // Defense-in-depth blocklist (secret locations, /etc, /root). + // workingDir is USER-CONTROLLED, so it is not a trust boundary by itself: + // realpath-resolve it (a symlinked workspace must not defeat containment) + // and reject blocked/pseudo-fs trees — otherwise workingDir '/proc' would + // make '/proc/self/environ' pass the containment check below. + let realWorkingDir: string; + try { + realWorkingDir = realpathSync(workingDir); + } catch { + throw new Error('job working directory could not be resolved'); + } const guard = await loadAttachmentGuardConfig(); - if (isBlockedAttachmentPath(resolved, guard.blockedTrees)) { + const blockedTrees = [...guard.blockedTrees, ...CRON_PSEUDO_FS_TREES]; + if (realWorkingDir === '/' || isBlockedAttachmentPath(realWorkingDir, blockedTrees)) { + throw new Error('job working directory is blocked'); + } + + // Defense-in-depth blocklist (secret locations, /etc, /root, pseudo-fs). + if (isBlockedAttachmentPath(resolved, blockedTrees)) { throw new Error('prompt file path is blocked'); } // Primary gate: the prompt file must live inside the job's workspace. - if (!validateSessionFilePath(workingDir, resolved)) { + if (!validateSessionFilePath(realWorkingDir, resolved)) { throw new Error('prompt file path must be inside the job working directory'); } @@ -402,15 +492,33 @@ export class CronService { await delay(CRON_READY_SETTLE_MS); } else { await delay(1000); + // Shell mode: deliver the optional custom launch command as the + // first input line (single-line, schema-enforced), then give it a + // moment to start before the prompt follows. + if (job.launchCommand) { + const shell = this.deps.sessions.get(sessionId); + if (!shell) return; + const sent = await shell.writeViaMux(`${job.launchCommand}\r`); + if (!sent) { + this.failRun(job, run, 'Failed to send launch command: mux write failed'); + return; + } + await delay(1000); + } } const s = this.deps.sessions.get(sessionId); if (!s) return; try { const payload = prompt.endsWith('\r') ? prompt : `${prompt}\r`; + let delivered = true; if (job.inputMode === 'paste') { s.write(payload); } else { - await s.writeViaMux(payload); + delivered = await s.writeViaMux(payload); + } + if (!delivered) { + this.failRun(job, run, 'Failed to send prompt: mux write failed'); + return; } run.status = 'prompt_sent'; run.finishedAt = Date.now(); @@ -425,6 +533,46 @@ export class CronService { }); } + /** 400-shaped error for route handlers (mirrors parseBody's error contract). */ + private badRequest(msg: string): Error { + return Object.assign(new Error(msg), { + statusCode: 400, + body: createErrorResponse(ApiErrorCode.INVALID_INPUT, msg), + }); + } + + /** + * Create/update gate for a job's workingDir: must exist, be a directory, and + * not resolve into a blocked or pseudo-filesystem tree (nor the fs root). + * The user-supplied workingDir doubles as the prompt-file confinement root, + * so an unrestricted value would defeat that boundary (e.g. '/proc'). + */ + private assertValidWorkingDir(workingDir: string): void { + let real: string; + try { + real = realpathSync(workingDir); + } catch { + throw this.badRequest('workingDir does not exist'); + } + if (!statSync(real).isDirectory()) throw this.badRequest('workingDir is not a directory'); + if (real === '/' || isBlockedAttachmentPath(real, CRON_WORKING_DIR_BLOCKED_TREES)) { + throw this.badRequest('workingDir is not allowed (blocked or pseudo-filesystem tree)'); + } + } + + /** Close still-open sessions created by this job's previous runs (normal cleanup path). */ + private async closePreviousRunSessions(job: CronJob, currentRunId: string): Promise { + for (const prev of this.listRuns(job.id)) { + if (prev.id === currentRunId || !prev.sessionId) continue; + if (!this.deps.sessions.has(prev.sessionId)) continue; + try { + await this.deps.cleanupSession(prev.sessionId, true, 'cron: superseded by the next run of this job'); + } catch (err) { + console.error(`[cron] failed to auto-close previous session ${prev.sessionId}:`, getErrorMessage(err)); + } + } + } + private failRun(job: CronJob, run: CronJobRun, message: string): CronJobRun { run.status = 'failed'; run.errorMessage = message; diff --git a/src/types/cron.ts b/src/types/cron.ts index 18e96a99..05bfdf13 100644 --- a/src/types/cron.ts +++ b/src/types/cron.ts @@ -63,6 +63,13 @@ export interface CronJob { notes?: string; /** Applies to automatic (scheduled) runs only. Manual Run Now always warns client-side. */ concurrencyPolicy: ConcurrencyPolicy; + /** + * Close the still-open session created by this job's previous run before the + * next run launches (via the normal session-cleanup path), so unattended + * recurring jobs don't accumulate tabs until the global session cap. + * Default true. Ignored for 'once' schedules. + */ + autoClosePreviousSession?: boolean; // ── Bookkeeping (server-maintained) ───────────────────────────────────── createdAt: number; diff --git a/src/web/public/cron-ui.js b/src/web/public/cron-ui.js index 3bbfa29b..1ddf7eb1 100644 --- a/src/web/public/cron-ui.js +++ b/src/web/public/cron-ui.js @@ -126,6 +126,7 @@ Object.assign(CodemanApp.prototype, { document.getElementById('schName').value = job ? job.name || '' : ''; document.getElementById('schAgentType').value = job ? job.agentType || 'claude' : 'claude'; document.getElementById('schWorkingDir').value = job ? job.workingDir || '' : ''; + document.getElementById('schLaunchCommand').value = job ? job.launchCommand || '' : ''; document.getElementById('schPromptMode').value = job ? job.promptMode || 'inline_text' : 'inline_text'; document.getElementById('schPromptText').value = job ? job.promptText || '' : ''; document.getElementById('schPromptFilePath').value = job ? job.promptFilePath || '' : ''; @@ -140,9 +141,11 @@ Object.assign(CodemanApp.prototype, { cb.checked = weekly.includes(Number(cb.value)); }); document.getElementById('schConcurrencyPolicy').value = job ? job.concurrencyPolicy || 'warn_only' : 'warn_only'; + document.getElementById('schAutoClosePrev').checked = job ? job.autoClosePreviousSession !== false : true; document.getElementById('schEnabled').checked = job ? !!job.enabled : true; document.getElementById('schNotes').value = job ? job.notes || '' : ''; + this.onCronAgentTypeChange(); this.onCronPromptModeChange(); this.onCronScheduleTypeChange(); form.classList.remove('hidden'); @@ -158,6 +161,12 @@ Object.assign(CodemanApp.prototype, { if (form) form.classList.add('hidden'); }, + onCronAgentTypeChange() { + // Launch command is only meaningful for shell mode (first input line). + const isShell = document.getElementById('schAgentType').value === 'shell'; + document.getElementById('schLaunchCommandRow').classList.toggle('hidden', !isShell); + }, + onCronPromptModeChange() { const mode = document.getElementById('schPromptMode').value; document.getElementById('schPromptTextRow').classList.toggle('hidden', mode !== 'inline_text'); @@ -191,11 +200,18 @@ Object.assign(CodemanApp.prototype, { inputMode: document.getElementById('schInputMode').value, scheduleType: t, concurrencyPolicy: document.getElementById('schConcurrencyPolicy').value, + autoClosePreviousSession: document.getElementById('schAutoClosePrev').checked, enabled: document.getElementById('schEnabled').checked, notes: document.getElementById('schNotes').value.trim() || undefined, }; - if (promptMode === 'inline_text') body.promptText = document.getElementById('schPromptText').value; - else body.promptFilePath = document.getElementById('schPromptFilePath').value.trim(); + // Always sent for shell (an emptied field must clear a saved command on edit). + if (body.agentType === 'shell') body.launchCommand = document.getElementById('schLaunchCommand').value.trim(); + if (promptMode === 'inline_text') { + // Prompt delivery is single-line only; trailing newlines are harmless, strip them. + body.promptText = document.getElementById('schPromptText').value.replace(/[\r\n]+$/, ''); + } else { + body.promptFilePath = document.getElementById('schPromptFilePath').value.trim(); + } if (t === 'once') { const v = document.getElementById('schRunAt').value; @@ -225,6 +241,10 @@ Object.assign(CodemanApp.prototype, { errEl.textContent = 'Working directory is required.'; return; } + if (body.promptText !== undefined && /[\r\n]/.test(body.promptText)) { + errEl.textContent = 'Prompt must be a single line — multi-line prompts are not supported.'; + return; + } const id = document.getElementById('schJobId').value; const res = id ? await this._apiPut(`/api/cron/jobs/${id}`, body) : await this._apiPost('/api/cron/jobs', body); if (!res || !res.ok) { @@ -279,9 +299,13 @@ Object.assign(CodemanApp.prototype, { }, _countActiveAgents(agentType) { + // Mirrors the server's countActiveAgents: only LIVE sessions count — a + // tab whose CLI already exited (stopped/error) doesn't block anything. if (!this.sessions) return 0; let n = 0; - for (const s of this.sessions.values()) if (s && s.mode === agentType) n++; + for (const s of this.sessions.values()) { + if (s && s.mode === agentType && s.status !== 'stopped' && s.status !== 'error') n++; + } return n; }, diff --git a/src/web/public/index.html b/src/web/public/index.html index 9ddf7aab..fe209301 100644 --- a/src/web/public/index.html +++ b/src/web/public/index.html @@ -594,7 +594,7 @@
- @@ -603,6 +603,7 @@
+
+
+ +
diff --git a/src/web/routes/cron-routes.ts b/src/web/routes/cron-routes.ts index b411c5c5..9aae6de2 100644 --- a/src/web/routes/cron-routes.ts +++ b/src/web/routes/cron-routes.ts @@ -20,7 +20,9 @@ export function registerCronRoutes(app: FastifyInstance, ctx: CronPort): void { }); app.post('/api/cron/jobs', async (req) => { - const body = parseBody(CronJobSchema, req.body, 'Invalid cron job'); + // No custom errorMessage: surface the schema's field-specific messages + // (e.g. "runAt is required for a one-time schedule"). + const body = parseBody(CronJobSchema, req.body); return { job: ctx.cron.createJob(body) }; }); @@ -33,7 +35,7 @@ export function registerCronRoutes(app: FastifyInstance, ctx: CronPort): void { app.put('/api/cron/jobs/:id', async (req) => { const { id } = req.params as { id: string }; - const body = parseBody(CronJobUpdateSchema, req.body, 'Invalid cron job update'); + const body = parseBody(CronJobUpdateSchema, req.body); const job = ctx.cron.updateJob(id, body); if (!job) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Cron job not found'); return { job }; @@ -62,7 +64,7 @@ export function registerCronRoutes(app: FastifyInstance, ctx: CronPort): void { const job = ctx.cron.getJob(id); if (!job) return createErrorResponse(ApiErrorCode.NOT_FOUND, 'Cron job not found'); const run = await ctx.cron.runNow(id); - return { run, activeAgents: ctx.cron.countActiveAgents(job.agentType) }; + return { run, activeAgents: ctx.cron.countActiveAgents(job.agentType, job.id) }; }); // ── Run history ────────────────────────────────────────────────────────── diff --git a/src/web/schemas.ts b/src/web/schemas.ts index 4544c676..543d4689 100644 --- a/src/web/schemas.ts +++ b/src/web/schemas.ts @@ -579,14 +579,21 @@ export const ScheduledRunSchema = z.object({ /** 'HH:MM' 24-hour time. */ const hhmmSchema = z.string().regex(/^([01]?\d|2[0-3]):[0-5]\d$/, 'Time must be HH:MM (24-hour)'); +/** Prompt delivery is single-line only (writeViaMux/Ink constraint) — reject newlines outright. */ +const noNewlines = (v: string) => !/[\r\n]/.test(v); + /** Shared field shape for creating/updating a scheduled job. */ const CronJobBaseSchema = z.object({ name: z.string().min(1).max(200), agentType: z.enum(['claude', 'shell', 'opencode', 'codex', 'gemini']), workingDir: safePathSchema, - launchCommand: z.string().max(2000).optional(), + launchCommand: z.string().max(2000).refine(noNewlines, 'launchCommand must be a single line').optional(), promptMode: z.enum(['inline_text', 'prompt_file_path']), - promptText: z.string().max(100000).optional(), + promptText: z + .string() + .max(100000) + .refine(noNewlines, 'promptText must be a single line (multi-line prompts are not supported)') + .optional(), promptFilePath: safePathSchema.optional(), inputMode: z.enum(['paste', 'typed']), scheduleType: z.enum(['once', 'interval', 'daily', 'weekly']), @@ -598,6 +605,7 @@ const CronJobBaseSchema = z.object({ enabled: z.boolean(), notes: z.string().max(2000).optional(), concurrencyPolicy: z.enum(['warn_only', 'skip_if_same_agent_running']), + autoClosePreviousSession: z.boolean().optional(), }); /** Cross-field validation: required fields depend on promptMode + scheduleType. */ diff --git a/test/cron-service.test.ts b/test/cron-service.test.ts index af83bed5..f8db095e 100644 --- a/test/cron-service.test.ts +++ b/test/cron-service.test.ts @@ -3,25 +3,32 @@ * state machine of the cron. The pure next-run math lives in * cron-time.test.ts; this exercises the service that sits on top of it. * - * Launch attempts are steered down the "workingDir does not exist" failure path - * so no real Session/tmux objects are constructed — we assert the scheduling - * state machine (due detection, dedup guard, schedule advance, once-completion, - * concurrency skip, run-history recording), not the session layer it reuses. + * Launch attempts are steered down the "Session launch failed" path (the mock + * deps lack the session-construction config getters) so no real Session/tmux + * objects are constructed — we assert the scheduling state machine (due + * detection, dedup guard, schedule advance, once-completion, concurrency skip, + * auto-close, run-history recording), not the session layer it reuses. * * Port: N/A (no HTTP server). */ import { describe, it, expect, beforeEach, vi } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, symlinkSync } from 'node:fs'; +import { existsSync, mkdtempSync, mkdirSync, writeFileSync, symlinkSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { CronService, type CronDeps } from '../src/cron/cron-service.js'; +import { CronJobSchema } from '../src/web/schemas.js'; +import { MAX_CRON_JOBS } from '../src/config/map-limits.js'; import type { CronJob, CronJobRun } from '../src/types/cron.js'; import type { CronJobInput } from '../src/cron/cron-input.js'; const MISSING_DIR = '/nonexistent-codeman-cron-test-dir'; +/** A real dir: createJob/updateJob validate workingDir existence up front. */ +const VALID_DIR = mkdtempSync(join(tmpdir(), 'codeman-cron-wd-')); const flush = (): Promise => new Promise((r) => setImmediate(r)); +type FakeSession = { mode: string; status?: string }; + function makeStore() { const jobs: Record = {}; const runs: Record = {}; @@ -45,22 +52,26 @@ function makeStore() { }; } -function makeService(sessions = new Map()) { +function makeService(sessions = new Map()) { const store = makeStore(); const broadcast = vi.fn(); + const cleanupSession = vi.fn(async (id: string) => { + sessions.delete(id); + }); const deps = { store, broadcast, sessions, + cleanupSession, } as unknown as CronDeps; - return { service: new CronService(deps), store, broadcast, sessions }; + return { service: new CronService(deps), store, broadcast, sessions, cleanupSession }; } function mkInput(overrides: Partial = {}): CronJobInput { return { name: 'job', agentType: 'claude', - workingDir: MISSING_DIR, + workingDir: VALID_DIR, promptMode: 'inline_text', promptText: 'hello', inputMode: 'typed', @@ -254,7 +265,7 @@ describe('CronService', () => { expect(after.nextRunAt).toBeNull(); const runs = svc.service.listRuns(job.id); expect(runs.length).toBe(1); - expect(runs[0].status).toBe('failed'); // workingDir missing → fails before session launch + expect(runs[0].status).toBe('failed'); // mock deps can't construct a Session → fails at launch // A second tick must not re-fire it. await svc.service.tickDueJobs(Date.now()); @@ -348,13 +359,29 @@ describe('CronService', () => { expect(svc.sessions.size).toBe(0); }); - it('blocks /proc/self/environ (server-process env exfil) via workspace confinement', async () => { + it('blocks /proc/self/environ (server-process env exfil) via the pseudo-fs blocklist', async () => { const run = await svc.service.runNow(fileJob('/proc/self/environ', ws).id); expect(run!.status).toBe('failed'); expect(run!.errorMessage).toMatch(/Prompt error/i); - expect(run!.errorMessage).toMatch(/inside the job working directory/i); + expect(run!.errorMessage).toMatch(/block/i); }); + it.skipIf(!existsSync('/proc/self/environ'))( + 'blocks the workingDir=/proc + /proc/self/environ confinement bypass at fire time', + async () => { + // Create-time validation rejects a /proc workingDir, so simulate a + // legacy/hand-edited job by mutating the stored record directly. + const job = fileJob('/proc/self/environ', ws); + const stored = svc.store.getCronJob(job.id)!; + stored.workingDir = '/proc'; + svc.store.setCronJob(stored.id, stored); + const run = await svc.service.runNow(job.id); + expect(run!.status).toBe('failed'); + expect(run!.errorMessage).toMatch(/Prompt error/i); + expect(run!.errorMessage).toMatch(/blocked/i); + } + ); + it('blocks a regular file that lives OUTSIDE the job workspace', async () => { const outside = mkdtempSync(join(tmpdir(), 'codeman-cron-outside-')); const file = join(outside, 'prompt.md'); @@ -420,4 +447,188 @@ describe('CronService', () => { expect(await svc.service.runNow('nope')).toBeNull(); }); }); + + describe('workingDir validation (create/update)', () => { + it('rejects a nonexistent workingDir at create', () => { + expect(() => svc.service.createJob(mkInput({ workingDir: MISSING_DIR }))).toThrow(/does not exist/); + }); + + it('rejects blocked trees and the filesystem root at create', () => { + expect(() => svc.service.createJob(mkInput({ workingDir: '/etc' }))).toThrow(/not allowed/); + expect(() => svc.service.createJob(mkInput({ workingDir: '/' }))).toThrow(/not allowed/); + }); + + it('rejects an invalid workingDir on update and leaves the job untouched', () => { + const job = svc.service.createJob(mkInput()); + expect(() => svc.service.updateJob(job.id, { workingDir: MISSING_DIR })).toThrow(/does not exist/); + expect(svc.service.getJob(job.id)!.workingDir).toBe(VALID_DIR); + }); + }); + + describe('single-line prompt enforcement', () => { + it('schema rejects a multi-line promptText and launchCommand', () => { + expect(CronJobSchema.safeParse(mkInput({ promptText: 'a\nb' })).success).toBe(false); + expect(CronJobSchema.safeParse(mkInput({ agentType: 'shell', launchCommand: 'a\nb' })).success).toBe(false); + expect(CronJobSchema.safeParse(mkInput()).success).toBe(true); + }); + + it('fails the run when a legacy job carries a multi-line promptText', async () => { + const job = svc.service.createJob(mkInput()); + const stored = svc.store.getCronJob(job.id)!; + stored.promptText = 'line one\nline two'; + svc.store.setCronJob(stored.id, stored); + const run = await svc.service.runNow(job.id); + expect(run!.status).toBe('failed'); + expect(run!.errorMessage).toMatch(/single line/i); + }); + + it('fails the run when the prompt file is multi-line', async () => { + const ws = mkdtempSync(join(tmpdir(), 'codeman-cron-ml-')); + const file = join(ws, 'prompt.md'); + writeFileSync(file, 'line one\nline two\n'); + const job = svc.service.createJob( + mkInput({ promptMode: 'prompt_file_path', promptFilePath: file, promptText: undefined, workingDir: ws }) + ); + const run = await svc.service.runNow(job.id); + expect(run!.status).toBe('failed'); + expect(run!.errorMessage).toMatch(/single line/i); + }); + + it('tolerates trailing newlines in a prompt file (every editor writes one)', async () => { + const ws = mkdtempSync(join(tmpdir(), 'codeman-cron-tn-')); + const file = join(ws, 'prompt.md'); + writeFileSync(file, 'do the thing\n'); + const job = svc.service.createJob( + mkInput({ promptMode: 'prompt_file_path', promptFilePath: file, promptText: undefined, workingDir: ws }) + ); + const run = await svc.service.runNow(job.id); + // Past prompt resolution; fails only at the mock-incomplete session step. + expect(run!.errorMessage).toMatch(/Session launch failed/i); + }); + }); + + describe('concurrency-skip session filtering', () => { + const prevRun = (jobId: string, sessionId: string): CronJobRun => ({ + id: `r-${sessionId}`, + cronJobId: jobId, + sessionId, + sessionName: 'job', + startedAt: Date.now() - 60_000, + finishedAt: Date.now() - 59_000, + status: 'prompt_sent', + triggerType: 'scheduled', + createdSessionUrl: null, + }); + + it('does not skip when the only same-mode sessions are stopped/error (dead tabs)', async () => { + const sessions = new Map([ + ['dead1', { mode: 'claude', status: 'stopped' }], + ['dead2', { mode: 'claude', status: 'error' }], + ]); + const local = makeService(sessions); + const job = local.service.createJob( + mkInput({ concurrencyPolicy: 'skip_if_same_agent_running', intervalMinutes: 10 }) + ); + await local.service.tickDueJobs(job.nextRunAt! + 1000); + await flush(); + const runs = local.service.listRuns(job.id); + expect(runs.length).toBe(1); + expect(runs[0].status).toBe('failed'); // launched (mock session step), NOT skipped + }); + + it("does not skip on the job's own previous-run session (no self-deadlock)", async () => { + const sessions = new Map([['own-1', { mode: 'claude' }]]); + const local = makeService(sessions); + const job = local.service.createJob( + mkInput({ concurrencyPolicy: 'skip_if_same_agent_running', intervalMinutes: 10 }) + ); + local.store.setCronJobRun('r-own-1', prevRun(job.id, 'own-1')); + await local.service.tickDueJobs(job.nextRunAt! + 1000); + await flush(); + expect(local.service.listRuns(job.id)[0].status).not.toBe('skipped'); + }); + + it('does not consume a skipped once-job — it stays armed and fires when unblocked', async () => { + const sessions = new Map([['s1', { mode: 'claude' }]]); + const local = makeService(sessions); + const runAt = Date.now() - 1000; + const job = local.service.createJob( + mkInput({ + scheduleType: 'once', + runAt, + intervalMinutes: undefined, + concurrencyPolicy: 'skip_if_same_agent_running', + }) + ); + + await local.service.tickDueJobs(Date.now()); + await flush(); + let after = local.service.getJob(job.id)!; + expect(after.lastStatus).toBe('skipped'); + expect(after.completedOnce).toBeFalsy(); + expect(after.enabled).toBe(true); + expect(after.nextRunAt).toBe(runAt); // still armed + + // The blocking session goes away → the next tick fires the single run. + sessions.delete('s1'); + await local.service.tickDueJobs(Date.now()); + await flush(); + after = local.service.getJob(job.id)!; + expect(after.completedOnce).toBe(true); + expect(after.enabled).toBe(false); + expect(local.service.listRuns(job.id).some((r) => r.status === 'failed')).toBe(true); // it launched + }); + }); + + describe('autoClosePreviousSession', () => { + const prevRun = (jobId: string, sessionId: string): CronJobRun => ({ + id: `r-${sessionId}`, + cronJobId: jobId, + sessionId, + sessionName: 'job', + startedAt: Date.now() - 60_000, + finishedAt: Date.now() - 59_000, + status: 'prompt_sent', + triggerType: 'scheduled', + createdSessionUrl: null, + }); + + it("closes the previous run's still-open session before launching (default on)", async () => { + const sessions = new Map([['prev-1', { mode: 'claude' }]]); + const local = makeService(sessions); + const job = local.service.createJob(mkInput({ intervalMinutes: 10 })); + local.store.setCronJobRun('r-prev-1', prevRun(job.id, 'prev-1')); + await local.service.runNow(job.id); + expect(local.cleanupSession).toHaveBeenCalledWith('prev-1', true, expect.stringContaining('cron')); + expect(sessions.has('prev-1')).toBe(false); + }); + + it('does not close anything when autoClosePreviousSession is false', async () => { + const sessions = new Map([['prev-1', { mode: 'claude' }]]); + const local = makeService(sessions); + const job = local.service.createJob(mkInput({ intervalMinutes: 10, autoClosePreviousSession: false })); + local.store.setCronJobRun('r-prev-1', prevRun(job.id, 'prev-1')); + await local.service.runNow(job.id); + expect(local.cleanupSession).not.toHaveBeenCalled(); + expect(sessions.has('prev-1')).toBe(true); + }); + + it('never auto-closes for a once schedule', async () => { + const sessions = new Map([['prev-1', { mode: 'claude' }]]); + const local = makeService(sessions); + const job = local.service.createJob( + mkInput({ scheduleType: 'once', runAt: Date.now() + 3_600_000, intervalMinutes: undefined }) + ); + local.store.setCronJobRun('r-prev-1', prevRun(job.id, 'prev-1')); + await local.service.runNow(job.id); + expect(local.cleanupSession).not.toHaveBeenCalled(); + }); + }); + + describe('job-count cap', () => { + it(`rejects creation beyond MAX_CRON_JOBS (${MAX_CRON_JOBS})`, () => { + for (let i = 0; i < MAX_CRON_JOBS; i++) svc.service.createJob(mkInput({ name: `j${i}` })); + expect(() => svc.service.createJob(mkInput({ name: 'overflow' }))).toThrow(/Maximum number of cron jobs/); + }); + }); });