Skip to content

fix(streams): refuse completed terminal for unrecorded tests - #4850

Open
ScriptedAlchemy wants to merge 16 commits into
masterfrom
fix/4450-managed-test-terminal-settlement
Open

ScriptedAlchemy wants to merge 16 commits into
masterfrom
fix/4450-managed-test-terminal-settlement

Conversation

@ScriptedAlchemy

@ScriptedAlchemy ScriptedAlchemy commented Nov 9, 2026 •

Copy link
Copy Markdown
Owner

Exact-head Clippy repair — November 9, 2026 (cbf262c)

Previous-head run #42691002146 on 24fabcc passed Repository gates and Dashboard but failed actual strict Clippy in crates/tracedecay/src/mcp/server.rs:810: -D clippy::redundant-closure-for-method-calls rejected the new daemon owner access .map(|service| service.operation_events()). Clippy reported the same diagnostic across production and test-transport configurations. Commit cbf262c uses exactly the compiler-suggested DaemonInvocationService::operation_events method reference, preserving the existing Option/default behavior. The commit changes one line in one production file, has one parent and was published by exact-old-head force=false guarded fast-forward; the PR head and remote branch independently match. No assertions, authorization, stream lifecycle or test scope changed.

Verification boundary: The previous-head Clippy result is red; a fresh exact-head passing Clippy/Bazel matrix is not yet observed. The older Linux fleet was still in progress at inspection; native macOS was skipped. Do not merge or close #4450 until completed new-head Rust/Bazel/Clippy, independent review and full daemon/HTTP/MCP/LSP/advisory/shutdown isolation, resume, restart and cancellation acceptance. Refs #4450, not Fixes.


Current-head owner propagation CI repair — 2026-11-09 (24fabcc)

Exact preceding-head CI #42680244918 failed nonzero Linux Bazel compilation/Clippy with Rust E0061: build_http_application_router now requires the daemon-owned OperationEventAuthority, but mount_http_application_router and the native-poll regression still supplied three arguments. Repository gates, Dashboard and Feature gates succeeded on that preceding head; Linux fleet and Clippy failed, the Linux summary failed, Windows was in progress, and macOS was skipped. Not green.

Commit 24fabcc threads the same existing daemon invocation authority through the live project-composition HTTP mount to the router builder, while the standalone poll regression owns an explicit test-scoped authority. This is a two-file, one-parent, non-force expected-head fast-forward, independently verified on the PR ref and GitHub comparison. No assertions, HTTP authorization, event-capacity, resume, or durable-settlement semantics were loosened.

New exact-head CI #42691002146 is PENDING. No claimed current-head Rust/Bazel/Clippy/macOS or full HTTP/MCP/LSP/restart/resume proof. This PR remains ready for review, Refs #4450, NOT MERGEABLE ON CURRENT EVIDENCE; #4450 stays open pending end-to-end lifecycle acceptance.


Latest exact-head Bazel formatter repair — 2026-11-09 (7bb060c)

CI #42676742347 on the preceding head 2897268 ran eight repository script checks successfully but failed Repository gates at exactly four Rustfmt hunks in tracedecay-daemon-service/src/invocation.rs, tracedecay-application/src/operation_stream.rs, tracedecay/src/daemon/project_open_owners/advisory_runtime.rs, and tracedecay/tests/mcp_suite/mcp_handler_test/affected_tests_behavior_test.rs. Its Feature gates failed downstream, and its Clippy/Linux/Dashboard jobs were cancelled after supersession; ordinary macOS was skipped. These are not passing Rust/Bazel acceptance results.

The focused commit 7bb060c applies the exact four emitted Rustfmt edits with no behavior, assertion, test-target, or protection changes. Its four-file patch and one-parent expected-head, non-force branch fast-forward were independently verified against the GitHub commit. Current-head CI #42680244918 is QUEUED and has not yet established format, Clippy, nonzero Rust/Bazel or native macOS success. The separate full daemon-owned operation-stream isolation, shutdown/cancellation, HTTP/MCP/LSP and restart/resume acceptance remains subject to current-head tests/review. Refs #4450; keep this non-model PR ready for review but unmerged, and keep #4450 open.


Full owner cutover: compile and isolation regression repair — 2026-11-09 (head 2897268)

