fix(workflows): make end-to-end runs survive without mid-run caps or stale state

[chrisleekr]

Description

Fix four classes of bug found while running real bot:ship cascades on issue #11 end-to-end. The cascade was losing progress mid-run (turn cap, wall-clock cap), miscounting concurrency at the process boundary, and producing one-line tracking comments that hid all evidence the agent produced. Triage was a regex keyword classifier — now it actually reads the repo and returns a structured verdict that halts the cascade when the issue is invalid.

While diagnosing the in-flight bookkeeping drift, we also replaced the time-threshold stale-row reaper with a heartbeat-based one so that crashed/killed orchestrators and daemons release their workflow_runs rows immediately rather than after a 60-min timeout.

Before

flowchart LR
  Webhook["webhook router"]:::own --> RecordRow["recordWorkflowExecution<br/>increments activeCount"]:::bug
  RecordRow --> Enqueue["enqueue job"]:::own
  Enqueue --> Daemon["daemon claims"]:::own
  Daemon --> RecordRow2["daemon recordWorkflowExecution<br/>increments LOCAL activeCount<br/>other process"]:::bug
  Daemon --> Result["job result"]:::own
  Result --> Decrement["decrement activeCount<br/>orchestrator side<br/>lost daemon increment"]:::bug
  Decrement --> Drift["activeCount &lt; 0 warnings"]:::bug

  Triage["triage handler"]:::bug --> Regex["regex keyword match"]:::bug
  Regex --> Cascade["always recommends plan"]:::bug

  Plan["plan handler"]:::bug --> Cap30["maxTurns: 30 hard-coded"]:::bug
  Implement["implement handler"]:::bug --> Cap50["executor floor maxTurns ?? 50"]:::bug
  Cap50 --> Lost["10-min wall-clock cap<br/>progress lost"]:::bug

  Comment["tracking comment"]:::bug --> OneLine["one-line success/failure"]:::bug

  classDef own fill:#0b3d2e,stroke:#3ddc97,color:#ffffff
  classDef bug fill:#5a1d1d,stroke:#ff6b6b,color:#ffffff

Loading

After

flowchart LR
  Webhook["webhook router"]:::own --> Enqueue["enqueue job<br/>no activeCount mutation"]:::keep
  Enqueue --> Daemon["daemon claims"]:::own
  Daemon --> Accept["orchestrator handleAccept<br/>increments activeCount once"]:::keep
  Accept --> Pipeline["pipeline runs end-to-end<br/>maxTurns optional<br/>timeout 60min"]:::keep
  Pipeline --> Result["handleResult<br/>decrements activeCount once"]:::keep

  Triage["triage handler"]:::keep --> Sdk["clones repo<br/>runs Agent SDK<br/>writes TRIAGE.md + verdict.json"]:::keep
  Sdk --> Verdict["valid=false halts cascade<br/>valid=true continues"]:::keep

  Comment["tracking comment"]:::keep --> Rich["full PLAN/IMPLEMENT/REVIEW/TRIAGE markdown<br/>+ cost / turns / duration footer"]:::keep

  Reaper["liveness reaper"]:::keep --> Heartbeat["Valkey orchestrator/daemon alive keys<br/>flip orphaned workflow_runs to failed"]:::keep

  classDef own fill:#0b3d2e,stroke:#3ddc97,color:#ffffff
  classDef keep fill:#1d3a5a,stroke:#67a9ff,color:#ffffff

Loading

Changes

Concurrency bookkeeping

• Centralise incrementActiveCount / decrementActiveCount ownership in src/orchestrator/connection-handler.ts (handleAccept increments, handleResult and error paths decrement).
• Remove all activeCount mutation from src/webhook/router.ts, src/workflows/dispatcher.ts, src/workflows/handlers/ship.ts, src/workflows/orchestrator.ts, and src/workflows/execution-row.ts (the daemon-side helper was double-counting across processes).
• Counter now tracks in-flight at daemons, not enqueued — admission gate (isAtCapacity) still works correctly.

maxTurns made truly optional

• src/config.ts — defaultMaxTurns is .optional() (no implicit 30).
• src/shared/ws-messages.ts — maxTurns field is optional.
• src/orchestrator/job-dispatcher.ts, src/daemon/job-executor.ts, src/core/executor.ts — conditionally spread maxTurns rather than coalesce to 50.
• src/workflows/handlers/plan.ts — drop hard-coded maxTurns: 30.

Wall-clock timeout

• AGENT_TIMEOUT_MS and STALE_EXECUTION_THRESHOLD_MS defaults raised from 600_000ms to 3_600_000ms (60 min). Real implement runs were hitting the 10-min cap mid-task.

Code-aware triage rewrite

• src/workflows/handlers/triage.ts — replaces regex keyword classifier with an Agent-SDK-driven validator. Clones the repo, runs the agent with Read / Grep / Glob / Bash / Write, asks for TRIAGE.md + TRIAGE_VERDICT.json (zod-validated). Returns succeeded (valid=true) or failed (valid=false) — the latter halts the bot:ship cascade.

Tracking comment richness

• plan.ts, implement.ts, review.ts, triage.ts — embed the full structured agent report (PLAN.md / IMPLEMENT.md / REVIEW.md / TRIAGE.md) plus a cost · turns · duration metadata footer.
• src/types.ts — ExecutionResult.capturedFiles?: Record<string, string>.
• src/core/pipeline.ts — RunPipelineOverrides.captureFiles?: string[]; new readCapturedFiles helper reads requested files before workspace cleanup.

