Skip to content

fix: make task splits atomic - #15

Merged
AbirAbbas merged 1 commit into
mainfrom
fix/atomic-task-split
Sep 24, 2026
Merged

AbirAbbas merged 1 commit into
mainfrom
fix/atomic-task-split

Conversation

@santoshkumarradha

Copy link
Copy Markdown
Member

Failed splits currently leave newly created tasks behind and clear the parent's active claim. Successful splits also expose children before all dependency edges have been inserted, allowing concurrent workers to claim an incomplete graph.

Perform the parent transition, child creation, dependency insertion, creation events, and readiness promotion in one immediate SQLite transaction. Any validation or SQL failure rolls back the whole mutation. A shared connection-level task insertion helper preserves existing task defaults without re-locking the database inside the transaction.

Validation: all 14 Rust unit tests and 131 functional assertions pass locally. Regression coverage checks duplicate titles, unknown dependency references, a duplicate-edge SQL failure after an earlier edge was inserted, preservation of the complete parent record and event count, and successful results/dependency readiness. Repository-wide formatting has existing failures outside this change; the workflow treats formatting and Clippy as advisory.

Fixes #13.

@AbirAbbas AbirAbbas left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verified at 523abec.

The fix does what it says: the parent update, child creation, dependency edges, and promotion now happen inside one BEGIN IMMEDIATE transaction with a single commit, instead of separate writes an agent could see partially applied.

  • Repro: on main, a 4-worker probe against a 100-part split claimed a child with only 28/100 sibling tasks and 0/99 edges present, and hit an early claim before the graph was complete. Same probe against this branch: 0 early claims, 0 partial-graph observations, across two runs.
  • Injected a rollback mid-transaction (extra commit/begin before the dependency loop) and split_failure_preserves_parent_children_dependencies_and_events correctly failed, then passed again once reverted — the test is actually pinning this behavior.
  • cargo build, cargo test (14/14), and tests/functional_test.sh (131/131) all pass. cargo fmt --check and clippy -D warnings both fail, but on the same lines as main — not introduced by this PR.

No merge blockers. Approving.

(Separately: found a pre-existing race where concurrent CLI starts can hit task_readiness view errors — that's independent of this PR and main has it too. Filing it as its own issue.)

@AbirAbbas
AbirAbbas merged commit ccc3715 into main Sep 24, 2026
3 checks passed
@AbirAbbas
AbirAbbas deleted the fix/atomic-task-split branch September 24, 2026 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

split: failed mutations leave children and clear parent claims

2 participants