fix: keep automation runs alive and route monitoring waits - #1377
Conversation
📝 WalkthroughWalkthroughAutomation run history is decoupled from automation definition lifetime. A DB migration rebuilds ChangesAutomation run history survives definition delete
Sequence Diagram(s)sequenceDiagram
participant Client
participant AutomationRoute as DELETE /automation/:id
participant AutomationModule as Automation.remove()
participant DB
Client->>AutomationRoute: DELETE /automation/{automationID}
AutomationRoute->>AutomationModule: remove(id)
AutomationModule->>DB: delete automation_definition row
DB-->>AutomationModule: ok
AutomationModule-->>AutomationRoute: { tombstone }
AutomationRoute-->>Client: 200 { tombstone }
note over DB: automation_run rows<br/>remain untouched<br/>(no cascade delete)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related PRs
Suggested labels
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Related to #936. Goal: - Add local Effect HttpApi coverage for ordinary Automation JSON routes while preserving the existing Hono compatibility entry point. Changes: - Extract shared automation route actions so Hono and HttpApi handlers use the same lifecycle, validation, run ledger, pause/resume, run-now, and delete behavior. - Add local HttpApi declarations and handlers for list, create, get, update, delete, runs, run, pause, and resume automation routes. - Keep delete semantics from #1377: deleting an automation stops future scheduling but preserves already-started run history. - Tighten automation HttpApi route contracts for automation IDs, run cursors, and run pagination limits so they match runtime validators. - Update route inventory harness coverage for ordinary Automation routes. Verification: - bun install --frozen-lockfile - Baseline focused tests before changes: bun test test/server/automation-routes.test.ts test/server/automation-runner.test.ts test/tool/automate-manage.test.ts test/server/route-inventory-harness.test.ts passed, 111 tests. - Review-fix RED: the automation OpenAPI contract test failed before tightening automationID, limit, and cursor schemas. - Focused tests after final changes: bun test test/server/automation-routes.test.ts test/server/automation-runner.test.ts test/tool/automate-manage.test.ts test/server/route-inventory-harness.test.ts passed, 119 tests. - GOMAXPROCS=2 bun run typecheck - git diff --check origin/dev...HEAD - Final fresh-eye review: PASS under P0/P1 gate; no P0/P1/P2/P3 findings. - PR CI passed, including unit-opencode and split Windows advisory shards. Residual risk: - Production server routing is intentionally not switched in this PR; Hono remains the compatibility entry point until the broader HttpApi cutover is ready.
Summary
automation_run.automation_id, and sync the delete OpenAPI/SDK contract.automate, while short current-turn sleeps stay capped at 60 seconds.Why
A run could interrupt itself by deleting its own Automation definition, because
Automation.remove()aborted the active controller and the run row was cascaded away with the definition. Separately, theautomaterouting contract did not make periodic monitoring obvious enough, so models could drift toward longsleeploops for tasks such as checking CI every few minutes.Related Issue
No linked issue; this was requested directly by the maintainer after discussion.
Human Review Status
Approved by @Astro-Han (fresh-eye review gate passed with no P0/P1 findings)
Review Focus
remove()no longer aborts active runs or returns a stopped run, and the HTTP/SDK contract no longer exposes a delete-time active-run 409.automation_runwithout the Automation definition foreign-key cascade while keeping the project cascade.Risk Notes
automation_run.automation_idis intentionally no longer constrained to an existing definition so run history survives definition deletion;project_idstill cascades with project deletion.How To Verify
Screenshots or Recordings
Not applicable; no visible UI changed.
Checklist
bug,enhancement,task,documentation. Type labels are author-added; the labeler bot does NOT assign them. Add the label in the GitHub UI, then tick this.app,ui,platform,harness,ci. The labeler bot assigns these on PR open based on changed paths. Confirm the bot's choice (or override if wrong), then tick this.P0,P1,P2,P3. The priority-triage bot suggests one on PR open. Confirm or override, then tick this.Pending,Approved by @<reviewer>, orNot required: <reason>(default isPending; "not required" is restricted to bot-authored low-risk PRs).dev, and my PR title and commit messages use Conventional Commits in English.Summary by CodeRabbit
Release Notes
Improvements
Documentation