PR detection robustness

• implement.ts findRecentOpenedPr — filter on pr.user?.type === "Bot" instead of a hard-coded slug. Dev installs publish as chrisleekr-bot-dev[bot], prod as chrisleekr-bot[bot] — the slug check produced false negatives.

Heartbeat-based liveness reaper (new)

• src/db/migrations/006_workflow_runs_ownership.sql — adds owner_kind / owner_id to workflow_runs (nullable; pre-existing rows ignored by reaper).
• src/orchestrator/instance-id.ts, src/daemon/daemon-id.ts — stable IDs (k8s pod name in prod, host+pid in dev/tests).
• src/orchestrator/instance-liveness.ts — orchestrator publishes orchestrator:{id}:alive with 60s TTL, refreshed every 20s.
• src/orchestrator/liveness-reaper.ts — scans Valkey alive keys; flips workflow_runs rows whose owning process no longer heartbeats to failed.
• src/orchestrator/queue-worker.ts — pulls the queue independently of WS connect events so a slow connect can't starve the queue.
• src/orchestrator/valkey-cleanup.ts — drops orphaned per-instance processing lists and stale daemon active-job sets on startup.
• src/workflows/execution-row.ts — extracted recordWorkflowExecution (ownership-aware row insert).

Tests

• test/workflows/handlers/triage.test.ts — full rewrite; 6 cases (valid, invalid, agent error, missing markdown, malformed JSON, wrong target). void mock.module(...) + spread real node:fs/promises to avoid cross-file mock pollution.
• New: test/daemon/, test/orchestrator/instance-liveness.test.ts, test/orchestrator/liveness-reaper.test.ts, test/orchestrator/queue-worker.test.ts, test/orchestrator/valkey-cleanup.test.ts.
• Updated existing tests for new ownership/queue-worker/registry contracts.

Docs / config

• .env.example, docs/CONFIGURATION.md, docs/ARCHITECTURE.md — reflect new defaults, remove the TRIAGE_MAXTURNS_* ladder, document the liveness reaper.
• Dockerfile.daemon, Dockerfile.orchestrator — minor surface updates to match new module layout.

Related Issues

• Validates against issue refactor(testing): extract shared test factories to eliminate duplicated makeCtx/silentLog boilerplate across 7 test files #11 end-to-end (3 live runs); produced PR refactor(testing): extract shared test factories #54 (now reviewed by bot:review).

Testing

• I have tested these changes locally (3 live bot:ship E2E runs against issue refactor(testing): extract shared test factories to eliminate duplicated makeCtx/silentLog boilerplate across 7 test files #11)
• I have added/updated tests as needed
• All existing tests pass — some upstream test imports on main are broken (finalizeTrackingComment, getExecutionState, requireValkeyClient, recoverStaleExecutions); not introduced by this PR

[coderabbitai]

Warning

Rate limit exceeded

@chrisleekr has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 33 minutes and 46 seconds before requesting another review.

Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 33 minutes and 46 seconds.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 44512e13-5925-4d24-b7f3-211ba1a8a9a8

📥 Commits

Reviewing files that changed from the base of the PR and between 3d08e69 and 817c885.

📒 Files selected for processing (54)

• .env.example
• .github/workflows/docker-build.yml
• Dockerfile.daemon
• Dockerfile.orchestrator
• docs/ARCHITECTURE.md
• docs/BOT-WORKFLOWS.md
• docs/CONFIGURATION.md
• package.json
• release.config.mjs
• src/app.ts
• src/config.ts
• src/core/executor.ts
• src/core/pipeline.ts
• src/daemon/daemon-id.ts
• src/daemon/job-executor.ts
• src/daemon/main.ts
• src/daemon/workflow-executor.ts
• src/daemon/ws-client.ts
• src/db/migrations/006_workflow_runs_ownership.sql
• src/orchestrator/concurrency.ts
• src/orchestrator/connection-handler.ts
• src/orchestrator/daemon-registry.ts
• src/orchestrator/instance-id.ts
• src/orchestrator/instance-liveness.ts
• src/orchestrator/job-dispatcher.ts
• src/orchestrator/job-queue.ts
• src/orchestrator/liveness-reaper.ts
• src/orchestrator/queue-worker.ts
• src/orchestrator/valkey-cleanup.ts
• src/shared/ws-messages.ts
• src/types.ts
• src/webhook/router.ts
• src/workflows/dispatcher.ts
• src/workflows/execution-row.ts
• src/workflows/handlers/implement.ts
• src/workflows/handlers/plan.ts
• src/workflows/handlers/review.ts
• src/workflows/handlers/ship.ts
• src/workflows/handlers/triage.ts
• src/workflows/orchestrator.ts
• src/workflows/registry.ts
• src/workflows/runs-store.ts
• test/daemon/ws-client.test.ts
• test/orchestrator/daemon-registry.test.ts
• test/orchestrator/instance-liveness.test.ts
• test/orchestrator/job-queue.test.ts
• test/orchestrator/liveness-reaper.test.ts
• test/orchestrator/queue-worker.test.ts
• test/orchestrator/valkey-cleanup.test.ts
• test/workflows/dispatcher.test.ts
• test/workflows/handlers/ship.test.ts
• test/workflows/handlers/triage.test.ts
• test/workflows/orchestrator.test.ts
• test/workflows/runs-store.test.ts

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/test-run-bugs

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[chrisleekr]

🎉 This PR is included in version 1.3.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