feat(runs): make the parked-session idle timeout configurable, null to never close

[AGmakonts]

The default is unchanged. resources.sessionIdleMinutes ships at 15 — exactly the IDLE_TIMEOUT_MS constant it replaces — so the zero-config path closes an idle waiting session after the same 15 minutes with the same lifecycle message. This adds a knob whose default is the current behavior; the mechanism is untouched.

Why a knob at all. A session parked on a CEZ:ASK card waiting for a human — a client clarification workshop, a long review — is killed after 15 minutes of silence, with no config or env escape (IDLE_TIMEOUT_MS was a module constant). The close is recoverable via Continue/--resume, but a session that needs a click every 15 minutes through a multi-hour interactive workshop is broken in practice. null = never close on idle: the same explicit, user-chosen opt-out shape as monitoringWakeIntervalMinutes: null ("park until resumed") and memoryLimitMb: null.

The precedent is #661. cezar has already accepted that a parked session need not be bounded by this timer — #661 removed it from the monitoring branch. A waiting run holds no maxParallel slot (#347), so a long park costs nothing.

Who fires this, with null? A parked waiting session's exits are a user message and (if autonomous) the auto-nudge — precisely what an operator setting null asked for. Unlike the #661 monitoring dead end, a waiting session is visible in the cockpit with its ask card and Continue button: not a state with no exit, a state whose only exits are deliberate.

Wired end-to-end on the #810 pattern: workspace schema (workspace/config.ts, with DEFAULT_SESSION_IDLE_MINUTES written once), contract, GET/PUT /api/v1/workspace/config, a WorkspaceSemaphore.sessionIdleMinutes() accessor with the #810 absent-vs-null discipline (a settings change applies on the next arm, no restart), armIdleTimer (both turn-end arm sites route through it; nothing else read the constant), and a number-or-never control in Settings → Resources beside the wake interval. The lifecycle message names the configured minutes.

Tests, mirroring the #810 blocks at every site:

• workspace/config.test.ts — default 15 when absent, explicit null preserved, out-of-range degrades to 15 (never to never-close)
• workspace/semaphore.test.ts — absent vs explicit-null vs explicit-number, refresh re-arm
• server/workspace-api.test.ts — GET includes it; PUT round-trips null and a custom value through the file to the semaphore
• workflows/run.test.ts — a zero-config park still arms the timer; null parks with no timer and an open session; a custom value closes and the message names it
• web/.../resources-section.test.tsx — the control PUTs the right body in both directions; out-of-range values disable Save

🤖 Generated with Claude Code

[CLAassistant]

All committers have signed the CLA.

[pat-lewczuk]

🤖 om-auto-review-pr started by @pat-lewczuk at 2026-08-28T21:49:05Z. Other auto-skills will skip this PR until the lock is released.

[pat-lewczuk]

🔍 Code Review: feat(runs): make the parked-session idle timeout configurable, null to never close

🎯 Summary

This PR turns the fixed IDLE_TIMEOUT_MS = 15 * 60_000 constant in packages/cezar/src/workflows/run.ts into the workspace setting resources.sessionIdleMinutes, wired end to end: the workspace schema (workspace/config.ts) with a single DEFAULT_SESSION_IDLE_MINUTES = 15, the WorkspaceSemaphore.sessionIdleMinutes() accessor with the absent-vs-explicit-null discipline the monitoring wake cadence established, the GET/PUT /api/v1/workspace/config contract on both the contract package and the server, the armIdleTimer read site, and a number-or-never control in Settings → Resources that mirrors the monitoring wake-up field slot for slot. I reviewed all sixteen changed files against the PR head 74f2a00, plus the surrounding lifecycle code the knob reaches into (armIdleTimer, pump, acquireRepoRoot, busySlots).

The craft here is high and the parts that usually go wrong in this repo went right. The zero-config path really is unchanged — the schema default is exactly the constant it replaced, the lifecycle message still reads 15m for the default, and config.test.ts pins that an out-of-range value degrades to 15 rather than to never-close, which is the trap .catch(null) would have been. The absent-vs-null distinction is respected at all three layers (schema .default(), semaphore accessor, and the !== undefined guard in the PUT handler), so an operator's explicit "never" is never silently re-defaulted. The claim in the PR body that a waiting run holds no maxParallel slot checks out: busySlots() (packages/cezar/src/workflows/run.ts:810) subtracts waiting from the active count, so a long park costs no concurrency. Test coverage mirrors the #810 blocks at every site.