The existing non-model #4450 PR now carries the daemon-owned authority cutover published at f71fc27. This follow-up reviewed its actual new branch code and repaired two concrete gaps left by that cutover, without rewriting any other contributor work:

  • 92d25a0: existing registered-store settlement regression fixtures were missing the new required ManagedTestRunRecording.events field. Both now explicitly reuse the exact OperationEventAuthority owning the already-subscribed emitter, rather than constructing a detached authority. All three constructors in the fixture were statically inspected; none lacks the required field.
  • 347b1b7 and 2897268: retired the stale integration test's removed process-global accessor. The replacement uses the actual composed daemon owners exposed by the existing test-only production harness, requires both independent compositions to admit the same request identity, then shuts down one and requires the other's already-accepted stream to publish its first result and terminal unchanged. The obsolete singleton import is removed.

GitHub guarded, non-force fast-forwards were verified; exact three-commit compare changes exactly three files. This is a compilation/behavioral regression repair based on real source tracing, NOT a claim of passing Rust tests. Exact-head hosted CI #42676742347 was QUEUED at last read. Bazel, Clippy, rustfmt, HTTP/MCP/LSP integration, Linux fleet and native macOS on this head are not yet verified. Do not merge or close #4450 before current-head nonzero test acceptance, review, and owner lifecycle proof. Refs #4450 only.

Refuse unverified completion for a managed test operation — Refs #4450

Daemon-owned operation stream cutover — f71fc27 (November 9, 2026)

Commit f71fc27 is a guarded, single-parent, non-force update of the existing issue PR. It removes the application-global OPERATION_EVENTS / operation_event_authority() and daemon forwarding accessor, constructs a fresh authority in each DaemonInvocationService, and propagates that exact clone through cold HTTP/SSE routing, project-open advisory cycles, MCP managed-test producer/dispatch (including explicit per-standalone-server ownership), and registered LSP managed-test projection. The daemon's own shutdown now visits only its authority; it still retains accepted live producers during drain. The existing bounded stream, tokens, generation semantics, durable test-run record, and authorizations remain unchanged.

A new nonzero Tokio behavioral regression admits the same managed-test request ID into two separately constructed daemon services, expires the first owner, and requires a live test-result publication through the second. Under the old process-global singleton, admitting the duplicate ID into the second service fails. Direct affected-test test fixtures now explicitly construct their own owner rather than using ambient lookup. Exact compare confirms 18 affected production/test files and no deleted assertions.

Verification / HOLD: The full 18-file diff was independently reviewed with GitHub's commit comparison; no local Bazel, rustfmt, Clippy, native macOS, or real HTTP/MCP/LSP integration suite is available in this execution environment. Hosted exact-head CI has not reported applicable results yet (it may not be queued immediately). Previous-head green evidence does not cover this cutover. Required before merge/closure: Bazel nonzero operation-stream, managed-test, daemon shutdown and HTTP/MCP/LSP acceptance; two real composition isolation/restart/resume/cancel lifecycle proof, then current-head CI and review. Refs #4450 only; do not merge/close on partial evidence.

Latest review repair — 4c209dc (November 9, 2026)

Commit 4c209dc fixes an independently confirmed, unresolved review observation: if the registered outcome write AND the failed-terminal publication both refuse, the former recorded? path correctly preserved the primary typed Database error but silently discarded the second publication failure. The outcome path now logs the operation ID, the durable-store refusal, and the independent terminal-publication refusal before returning the storage error; successful durable records still propagate publication failure normally.

The existing real registered-store/Tokio double-failure regression was extended to subscribe to the authenticated operation, verify the previously published Completed terminal remains the only terminal, and require the original Database refusal. This is a falsifiable stream/settlement regression, not a source-shape test. The exact two-file one-parent commit was reviewed and pushed with force=false and expected old head 21e5e01dc31a.

The previous head CI #42651633957 now verifies Repository gates (including Bazel Rustfmt), Clippy, Dashboard, Feature gates, and Linux fleet all passed; its log reports //crates/tracedecay-mcp:unit_test PASSED and 183/183 Linux Bazel targets passed, with Windows still running when checked and the ordinary macOS job skipped. These passing results apply to 21e5e01, not this commit. New exact-head CI #42662741792 is pending; the new regression and warning have not yet been executed in hosted Rust/Bazel. The broader daemon-owned authority migration/acceptance remains unfinished; this PR is Refs #4450 and must not merge or close #4450 on this partial evidence.

