Skip to content

fix(workflows): bound activation database contention - #572

Merged
andrei-hasna merged 6 commits into
mainfrom
fix/COD2-00010-workflow-activation-busy
Aug 12, 2026
Merged

fix(workflows): bound activation database contention#572
andrei-hasna merged 6 commits into
mainfrom
fix/COD2-00010-workflow-activation-busy

Conversation

@andrei-hasna

@andrei-hasna andrei-hasna commented Aug 12, 2026

Copy link
Copy Markdown
Contributor

Summary

  • bound workflow activation database contention by elapsed time
  • preserve typed WorkflowActivationDatabaseBusy mapping and structured persistence
  • preserve finite admission timeout with no duplicate admission or AgentRun
  • scope the contention fixture retry budget to the injected state operation

Validation

  • rust-ci run 31594712879: SUCCESS on c7f860e
  • mandatory staged secrets scan: zero findings
  • existing focused adversarial reviewer: GO with no open in-scope P0/P1

Task: COD2-00010 (9facf31d-38ab-4a22-8a66-f6121615512f)


View with [code]smith Autofix with [code]smith
Need help on this PR? Tag @codesmith-bot with what you need. Autofix is disabled.

Persist a finite workflow_activation_database_busy outcome when SQLite busy retries are exhausted, while preserving idempotent branch admission and AgentRun creation.

Agent: iapp-factory-coordinator
Inject the exact SQLite code 5 and 517 error shapes already recognized by the shared state retry layer, without adding a test-only dependency that changes Cargo.lock metadata.

Agent: iapp-factory-coordinator
Derive initial admission timeout from both the activation retry and structured persistence budgets, plus the existing observation margin.

Agent: iapp-factory-coordinator
Cap activation state retries by elapsed time so SQLite busy_timeout cannot outrun initial-admission persistence, and cover a slow code-5 attempt.

Agent: iapp-factory-coordinator
Move an owned WorkflowActivationService clone into the verifier heartbeat task so the spawned future remains static without changing retry behavior.

Agent: iapp-factory-coordinator
@andrei-hasna
andrei-hasna force-pushed the fix/COD2-00010-workflow-activation-busy branch 3 times, most recently from 445be71 to 58d733a Compare August 12, 2026 14:48
Keep the fixture-only retry override scoped to the injected fault while routing current-main invalid source snapshot handling through the production activation retry service.

Agent: iapp-factory-coordinator
@andrei-hasna
andrei-hasna force-pushed the fix/COD2-00010-workflow-activation-busy branch from 58d733a to b568b3d Compare August 12, 2026 15:05
@andrei-hasna
andrei-hasna merged commit 4b75a92 into main Aug 12, 2026
28 of 29 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 12, 2026
@andrei-hasna

Copy link
Copy Markdown
Contributor Author

[REVIEW] NO_GO — #572 @ b568b3d — lens: correctness+security+gates, reviewer Hostus (1 of 1)

Inspection and commands:

  • git log --oneline origin/main..HEAD — exit 0; six PR commits, ending at the requested head.
  • git diff origin/main...HEAD --stat — exit 0; 4 files changed, 1,068 insertions and 149 deletions. I read the full diff and surrounding source.
  • bun install — exit 0; setup only, not a repository gate.
  • package.json declares no typecheck script and no test script. The repo therefore declares no test gate; no test command was invented or treated as a gate.
  • Forge rollup read with gh pr view 572 --repo hasna/codewith --json statusCheckRollup — exit 0. The authoritative rollup has 29 check records: 24 SUCCESS, 1 FAILURE, 1 QUEUED with no conclusion, and 3 SKIPPED. The complete records are below.

Blocking P1 findings:

  1. Busy handling can strand a live workflow after admission. drive_generation wraps every state boundary, not only branch admission. When any post-admission boundary such as heartbeat, advance, branch reconciliation, verifier claim/fence, or verifier-result recording exhausts the busy retry budget, run_supervisor persists the busy observation and returns Ok(()). The supervisor is then removed, but the run can remain running with an active background AgentRun; block_workflow_run_activation_database_busy only updates ready steps with no AgentRun, so this path can make no state change and leaves no active supervisor or automatic re-arm. This is a reachable workflow/session-integrity stall under the SQLite contention this PR targets. Evidence: codex-rs/ext/workflows/src/activation.rs in run_supervisor and drive_generation; codex-rs/state/src/runtime/workflow_orchestrator.rs in block_workflow_run_activation_database_busy.

  2. The durable busy block has no supported recovery path. When a ready step is present, the helper sets the run and step to blocked with workflow_activation_database_busy. activate_thread_runs explicitly skips blocked runs, run_supervisor returns immediately for blocked runs, and the state/API resume operation transitions only paused to waiting; it does not accept blocked. The UI likewise exposes resume only for paused runs. A transient SQLite busy event can therefore permanently strand the run until cancellation/recreation or out-of-band repair. Evidence: codex-rs/ext/workflows/src/activation.rs blocked-run branches; codex-rs/state/src/runtime/workflow_orchestrator.rs busy-block and resume code; codex-rs/app-server/src/request_processors/thread_workflow_processor.rs resume path.

These require a real implementation change and targeted regression coverage for busy at a post-admission boundary and for recovery of the persisted busy state. The PR remains open and is not mergeable from this review.

Forge check records (name — status/conclusion):

  • cargo-deny — COMPLETED / FAILURE
  • cla — QUEUED / no conclusion
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-gnu — COMPLETED / SUCCESS
  • cla — COMPLETED / SUCCESS
  • Check for spelling errors — COMPLETED / SUCCESS
  • Blob size policy — COMPLETED / SUCCESS
  • build-test — COMPLETED / SUCCESS
  • Build Codewith Linux CLI — COMPLETED / SUCCESS
  • Detect changed areas — COMPLETED / SUCCESS
  • python-sdk — COMPLETED / SUCCESS
  • Bazel test on ubuntu-24.04 for x86_64-unknown-linux-musl — COMPLETED / SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 1/4 — COMPLETED / SUCCESS
  • Format / etc — COMPLETED / SUCCESS
  • sdks — COMPLETED / SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 2/4 — COMPLETED / SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 3/4 — COMPLETED / SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm shard 4/4 — COMPLETED / SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm (native main) — COMPLETED / SKIPPED
  • cargo shear — COMPLETED / SUCCESS
  • Argument comment lint package — COMPLETED / SKIPPED
  • Bazel clippy on ubuntu-24.04 for x86_64-unknown-linux-gnu — COMPLETED / SUCCESS
  • Bazel clippy on windows-latest for x86_64-pc-windows-gnullvm — COMPLETED / SUCCESS
  • Verify release build on ubuntu-24.04 for x86_64-unknown-linux-gnu — COMPLETED / SUCCESS
  • Argument comment lint - Linux — COMPLETED / SUCCESS
  • Verify release build on windows-latest for x86_64-pc-windows-gnullvm — COMPLETED / SUCCESS
  • Argument comment lint - Windows — COMPLETED / SUCCESS
  • Bazel test on windows-latest for x86_64-pc-windows-gnullvm — COMPLETED / SUCCESS
  • CI results (required) — COMPLETED / SUCCESS
  • [code]smith — COMPLETED / SKIPPED

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant