Repository navigation
refactor(native-integration): make stores async, drop actors - #4143
Conversation
|
There was a problem hiding this comment.
Devin Review found 1 potential issue.
2 flags not posted on this PR by your GitHub settings — view them in Devin Review. (Configure)
| .await?; | ||
| let outcome = self | ||
| .ports | ||
| .preflight_outcomes |
There was a problem hiding this comment.
🔴 Concurrent stack preflights lose previews
When identical declared-stack preflights overlap, preflight_outcomes.remove gives the saved preview to only one caller. The other returns Unavailable despite a successful preflight.
(Refers to this code)
Learn more
The stack coordinator shares one disposition among identical in-flight preflights in optional_preflight. Its native preflight port caches the concrete preview once in preflight. Each caller then independently consumes that single cache entry. Even though the underlying preflight succeeds, one caller receives Unavailable; a later caller can also consume a new run's entry if it arrives before the earlier caller removes it.
Example: Two callers submit the same declared-stack request. One native preflight saves Preview P. Both receive Complete, but the first removal takes P and the second finds no outcome, so only one caller receives P.
Recommended fix: Carry the complete NativeIntegrationPreflightOutcomeV1 in the shared in-flight result, or otherwise publish an immutable shared result to every joiner. Avoid a destructive lookup keyed solely by request digest after the single-flight future returns.
Was this helpful? React with 👍 or 👎 to provide feedback.
Summary
Step 5 of the storage redesign (
docs/plans/storage/README.md), fifth slice. It deletes the native-integration storage actor thread, and the GitHub stack-delivery actor that had the same shape. Same pattern as #4140. Two commits:NativeIntegrationStoreis now async.DaemonNativeIntegrationStoreis a cloneable handle that holds the lease and awaits the global-db store.tracedecay-native-integration-storethread, its 21-variantStoreCommand, itsblock_onsettle and theSharedDaemonNativeIntegrationStorewrapper.NativeIntegrationPortand its service are async.NativeWorktreePortis async forremoveandreconcile, the two calls that use the store.OptionalStackPreflightPortandGitHubStackSignalExpandPortare async.tracedecay-github-stack-delivery-storethread and its 10-variant protocol are deleted.DaemonStackDeliveryStoreV1holds the lease and the project id.StackCoordinatorStoreand its authorize and deliver steps are async.spawn_blocking, and keeps its 250 ms cadence.github_stack_hook_availableis async, which reaches MCP hook admission.How each invariant is kept:
shutdowntakes the shared lease once for all clones, and later calls fail as unavailable.RegisteredGlobalDbwriter wait, as in refactor(git-index): make transaction store async, drop actor #4140.on_blocking_pool: preflight, apply, worktree remove and reconcile, startup recovery, and cleanup recovery. Store-only operations await in place.Test plan
Each test below was mutation-checked. The mutation was applied, the test failed with the stated assertion, and the mutation was reverted.
store_awaited_on_a_native_worker_never_parks_it: noblock_on, and the wait counters do not move. Ablock_onsettle insave_approvalfails it.delivery_store_awaited_on_a_native_worker_never_parks_it: ablock_oninpendingfails it.shutdown_closes_every_clone_once_and_fails_later_calls_closed: keeping the lease fails it.concurrent_applies_to_one_repository_never_overlap: dropping the repository guard early fails it (left 2, right 1).concurrent_duplicate_debounced_signals_append_once: dropping the dedupe guard before the append fails it (left 2, right 1).joined_preflight_returns_the_single_owner_result: covers the rewritten single-flight join.bazel test --config=clippy:unit_test(540 passed)unit_testandapplication_suiteunit_testunit_testandcontracts_suite//crates/tracedecay-mcp:unit_test --test_arg=hook,//crates/tracedecay:unit_test --test_arg=native_integrationand--test_arg=stack, and//crates/tracedecay:product_surface_suite --test_arg=native_integrationbazel build --config=clippyfor//crates/tracedecay:unit_test,:daemon_suite,:mcp_suiteand//crates/tracedecay-mcp:unit_testrustfmt_check_2024for every touched crate//crates/tracedecay:daemon_suite,:mcp_suite,:session_suite,:unit_testin CIKnown gap:
optional_preflightnow awaits between admission and counter release. If its owner future were dropped mid-preflight, the in-flight entry would leak. The daemon runs it on the blocking pool, where it always completes.Refs #4052