Repository navigation
refactor(work): run adjudications and placements as writer jobs - #4142
Conversation
AdjudicateDuplicate now runs the contracts duplicate service inside one ExactSqlHandle::transact job over WorkSqliteJobStorage. The command replay check, the attempt checks, the revision compare and the append commit together, and the awaiting task parks no native thread. The append moves to a WorkDuplicateAdjudicationRecordPortV1 without a Send + Sync bound, implemented only by the job storage; the handle-based append had no other production caller and is deleted. The current Work and topology generations the command must cite are still read from the product graph before the job, on a reader, and passed in owned, so the graph read never holds the writer.
AdjudicateLeak now runs the contracts leak service inside one ExactSqlHandle::transact job over WorkSqliteJobStorage. The command replay read, the verdict, the attempt check, the revision compare and the append commit together, and the awaiting task parks no native thread. The leak evidence the verdict reads (the attempt, its placement and effect holder, the global-db delivery census and the process registry's live-owner answer) is still collected before the job and handed to it owned, so no global-db read or registry lookup runs on the writer. WorkLeakAdjudicationStoragePortV1 drops its Send + Sync bound; the job storage is its only implementation and the handle-based one is deleted.
AdmitPlacement and ReleasePlacement now run the contracts placement service inside one ExactSqlHandle::transact job over WorkSqliteJobStorage, and the awaiting task parks no native thread. The Git observation (discovery, status and the rev-list count) no longer runs inside a storage transaction. Admit observes command.target before the job and hands the job the owned observation; the replay check, the exclusivity read and the publication commit together. Release reads the placement on a reader, refuses a stale expected authority version before touching Git, observes the target, and the job then publishes only while the placement still has that target and version, refusing with the existing authority conflict when another release committed in between. publish_placement moves to a WorkPlacementPublicationPort, and WorkPlacementStoragePort drops its Send + Sync bound so the job storage can implement both. The handle-based publication had no other production caller and is deleted; the handle keeps the reads that preflight, status and cleanup use, sharing their SQL with the job. work_authority is exported so the release pre-read uses the same authority the service derives.
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
There was a problem hiding this comment.
Devin Review found 2 potential issues.
1 flag not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)
| self.storage | ||
| .transact(move |storage| { | ||
| tracedecay_contracts::WorkLeakAdjudicationServiceV1::new(storage, evidence.clone()) | ||
| .adjudicate(&context, command.clone(), scan_started_at, scan_deadline) |
There was a problem hiding this comment.
🔴 Queued leak scan records stale verdict
When Work state changes while the writer is queued, adjudicate_leak classifies an older snapshot as current. DaemonWorkLeakEvidenceV1::inspect timestamps that snapshot only after the job starts, so it can publish a false leak verdict.
Learn more
The daemon gathers the attempt, process-owner status, placement, effect and delivery census before enqueuing this job in read_leak_evidence. The writer can process other work before this job begins. inspect then assigns a new completion timestamp to the previously gathered snapshot and derives a verdict from it. State transitions during the queue wait are invisible, but the receipt attributes the verdict to the later scan completion.
Example: A running attempt has no live owner when the daemon reads it. The owner resumes before the queued job starts. The job still records AttemptWithoutLiveOwner as though the scan had just found an ownerless attempt.
Recommended fix: Determine the scan result and its completion timestamp before enqueueing, or revalidate the versioned evidence at the writer boundary before committing. Keep the replay and revision comparison in the writer job; do not timestamp frozen evidence at job execution time.
Was this helpful? React with 👍 or 👎 to provide feedback.
| if current.authority_version() != command.expected_authority_version { | ||
| return Err(tracedecay_contracts::WorkPlacementStorageError::AuthorityConflict.into()); |
There was a problem hiding this comment.
🟡 Stale placement masks admission refusal
For a cancelled or expired release with a stale version, release_placement returns authority conflict before checking admission. The service's admission check would instead return the cancellation or expiry refusal.
| if current.authority_version() != command.expected_authority_version { | |
| return Err(tracedecay_contracts::WorkPlacementStorageError::AuthorityConflict.into()); | |
| tracedecay_contracts::ApplicationProblem::ensure_admitted(&context, command.occurred_at)?; | |
| if current.authority_version() != command.expected_authority_version { | |
| return Err(tracedecay_contracts::WorkPlacementStorageError::AuthorityConflict.into()); | |
| } |
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Summary
Step 5 of the storage redesign (
docs/plans/storage/README.md), fourth slice. Four more Work operations run as one writer job each, in the shape #4139 set: the unchanged contracts decision runs insideWorkSqliteStorage::transactover a storage scoped to that transaction. A native worker awaiting any of them parks on neither the writer nor a reader. One commit per operation:WorkDuplicateAdjudicationRecordPortV1, a trait withoutSend + Syncthat only the job storage implements. Its evidence (the current Work and topology generation) was already read on a reader before the transaction, so the job never replays the product journal while holding the writer.git rev-list) is hoisted out of the job. Admit observescommand.targetfirst. Release reads the placement on a reader (load_placement_async), refuses a stale expected version before touching Git, and observes. Inside the job it publishes only if the target and authority version are unchanged, otherwise it returns the existing authority conflict.publish_placementmoves toWorkPlacementPublicationPort. The handle keeps the reads that preflight, status and cleanup use, and shares their SQL with the job.Each handle-based write (duplicate record, leak record, placement publish) is deleted in the commit that moved its last caller.
work_authorityis now exported from contracts, so the release pre-read uses the same authority the service derives.Behavior changes:
Test plan
crates/tracedecay-application/tests/application_suite/work_leaf_jobs.rsadds four tests. Each was mutation-checked:tracedecay-rusqlite-writer). A synchronous release pre-read fails the reader-wait assertion.authority-conflict. Removing the in-job version check fails it (it returnedalready-released).bazel test --config=clippy:unit_testandrusqlite_suiteunit_testandcontracts_suiteunit_testandapplication_suiteunit_testbazel build --config=clippyfor//crates/tracedecay:unit_test,:daemon_suiteand:mcp_suite//crates/tracedecay:mcp_suitepassesrelease_placement_publishesandwork_attempt_consumers_read_the_public_start_attempt_effect, which cover admit, duplicate adjudication and release end to end through the daemon.rustfmt_check_2024for each touched crate//crates/tracedecay:daemon_suite,:mcp_suite,:session_suite,:unit_testin CIRefs #4052