Current-head CI repair — 21e5e01

Previous-head CI #42644720150 executed 8 repository script tests successfully but failed Bazel rustfmt_check_2024 and Clippy on the new managed-test fixture. The Rust formatter required a wrapped cancellation binding; Clippy rejected an unused futures_util::StreamExt as _ import inherited from the parent module. Commit 21e5e01 applies both exact reported corrections in affected_tests_tests.rs, with no production or assertion changes, via an expected-head non-force fast-forward. The new exact-head CI #42651633957 is pending; no passing current-head Rust/Bazel/Clippy/native test result is claimed. #4450 remains open pending full daemon/HTTP/MCP/LSP/advisory/shutdown ownership migration, isolation and lifecycle verification. Do not merge on this partial fix.

Follow-up: preserve the authoritative storage refusal during double failure

Current head 864e982cf6189af95586f39305bcc8b5a2f73d42 adds the independently reviewed commit 51c71ffc89b9f75f90b3b589fa278cb638bbd567 and a bounded Rust formatting follow-up. A second real error-precedence defect was present: when registered-store start/outcome persistence failed and its already-terminal/shutting-down operation stream refused publication, publish_test_run_terminal(...).await? replaced the primary typed TraceDecayError::Database with a secondary lifecycle Config error. record_start now logs a secondary terminal failure without hiding its durable-store refusal. finish attempts publication and then returns the storage error first, while a successful storage write still propagates publication failure.

Added a nonzero-path registered-store/Tokio regression that deliberately pre-terminates the authenticated authority, attempts to settle an unstarted global-DB run, and requires the database refusal to win over the independently rejected second terminal. Reorganized the prior regression imports to module scope per review and applied exact Rust formatting from the previous-head Repository gates failure (CI run 42635759877, rustfmt_check_2024). No committed Rust/Bazel test or current-head format/Clippy/native macOS result has been verified; all new head checks are pending. This remains partial Refs #4450, open for review but not eligible to merge or close without full daemon-ownership acceptance.

An admitted MCP managed-test run can finish its test process successfully while its registered-store outcome UPDATE fails (for example, the start row is absent or the writer refuses the update). Previously ManagedTestRun::finish published a Completed operation-stream terminal before propagating that database error. HTTP/live subscribers could therefore observe successful completion with no durable outcome, and LSP readers could find an unsettled record. This violates #4450's requirement to preserve truthful live-stream terminals and durable effect receipts.

Focused correction: keep the canonical recorded outcome as the authority. Publish the successful receipt only after a successful registered-store settlement; when storage refuses, publish a terminal with Failed termination/receipt and the actual consumed-byte count before returning the original typed storage error. The existing success path and stream-close behavior are preserved. No second event store or transport is added.

Falsifiable Rust regression committed: refused_durable_test_result_cannot_publish_a_completed_live_terminal uses a real registered-global-DB lease, the production ManagedTestRun::finish, a real authorized OperationEventAuthority subscription and StreamEventKind. It intentionally attempts to settle a never-started run (zero UPDATE rows). It requires a typed DB refusal, the accepted event, then a Failed terminal carrying the consumed bytes, followed by stream completion. Before the production fix, the same scenario emits Completed.

Evidence / limitations

  • Root AGENTS.md read; code and unchanged issue requirements checked against master 8d6fe6791a1b9983a066f9ada990d70cb6eb451a. Two source/test files only. Commits 2bc57afb46da107a536580cfa23f602544fd5769 and 3df8d6301a67fcc312f4a66527f4ab2085e4e1b0 pushed by exact-parent non-force, expected-old-head updates and independently checked.
  • Standalone control-flow reproduction confirms the old rejected-settlement path reports completed, while the new path reports failed, with the accepted path unchanged. This is NOT an executed Rust regression.
  • Not yet executed: bazel test //crates/tracedecay-mcp:unit_test --test_output=errors (target verified in generated crates/tracedecay-mcp/BUILD.bazel), Bazel format/Clippy and native macOS. Local Rust/Bazel checkout unavailable. Hosted current-head CI must be inspected; queued/skipped jobs are not a passing suite.
  • Important remaining simplify(streams): make operation-event lifetime belong to its daemon instead of a process-global singleton #4450 acceptance: retire OPERATION_EVENTS singleton and forwarding accessor, pass one daemon-owned handle through invocation/HTTP/MCP/LSP/advisory/shutdown, support explicit standalone test owners, and verify multi-composition isolation, shutdown, resume expiry, cancellation and nonzero Bazel/native tests. This PR covers only a verified integrity prerequisite, not the complete ownership migration. Do not merge or close simplify(streams): make operation-event lifetime belong to its daemon instead of a process-global singleton #4450 merely for this slice.
  • No releases, tags, infrastructure, credentials, test/protection weakening or peer branch changes.

