feat(projects): make .cascade/setup.sh timeout configurable per project (MNG-1701)#1467
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
nhopeatall
left a comment
There was a problem hiding this comment.
Summary
APPROVE — clean, complete implementation of MNG-1701 that faithfully follows the proven maxInFlightItems / snapshotTtlMs / workerImage precedent across every layer, with correct semantics and thorough tests. CI 7/7 green.
I traced the full path rather than pattern-matching the plumbing:
Behavior is correct and backward-compatible
src/agents/shared/repository.ts:const setupWallTimeoutMs = project.setupTimeoutMs ?? 0→{ idleTimeoutMs: 0, wallTimeoutMs: setupWallTimeoutMs }. Idle stays disabled unconditionally.- Confirmed in
src/utils/repo.tsthatcreateSubprocessWatcheronly arms the wall timer whenwallTimeoutMs > 0(line 169), so0/unset = disabled. The explicit value also bypasses the?? DEFAULT_WALL_TIMEOUT_MSfallback inresolveOptions, so passing0genuinely disables rather than reverting to the 10-min default. - Unset →
?? 0→wallTimeoutMs: 0, identical to current post-#1463 behavior — additive nullable column, no behavior change for existing projects.
0 as a first-class value (the one real correctness risk) is handled everywhere
- Zod config schema + both tRPC inputs use
.nonnegative()(not.positive()). - CLI uses the
!== undefinedguard (oclif parses0fine; a truthy check would drop it) — verified by the explicit--setup-timeout-ms 0passthrough tests. - Web form relies on
"0"being truthy (numericFieldDefault(0)→"0"); empty →null. Correct.
Read path verified end-to-end (not just compile-time)
getProjectFull/listProjectsFulldodb.select().from(projects)(full row), andserializeProjectspreads...rest, sosetupTimeoutMsreaches the settings form at runtime — the saved value will actually repopulate the field.loadConfigFromDb/loadProjectConfigById→mapProjectRowalso include it, hydrating the worker'sProjectConfig.
Migration & completeness
0058is an additive nullable column; journal entry is correct (idx 58 after 57,when1793000000000 after 1792000000000, tag matches filename).- No surface was missed: the other files referencing
maxInFlightItems/snapshotTtlMs(pipeline-capacity-gate, snapshot-manager, worker-spawn-settings, backlog-check) are feature-specific consumers;setupTimeoutMs's sole consumer issetupRepository, correctly wired. - Authorization correctly mirrors
maxInFlightItems(project-admin, not superadmin-gated likeworkerImage) — appropriate for a benign operational setting.
Tests cover the meaningful cases: positive/0 wall-timeout in repository.test.ts, null→undefined + passthrough in configMapper.test.ts, CLI 0/passthrough/absent on both create+update, and a DB round-trip including the explicit-0-distinct-from-null case.
Docs (08-config-credentials.md, CHANGELOG.md) are accurate. No blocking or should-fix issues found.
🕵️ claude-code · claude-opus-4-8 · run details
Summary
Makes the wall-clock timeout of
.cascade/setup.shconfigurable per project. Adds a new nullablesetup_timeout_mscolumn onprojects, plumbed through the provenmaxInFlightItems/snapshotTtlMs/workerImageprecedent: migration → Drizzle schema →ProjectConfigSchema→configMapper→projectsRepository→ tRPCprojects.create/update→ CLI → dashboard form.Issue: https://linear.app/issue/MNG-1701
Semantics
null/unset or0→ no per-project wall timeout (rely on the global worker/watchdog container timeout, the current safety net).> 0→ that millisecond value is passed aswallTimeoutMsto therunCommandcall forsetup.sh.idleTimeoutMs: 0) regardless — the new field only controls the wall timeout.The behavior lives in one place:
setupRepository(src/agents/shared/repository.ts), whereinput.projectis the fullProjectConfig, soproject.setupTimeoutMsis already available — no new params threaded through.Changes
Storage & config plumbing
src/db/migrations/0058_add_setup_timeout_ms.sql(new) +_journal.jsonentry (idx 58,when 1793000000000,tag 0058_add_setup_timeout_ms).src/db/schema/projects.ts—setupTimeoutMs: integer('setup_timeout_ms')(nullable, no default).src/config/schema.ts—setupTimeoutMs: z.number().int().nonnegative().optional()onProjectConfigSchema(noPROJECT_DEFAULTSentry — absent/0means disabled).src/db/repositories/configMapper.ts—setupTimeoutMsadded toProjectRow(DB select shape),ProjectConfigRaw, andbuildBaseProjectFields(row.setupTimeoutMs ?? undefined).src/db/repositories/projectsRepository.ts—setupTimeoutMs?: number | nullon bothcreateProjectdata andupdateProjectupdates param types;setupTimeoutMs: rest.setupTimeoutMsin the create.values({...})(update already spreads...rest).src/api/routers/projects.ts—setupTimeoutMs: z.number().int().nonnegative().nullish()on bothcreateandupdateinputs (both already spreadinputinto the repository call).The actual behavior
src/agents/shared/repository.ts—setup.shrunCommandnow passes{ idleTimeoutMs: 0, wallTimeoutMs: project.setupTimeoutMs ?? 0 }; the explanatory comment is rewritten to describe the per-project configurable wall timeout.CLI + dashboard surfaces
src/cli/dashboard/projects/update.ts&create.ts—--setup-timeout-msflag mirroring--max-in-flight-items, passed through with the!== undefinedguard.src/cli/dashboard/projects/show.ts—Setup Timeout (ms)row in the detail output.web/src/components/projects/project-general-form.tsx—setupTimeoutMson the localProjectinterface; state wired intoisDirty,handleReset, andhandleSubmit(sendssetupTimeoutMs ? Number.parseInt(...) : null); a numberInput(min="0") labelled "Setup script timeout (ms)" with helper text in the Budget & Limits card.Tests
tests/unit/agents/shared/repository.test.ts— new tests assertingsetupTimeoutMs: 1800000→wallTimeoutMs: 1800000andsetupTimeoutMs: 0→wallTimeoutMs: 0(the existing no-override test already expectswallTimeoutMs: 0).tests/unit/db/repositories/configMapper.test.ts—setupTimeoutMs: nulladded tobaseProjectRow; cases fornull → undefinedand positive passthrough.tests/unit/cli/dashboard/projects/projects.test.ts—--setup-timeout-ms 1800000on update + create, the0passthrough case, and absent-flag cases.tests/integration/db/projectsRepository.test.ts— DB round-trip (create + update + clear), including the explicit-0-distinct-from-null case.Docs
docs/architecture/08-config-credentials.md—setupTimeoutMs?added to theProjectConfiginterface block.CHANGELOG.md— Unreleased › Added entry.Testing
npm run typecheck(src) +npx tsc -b(web) — clean.repository.test.ts,configMapper.test.ts,projects.test.ts, projects-router tests pass.tests/integration/db/projectsRepository.test.ts(50 tests incl. 4 newsetup-timeout columntests) pass — the0058migration applies cleanly via the journal-driven migrator.tests/unit/architecture-docs.test.tspasses.🤖 Generated with Claude Code
🕵️ claude-code · claude-opus-4-8 · run details