What holds the merge is not the default path but the null path and the runtime plumbing around it. Two things are missing: a settings change does not reach an already-parked session, unlike its sibling setting in the very same file; and null removes the only bound on a parked run's lifetime without naming the second thing that bound was load-bearing for — the exclusive repo-root lease. Both are the failure mode AGENTS.md § "Changing a mechanism that already works" was written about, so I am raising them rather than waving them through.

Verdict

❌ request changes — two major findings drive this: the new setting is read only at arm time with no reconcile on semaphore.refresh() (so changing it, in either direction, does not affect a session that is already parked — including the "I am parked right now and want it to stop closing" case the PR exists to solve), and the null option leaves a parked worktree: false run holding the exclusive repo-root lease forever, which today's 15-minute close is what releases. Neither is a default-path regression, and both are fixable with a small, local change; everything else in the diff is ready.

🧪 Validation Gate

Command	Status	Notes
npm run typecheck	✅ PASS	All four projects (contract, api-client, server, web) compile clean.
npm test	✅ PASS	6170 of 6173 tests green across 326 files. Three files flaked in the full parallel run on my (heavily loaded) machine — repo-git.test.tsx, task-changes.test.tsx and accounts-defaults.test.tsx, all with expired async assertions rather than real mismatches — and a different set flaked on an earlier pass. Every one of them passes when re-run in isolation (44/44 for those three, 206/206 for the earlier batch), and none touches the code this PR changes; packages/cezar/src/workflows/run.test.ts alone is 84/84 green, including the new suite. An earlier batch of failures was an artifact of my sandbox's TMPDIR living inside a git repository, which breaks the "outside a git repo" assertions; with TMPDIR=/tmp they pass. I am therefore reading the suite as green rather than raising a blocker I cannot reproduce.
npm run test:unit	✅ PASS	36/36 node:test core-module tests pass.
npm run build	✅ PASS	Server and web builds succeed and the tarball gate is happy: check:pack ok — 475 files, 85 under web/dist (shell + assets present).
npm run test:package	✅ PASS	15/15 packaged-CLI tests pass.

Findings

⚠️ Major

1. packages/cezar/src/workflows/run.ts:3387 — a settings change never reaches an already-parked session, and there is no way back from null for one.

armIdleTimer reads this.semaphore.sessionIdleMinutes() at arm time and nothing re-arms afterwards. The sibling setting does not work this way: monitoringWakeIntervalMinutes is reconciled live, because pump() calls reconcileMonitoringWakeTimers() (run.ts:851, run.ts:3408) and WorkspaceSemaphore.refresh() — which PUT /api/v1/workspace/config invokes on every resources write (packages/cezar/src/server/server.ts:2910) — pumps every registered manager. So the monitoring cadence applies to sessions that are already parked, while this new knob does not.

The consequences run in both directions, and the second one has no recovery path:

• A user is parked on a CEZ:ASK card, notices the session is about to be closed, and sets Never close idle sessions. The timer armed at park time is still live, so the session is closed anyway — in the exact scenario the PR's own motivation describes. The user must answer, let the run park again, and only then is the setting in force.
• The reverse is worse. With null configured, armIdleTimer returns before arming (run.ts:3388), so parked runs carry no timer at all. Setting the value back to a number does not arm one for them: those sessions stay open forever even after the operator has turned the feature off, and the only way out is to cancel each run.

Fix: add a reconcileIdleTimers() next to reconcileMonitoringWakeTimers() in pump(), iterating the runs that are actually parked for the user (in this.waiting, not in this.monitoring, with state.session?.open — plain lease-waiters are in this.waiting too and must be skipped) and calling armIdleTimer/clearIdleTimer accordingly. Guard the re-arm on an unchanged value the way armMonitoringWakeTimer does at run.ts:3421 (if (state.monitoringWakeTimer && state.monitoringWakeIntervalMinutes === minutes) return;), so an unrelated pump does not silently restart every parked session's countdown — that will need a state.sessionIdleMinutes field mirroring state.monitoringWakeIntervalMinutes. Please cover it with a test in the new configurable waiting-session idle timeout describe block: park a run under the default, refresh() the semaphore onto null, and assert the timer is gone; then the reverse.