This is Refs #4450, not Fixes #4450.


Devin Review

@changeset-bot

changeset-bot Bot commented Nov 9, 2026 •

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: e473225

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

devin-ai-integration[bot]

This comment was marked as resolved.

devin-ai-integration[bot]

This comment was marked as resolved.

Copy link
Copy Markdown
Owner Author

Actionable exact-head CI diagnosis — no source change in this review. CI 42700273167 at cbf262cd14b21b5c9f1012ba1e93ce6e1b003dd5: Repository gates, Dashboard, Feature gates and blocking Clippy all passed. Linux fleet compiled and ran 183 targets, 182 passed and one failed: //crates/tracedecay:dashboard_api_test, test projects::dashboard_projects_endpoint_does_not_launder_registry_read_failure, at tests/common/mod.rs:1587 (dashboard server did not become ready at 127.0.0.1:42193) while the test log separately recorded tracedecay dashboard listening on http://127.0.0.1:42193. This is readiness/fixture behavior, not an operation-event Rust compile or Clippy failure. Do not raise readiness timeout or weaken the registry-failure assertion; distinguish a stale process/bind, probe refusal, or scheduling from the intended registry error. Overlapping draft dashboard readiness/loopback work #4834/#4835 should be checked before changing shared fixture ownership. Linux summary correctly red, macOS skipped, Windows was still in progress on inspection. PR/issue remain unmerged/open; daemon-owned HTTP/MCP/LSP/advisory/shutdown lifecycle acceptance remains outstanding.

Refs #4450. Add a real service-level regression opening the same root and request
in two independent daemon compositions. The first owner's cancellation and
expired-frontier cleanup must not cancel or remove the second owner's active
run. Keep the existing distinct-root test and all lifecycle assertions.

Copy link
Copy Markdown
Owner Author

New #4450 lifecycle regression — 2026-11-09, 92a77d06

Commit 92a77d06e3d6 was safely published on this existing PR from exact old head cbf262cd14b2 via force=false with expected-head check. GitHub commit diff/PR head/remote branch ref independently match. One test file +51 lines; no production source or existing assertions removed.

The new non-vacuous cancelled_managed_test_in_one_daemon_cannot_cancel_another regression constructs two independent DaemonInvocationService compositions with the same project URI and request identity. It asserts that the first owner's cancellation does not cancel the second owner's stream; expiring the first owner's frontier does not prevent the second from publishing a real managed-test result; and the second owner can independently admit its own cancellation. With the previous process-global operation table, the second cancellation would be an already-requested shared cancellation. This supplements—not replaces—the existing distinct-project isolation regression.

Verification boundary: This Rust regression is committed but has not yet executed via Bazel. Exact-parent hosted run 42700273167 passed Repository gates, Clippy, Dashboard and Feature gates but failed one of 183 Linux Bazel test targets: //crates/tracedecay:dashboard_api_test (88/89 individual cases passed); projects::dashboard_projects_endpoint_does_not_launder_registry_read_failure never reached the intended registry-failure assertions because the shared dashboard HTTP readiness probe failed. PR #4835 contains an independently developed loopback-listener ownership fix for startup races; reconcile its legitimate test correction, don't simply extend timeouts or suppress the coverage assertion. Windows was still in progress and macOS skipped on that parent run. These predecessor results do not qualify the new exact head.

Keep #4450 OPEN, Refs only, and this PR unmerged until new-head nonzero Bazel/Clippy/format checks and complete HTTP/MCP/LSP/advisory/shutdown, restart/resume/cancellation acceptance. No model, release, infrastructure or automation changes.

Copy link
Copy Markdown
Owner Author

