From ece7d27f09383c27ab341f0fa0c7fcbb6f4dc776 Mon Sep 17 00:00:00 2001 From: Zack Jackson <25274700+ScriptedAlchemy@users.noreply.github.com> Date: Sun, 8 Nov 2026 05:13:05 -0800 Subject: [PATCH] fix(feedback): keep editor and hook diagnostics independent of remote refresh Route local LSP and hook feedback without polling GitHub review refresh or CI discovery. Keep explicit advisory demand on its existing authorized remote refresh path, and preserve typed unavailable remote provider states. Add production-selector regressions proving a blocked remote CI read cannot be polled by local feedback and an explicit read waits for its own barrier. Refs #4423 --- .../project_open_owners/advisory_runtime.rs | 82 ++++++++++++++----- .../advisory_runtime/tests.rs | 59 +++++++++++++ 2 files changed, 122 insertions(+), 19 deletions(-) diff --git a/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime.rs b/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime.rs index bd756c6db29..9ac0b539039 100644 --- a/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime.rs +++ b/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime.rs @@ -1,6 +1,7 @@ //! Project-open composition for the canonical feedback and advisory owners. use std::fmt::Write as _; +use std::future::Future; use std::path::Path; use std::process::Command; use std::sync::Arc; @@ -151,6 +152,33 @@ struct ProjectOpenAdvisoryCycleExecutionV1 { feedback_cycle: Arc, } +/// Local editor/hook feedback must not wait for remote refresh. The explicit +/// advisory operation keeps its existing GitHub and CI refresh semantics. +#[derive(Clone, Copy)] +enum AdvisoryRemoteReadMode { + LocalFeedback, + ExplicitRefresh, +} + +async fn advisory_remote_requests( + mode: AdvisoryRemoteReadMode, + github: Option, + discover_ci: DiscoverCi, +) -> ( + Option, + ProductionCiFailureDiscoveryOutcomeV1, +) +where + DiscoverCi: Future, +{ + match mode { + AdvisoryRemoteReadMode::LocalFeedback => { + (None, ProductionCiFailureDiscoveryOutcomeV1::NotConfigured) + } + AdvisoryRemoteReadMode::ExplicitRefresh => (github, discover_ci.await), + } +} + #[derive(Clone)] struct ProjectOpenFeedbackCyclePinV1 { revision_id: ConfigurationRevisionId, @@ -220,6 +248,7 @@ impl ProjectOpenAdvisoryFeedbackCycleV1 { deadline: MonotonicDeadline, agent_stop_gate: bool, expected_generation: Option<&tracedecay_domain::CodeGenerationId>, + remote_mode: AdvisoryRemoteReadMode, ) -> std::result::Result { let pin = self.producer.feedback_cycle.read().await.clone(); let lsp_input = pin.runtime.lsp_input(); @@ -242,7 +271,7 @@ impl ProjectOpenAdvisoryFeedbackCycleV1 { Some(&latest.generation().manifest().generation_id) == invocation.request.input.target.generation_id.as_ref() }); - self.run_cycle_with_invocation(pin, invocation, deadline, agent_stop_gate) + self.run_cycle_with_invocation(pin, invocation, deadline, agent_stop_gate, remote_mode) .await } @@ -252,6 +281,7 @@ impl ProjectOpenAdvisoryFeedbackCycleV1 { mut invocation: FeedbackCycleInvocation, deadline: MonotonicDeadline, agent_stop_gate: bool, + remote_mode: AdvisoryRemoteReadMode, ) -> std::result::Result { if agent_stop_gate { invocation.request.input.request.trigger = FeedbackTriggerV1::AgentStopGate; @@ -265,17 +295,34 @@ impl ProjectOpenAdvisoryFeedbackCycleV1 { .request .configuration_digest .clone(); - let ci = match self.ci_discovery_config.as_ref() { - Some(config) => { - discover_production_ci_failure_request_v1( - &invocation.context, - config, - &self.feedback_scope, - ) - .await - } - None => ProductionCiFailureDiscoveryOutcomeV1::NotConfigured, - }; + // Construct the discovery future without polling it on editor or hook + // paths. A pending/denied remote provider cannot consume local feedback's + // deadline or cancel its independently authorized diagnostics. + let github_request = self.github_review_read.clone().map( + |(pull_request_id, operation)| GitHubReviewReadRequestV1 { + operation, + scope: self.feedback_scope.clone(), + pull_request_id, + }, + ); + let (github, ci) = advisory_remote_requests( + remote_mode, + github_request, + async { + match self.ci_discovery_config.as_ref() { + Some(config) => { + discover_production_ci_failure_request_v1( + &invocation.context, + config, + &self.feedback_scope, + ) + .await + } + None => ProductionCiFailureDiscoveryOutcomeV1::NotConfigured, + } + }, + ) + .await; let expires_at = UtcMicros(observed_at.0.saturating_add(5 * 60 * 1_000_000)); let operation = daemon_operation_event_authority() .begin( @@ -305,13 +352,7 @@ impl ProjectOpenAdvisoryFeedbackCycleV1 { }, AdvisoryCycleRequest { feedback: invocation.request, - github: self.github_review_read.clone().map( - |(pull_request_id, operation)| GitHubReviewReadRequestV1 { - operation, - scope: self.feedback_scope.clone(), - pull_request_id, - }, - ), + github, ci, proximity: Some(ProximityEvaluationRequestV1 { scope: self.feedback_scope.clone(), @@ -458,6 +499,7 @@ impl FeedbackCycleRuntimePort for ProjectOpenAdvisoryFeedbackCycleV1 { MonotonicDeadline::at(Instant::now() + Duration::from_secs(5)), false, None, + AdvisoryRemoteReadMode::LocalFeedback, ) .await?; Ok(()) @@ -549,6 +591,7 @@ impl DaemonAdvisoryCycleInvocationPort for ProjectOpenAdvisoryFeedbackCycleV1 { monotonic_deadline, false, None, + AdvisoryRemoteReadMode::ExplicitRefresh, ) .await .map_err(|failure| { @@ -1137,6 +1180,7 @@ async fn run_production_hook_cycle( MonotonicDeadline::at(Instant::now() + Duration::from_secs(5)), request.trigger == HookOrchestrationTriggerV1::Stop, Some(&indexed_generation.metadata().manifest().generation_id), + AdvisoryRemoteReadMode::LocalFeedback, ) .await { diff --git a/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime/tests.rs b/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime/tests.rs index aed4188bc6d..323c17dc5b1 100644 --- a/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime/tests.rs +++ b/crates/tracedecay/src/daemon/project_open_owners/advisory_runtime/tests.rs @@ -13,6 +13,65 @@ fn feedback_scope() -> FeedbackScopeV1 { } } +/// A blocked CI discovery must not even be polled for an editor/hook run. +/// The matching GitHub request is also excluded from local admission. +#[tokio::test] +async fn local_advisory_feedback_does_not_poll_remote_refresh() { + let github_request = GitHubReviewReadRequestV1 { + operation: GitHubReviewReadOperationV1::RestGetPullRequest, + scope: feedback_scope(), + pull_request_id: GitHubPullRequestIdV1::new("42").expect("pull request"), + }; + let forbidden_ci = std::future::poll_fn( + |_| -> std::task::Poll { + panic!("local feedback polled remote CI discovery") + }, + ); + let (github, ci) = advisory_remote_requests( + AdvisoryRemoteReadMode::LocalFeedback, + Some(github_request), + forbidden_ci, + ) + .await; + assert!(github.is_none(), "local feedback must not request GitHub refresh"); + assert!(matches!( + ci, + ProductionCiFailureDiscoveryOutcomeV1::NotConfigured + )); +} + +/// Explicit advisory demand still owns the remote read and propagates its +/// terminal result. The barrier proves the operation awaits actual discovery. +#[tokio::test] +async fn explicit_advisory_refresh_waits_for_its_own_remote_discovery() { + let (started_tx, started_rx) = tokio::sync::oneshot::channel::<()>(); + let (release_tx, release_rx) = + tokio::sync::oneshot::channel::(); + let github_request = GitHubReviewReadRequestV1 { + operation: GitHubReviewReadOperationV1::RestGetPullRequest, + scope: feedback_scope(), + pull_request_id: GitHubPullRequestIdV1::new("42").expect("pull request"), + }; + let mut refresh = Box::pin(advisory_remote_requests( + AdvisoryRemoteReadMode::ExplicitRefresh, + Some(github_request.clone()), + async move { + started_tx.send(()).expect("reader still owns the admission barrier"); + release_rx.await.expect("explicit discovery must settle") + }, + )); + tokio::select! { + result = &mut refresh => panic!("explicit refresh completed before its remote read: {result:?}"), + started = started_rx => started.expect("remote read started"), + } + release_tx + .send(ProductionCiFailureDiscoveryOutcomeV1::Denied) + .expect("remote read still awaited"); + let (github, ci) = refresh.await; + assert_eq!(github, Some(github_request)); + assert!(matches!(ci, ProductionCiFailureDiscoveryOutcomeV1::Denied)); +} + #[test] fn hook_notice_registration_is_released_with_the_published_owner() { let scope = feedback_scope();