2. packages/cezar/src/workspace/config.ts:352 and packages/web/src/routes/settings/resources-section.tsx:657 — null also un-bounds the exclusive repo-root lease, which the PR does not name.

AGENTS.md asks reviewers to name what a timer was load-bearing for before removing its bound, and #661 is the cautionary tale. The PR does that work for the maxParallel slot and is right about it (#347, confirmed in busySlots()), but the repo-root lease is the other thing the idle close ends. acquireRepoRoot (packages/cezar/src/workflows/run.ts:1531) documents it explicitly: "The lease is held for the run's whole lifetime, including the idle waiting parks between agent turns", and it is released only through dropActive (run.ts:1141) on a terminal path, or dispose(). Today the idle close is what produces that terminal path: state.session.end() resolves the await session.result the step is sitting on (run.ts:3000), the run settles, and the lease passes to the next waiter on repoRootTail.

Failure scenario: an operator sets Never, a task runs with worktree: false (or in a non-git directory, which is the README's documented in-place degradation and CODE_REVIEW.md review priority 2), it parks at waiting, and the user goes home. Every subsequent in-place run in that repository queues behind repoRootTail indefinitely, with no timeout anywhere in the chain — the pre-PR worst case was 15 minutes. The same applies, less sharply, to the agent CLI process and its worktree, which are now held for as long as the user leaves the card unanswered.

I do not think this has to block the feature, but it should not ship unnamed. Minimum fix: say it in the two places an operator reads — the sessionIdleMinutes JSDoc in workspace/config.ts (which currently names only the maxParallel and cockpit-visibility arguments) and the Settings hint, whose sub-line today says "A waiting session holds no task slot either way" and stops exactly where the interesting half begins. Stronger fix, if you want the knob to be safe by construction: keep arming the timer for a run that holds the exclusive repo-root lease, since that run is blocking others rather than only itself.

💅 Nit

• packages/cezar/src/workflows/run.ts:67 — with the constant gone, the comment that documented it is now an orphan sitting between two unrelated functions and pointing elsewhere ("see armIdleTimer"). Since armIdleTimer already carries the full rationale, this one can simply be dropped.
• AGENTS.md:30 and .ai/specs/2026-07-17-permission-modes.md:287 still name IDLE_TIMEOUT_MS as a live mechanism — the spec line states a requirement about it ("must treat a pending permission like a pending question: exempt while pending"). Nothing breaks, but a future grep IDLE_TIMEOUT_MS now lands on prose only. A one-line "now resources.sessionIdleMinutes" in the spec's edge-case bullet would keep that thread findable.
• packages/cezar/src/workflows/run.test.ts:182 — sessionIdleMinutes: 0.001 is below the schema's own floor (.int().min(1)), reachable only through the initial test seam, and the assertion pins a lifecycle string (session closed after 0.001m of inactivity) that production can never emit. It is a pragmatic way to keep the test at real-timer speed and I would not change it, but a short comment saying the value deliberately bypasses the schema would stop the next reader from treating 0.001 as supported.
• The PR body cites #661, #810 and #347 but the PR closes no issue. Per SDLC.md's intake stage, a Fixes #NNN line (or a filed issue for the workshop-session problem) would let post-merge housekeeping reconcile this automatically.

💥 Breaking Changes

• No exported/public symbol removed or renamed without a deprecation path — IDLE_TIMEOUT_MS was exported from packages/cezar/src/workflows/run.ts, but BACKWARD_COMPATIBILITY.md § 6 records that the package has no library API (exports is . → dist/index.js plus ./app-type), and a repo-wide grep finds no remaining code reference — only the two prose mentions listed as a nit above. This is an internal constant, not a protected surface.
• No function signature changed in a breaking way — armIdleTimer(runId, state) is private and unchanged; RunManager's constructor options are untouched.
• No required type field removed or narrowed — WorkspaceResourceLimits.sessionIdleMinutes is added as optional, so a loader that predates the key still satisfies the type and reads as the default.
• No HTTP route URL removed or renamed; no method changed — GET/PUT /api/v1/workspace/config keep their shapes.
• No field removed or retyped in an existing response shape — resources.sessionIdleMinutes is purely additive on the GET response and an optional key on the PUT body, matching the additive rule in BACKWARD_COMPATIBILITY.md § 2.
• No event or message name renamed or removed — the lifecycle event type is unchanged; only its human-readable message is now parameterized, and it still renders 15m on the default path.
• No CLI command or flag renamed or removed.
• No database table or column renamed or removed.
• No config key renamed and no default changed silently — ~/.cezar/config.json gains one optional key whose default reproduces the previous hard-coded behavior exactly, which config.test.ts asserts.
• Where a contract had to change: not applicable — nothing required a deprecation window.

🧪 Test Coverage

The PR adds coverage at every layer it touches, and the cases chosen are the ones that matter rather than the easy ones. workspace/config.test.ts pins the absent default (15), an explicit null surviving the load, an in-range value, and — the important one — an out-of-range value degrading to 15 rather than to never-close. workspace/semaphore.test.ts pins absent vs. explicit-null vs. explicit-number and a refresh() that flips between null and a number. server/workspace-api.test.ts round-trips both null and a custom value through the file to the semaphore and confirms GET carries the key. workflows/run.test.ts drives real parks through the dry-run mock and asserts the three behaviors that matter: a zero-config park still arms the timer, null parks with no timer and an open session, and a custom value closes with a message naming that value. resources-section.test.tsx covers both directions of the control and that out-of-range input disables Save without a PUT.

The gaps are the ones the two major findings describe, and both are behavioral rather than cosmetic:

1. No test asserts what a settings change does to a session that is already parked — in either direction. Add to the configurable waiting-session idle timeout describe block in packages/cezar/src/workflows/run.test.ts: park a run under the default, drive the semaphore to null via refresh() (the suite already builds a load-stub semaphore in semaphore.test.ts, so the seam exists), pump, and assert state.idleTimer is undefined while the session stays open; then the reverse — park under null, refresh onto a number, and assert a timer appears.
2. No test covers the lease interaction: a worktree: false run parked under sessionIdleMinutes: null and a second run queued behind it in the same repo root. Even if you resolve finding 2 by documenting rather than by code, a test that pins today's behavior would make the trade-off explicit instead of implicit.

Neither gap is about the default path, which is well covered.

[pat-lewczuk]

🤖 om-auto-review-pr — 🏷️ label rationale

• ❌ changes-requested — the code review found two major findings that need a pass from the author: the new sessionIdleMinutes value is read only when the idle timer is armed, with no reconcile on a settings change, and the null option leaves a parked worktree: false run holding the exclusive repo-root lease with no bound at all.
• ✨ feature — this PR adds a new capability (a configurable, nullable idle timeout for parked sessions) rather than fixing a defect; the previous behavior is preserved as the shipped default.
• 🧪 needs-qa — the change adds a real control to Settings → Resources and alters when a live agent session is closed, so a human should exercise both directions (a custom timeout and "never") in the cockpit before this merges.
• 🔹 priority-medium — an ordinary feature improvement: nothing is broken on main today, the zero-config path is unchanged, and the pain it addresses (a workshop session closed after 15 minutes) has a recoverable workaround in Continue/--resume.
• 🟡 risk-medium — the diff is additive at every contract boundary (a new optional workspace key, a new response field, a new PUT key) and comes with tests at each layer, which keeps it out of risk-high; it stays above risk-low because it changes run-lifecycle behavior, and CODE_REVIEW.md ranks run-lifecycle correctness as the first review priority.

[pat-lewczuk]

Thanks @AGmakonts — review found actionable items, so I'm handing this PR back to you for the next pass. When the updates are pushed, re-request review and the automation can pick it up from the latest head.

[pat-lewczuk]

🤖 om-auto-review-pr completed: CHANGES REQUESTED. Lock released.

The full review is posted above. The validation gate is green on this head (npm run typecheck, npm test, npm run test:unit, npm run build including check:pack, and npm run test:package all pass — the handful of intermittent failures I saw were load-induced flakes in my sandbox and pass in isolation, as the gate table explains). The verdict is driven by two major findings rather than by anything red: sessionIdleMinutes is read only at arm time with no reconcile on semaphore.refresh(), unlike its sibling monitoringWakeIntervalMinutes, so a settings change never reaches an already-parked session in either direction; and the null option removes the last bound on a parked run's lifetime without naming the exclusive repo-root lease that the 15-minute close is what releases today.

autofix: skipped (not my PR — re-run with --autofix to fix it here). The PR has been reassigned to @AGmakonts for the next pass.