New exact-head Linux CI blocker (2026-11-09 23:50 UTC), no speculative source change. Run 42718580170, Linux fleet job 129507551564, head 92a77d06e3d6d301a4dbf104633e140de8d1f244: 182/183 Bazel targets passed; the sole failure is //crates/tracedecay:hooks_lsp_suite → hook_spool_drain_test::a_receipt_backlog_beyond_one_batch_drains_without_waiting_for_the_sweep at hook_spool_drain_test.rs:436. The assertion expected {"status":"drained"}, while the actual canonical tracedecay_status.structuredContent.hook_replay was {"status":"failed","failures":[{"host":"cursor_desktop","spool":"pending_work","cause":"1 pending producer work obligation(s) remain unresolved; the first was backpressured"}]}.

Narrow source diagnosis: The fixture emits 65 genuine Cursor stop events to populate >64 delivery receipts; those events also admit separate producer-work obligations. It then drains the receipt files promptly and assumes the entire hook replay owner must be drained, even if a separate producer obligation remains under truthful backpressure. The replay consumer's redrive_pending_work tracks that obligation as owed, reports the exact pending_work failure, and applies a 30s initial retry backoff, which exceeds this test's 10s PROMPT_DRAIN window. The observed failure is not a leftover delivery receipt, nor the earlier dashboard listener-readiness failure. Distinguish whether the orphaned producer obligation is legitimately transient or fails to receive a recovery wake; ensure actual settlement or isolate receipt batching from unrelated producer admission without hiding the obligation, changing budgets, or weakening any assertions. The kernel's hook_v2_replay_consumer.rs and hook fixture are shared with other lanes, so do not race a speculative fix here.

Repository gates, strict Clippy, Dashboard, Feature gates passed at this exact head; macOS was skipped, Windows was still running when inspected. Linux summary is red. The new service-level daemon-isolation test is committed but this fleet failure prevents green integration acceptance. Keep PR #4850 OPEN/UNMERGED, #4450 OPEN/Refs only; full HTTP/MCP/LSP/advisory/shutdown lifecycle acceptance remains outstanding.

Refs #4450. The 65-stop backlog fixture introduced independent producer-work obligations and could report a truthful backpressured status after all receipts settled. Exercise the real receipt writer and daemon settlement with 65 validated, distinct hook delivery receipts but no unrelated producer events. Preserve the 10-second bound, exact 65-receipt assertion, full drained status and independent capture/replay tests. No production code or timing/backoff limits changed.

Copy link
Copy Markdown
Owner Author

Published a concrete non-model test isolation repair for the verified current-head Linux failure, without masking either receipt or producer-work state: commit e18430e (one parent, one test file, guarded force=false expected-head update, PR/head/ref readbacks match).

Exact preceding-head CI 42718580170 ran 183 Linux Bazel targets, 182 passed, with hooks_lsp_suite::hook_spool_drain_test::a_receipt_backlog_beyond_one_batch_drains_without_waiting_for_the_sweep failing at line 436. The 65 Stop callbacks generated independent end-of-turn producer-work obligations; although all delivery receipt files settled, one admission remained backpressured, so the daemon correctly reported status=failed with spool=pending_work rather than claiming drained. Its 30-second redrive backoff makes a 10-second receipt batch fixture incapable of proving the intended independence.

The test now constructs 65 distinct validated Hook-delivery source receipts with the existing production HookDeliverySourceReceiptV1::new authority, publishes them through the real HookDeliveryReceiptWriterV1, verifies all 65 distinct identities and 65 durable files, then restarts the actual daemon and requires all receipts to settle and the exact {\"status\":\"drained\"} result within the unchanged 10-second budget. No host producer events are invented or dropped, and unrelated capture/replay tests continue to exercise real host callbacks. No production backoff, status classification, assertion deadline, or delivery authorization was changed.

Verification boundary: This is a committed, independently diff-reviewed Rust regression adjustment, not yet an executed new-head passing Rust/Bazel test. New exact-head CI 42734412375 was QUEUED on inspection; macOS native is skipped by this workflow. Keep #4450 OPEN (Refs #4450 only) and #4850 unmerged pending actual applicable CI, code review, two-daemon isolation, restart/resume/cancellation and full HTTP/MCP/LSP/advisory/shutdown acceptance.

Refs #4450

The pinned rustfmt check reports import ordering and an over-long
assert_eq! call in the spool-drain regression. Apply exactly those
hunks without changing any assertion.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

simplify(streams): make operation-event lifetime belong to its daemon instead of a process-global singleton

1 participant