From d3fcfb29774552e5a55f8f896a523c5e7d030a99 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 08:58:31 +0900 Subject: [PATCH 01/14] fix: keep mixed Subjektiv Runtime waits off the sole DB worker --- crates/workspace-server/src/server.rs | 59 +-- .../src/server_db_execution_tests.rs | 370 +++++++++++++++++- 2 files changed, 395 insertions(+), 34 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 07a34f929..46e52a8e6 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -20521,8 +20521,8 @@ async fn scoped_create_subjektiv_subject( AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult<(StatusCode, Json)> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let role = crate::subjektiv::SubjectRole::new(request.role) @@ -20553,8 +20553,8 @@ async fn scoped_list_subjektiv_subjects( AxumPath(path): AxumPath, Query(query): Query, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let limit = query.limit.unwrap_or(20); @@ -20622,8 +20622,8 @@ async fn scoped_get_subjektiv_subject( State(api): State, AxumPath(path): AxumPath, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let subject = open_subjektiv_store(&api)? @@ -20640,8 +20640,8 @@ async fn scoped_update_subjektiv_subject_behavior( AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; crate::subjektiv::validate_subject_behavior(&request.behavior_md) @@ -20800,8 +20800,8 @@ async fn scoped_retire_subjektiv_subject( State(api): State, AxumPath(path): AxumPath, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let subject = open_subjektiv_store(&api)? @@ -20820,8 +20820,8 @@ async fn scoped_stage_subjektiv_candidate( context: server_api::ServerRequestContext, Json(mut request): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let attribution = require_existing_subjektiv_session_attribution( @@ -20885,8 +20885,8 @@ async fn scoped_record_subjektiv_session( context: server_api::ServerRequestContext, Json(request): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let attribution = if request.create_if_missing { @@ -21006,8 +21006,8 @@ async fn scoped_subjektiv_session_backend( context: server_api::ServerRequestContext, Json(request): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let (subject_id, controller, authority) = @@ -22251,8 +22251,8 @@ async fn scoped_subjektiv_memory_backend( context: server_api::ServerRequestContext, Json(request): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let (subject_id, source_worker, authority) = @@ -22402,13 +22402,22 @@ async fn scoped_subjektiv_memory_consolidation( context: server_api::ServerRequestContext, Json(operation): Json, ) -> ApiResult> { - validate_workspace_scope(&api, &path.workspace_id)?; - let (subject_id, _, authority) = subjektiv_subject_scope(&api, &path.workspace_id, &context)?; - require_subjektiv_worker_authority( - authority, - SubjektivWorkerAuthority::Subject, - "consolidation request", - )?; + let scope_api = api.clone(); + let subject_id = api + .host_executors + .host()? + .run(move || { + validate_workspace_scope(&scope_api, &path.workspace_id)?; + let (subject_id, _, authority) = + subjektiv_subject_scope(&scope_api, &path.workspace_id, &context)?; + require_subjektiv_worker_authority( + authority, + SubjektivWorkerAuthority::Subject, + "consolidation request", + )?; + Ok::<_, ApiError>(subject_id) + }) + .await?; Ok(Json( start_subjektiv_staging_consolidation(api, &subject_id, operation).await?, )) diff --git a/crates/workspace-server/src/server_db_execution_tests.rs b/crates/workspace-server/src/server_db_execution_tests.rs index e9a6538f2..24e613341 100644 --- a/crates/workspace-server/src/server_db_execution_tests.rs +++ b/crates/workspace-server/src/server_db_execution_tests.rs @@ -29,6 +29,8 @@ struct Fixture { server: WorkspaceServerApi, api: WorkspaceApi, executor: DbExecutor, + host_executor: DbExecutor, + store: Arc, _dir: tempfile::TempDir, } @@ -90,10 +92,22 @@ impl Fixture { revoked_at: None, }) .unwrap(); - let api = WorkspaceApi::new_for_test(config.clone(), store.clone()) - .await - .unwrap(); - let server = WorkspaceServerApi::new_for_test(config, store); + // Explicit small Host budget proves the actual mixed mutation owner; + // the production/default fixture limits remain 2 workers/16 queued. + let host_executor = DbExecutor::new(DbExecutorLimits { + workers: NonZeroUsize::new(1).unwrap(), + queued: NonZeroUsize::new(1).unwrap(), + }) + .unwrap(); + let host_executors = HostExecutors { + host: Arc::new(std::sync::OnceLock::from(Some(host_executor.clone()))), + ..Default::default() + }; + let api = + WorkspaceApi::new_with_host_executors(config.clone(), store.clone(), host_executors) + .await + .unwrap(); + let server = WorkspaceServerApi::new_for_test(config, store.clone()); // Cache only the real inner router; avoid starting the unrelated hook. server .apis @@ -110,6 +124,8 @@ impl Fixture { server, api, executor, + host_executor, + store, _dir: dir, } } @@ -527,9 +543,9 @@ async fn authenticated_subjektiv_mutation_waits_for_commit_before_response() { assert_eq!(body["role"], "locked-subject", "{body}"); assert!( body["memory_fingerprint"].is_string(), - "response assembly must finish in the DB operation: {body}" + "response assembly must finish in the admitted mutation operation: {body}" ); - fixture.executor.drain().await.unwrap(); + fixture.host_executor.drain().await.unwrap(); assert_eq!(subject_count(&path, "locked-subject"), 1); } @@ -556,7 +572,7 @@ async fn cancelled_http_waiter_keeps_deletion_pending_until_sql_finishes() { assert_eq!(subject_count(&path, "cancelled-subject"), 0); lock.release(); busy.resume(); - fixture.executor.drain().await.unwrap(); + fixture.host_executor.drain().await.unwrap(); let deletion = observed(deletion).await; assert_eq!( subject_count(&path, "cancelled-subject"), @@ -603,7 +619,7 @@ async fn cancelled_queued_http_mutation_keeps_capacity_until_commit_and_drain() ); assert_eq!(subject_count(&path, "queued-subject"), 0); assert_eq!(subject_count(&path, "rejected-subject"), 0); - let mut drained = Box::pin(fixture.executor.drain()); + let mut drained = Box::pin(fixture.host_executor.drain()); assert!(futures::poll!(&mut drained).is_pending()); lock.release(); busy.resume(); @@ -621,7 +637,7 @@ async fn cancelled_queued_http_mutation_keeps_capacity_until_commit_and_drain() ); let retry = fixture.dispatch(Fixture::subject_request("after-drain")); response_json(observed(retry).await.unwrap(), StatusCode::CREATED).await; - fixture.executor.drain().await.unwrap(); + fixture.host_executor.drain().await.unwrap(); assert_eq!( subject_count(&path, "after-drain"), 1, @@ -767,3 +783,339 @@ async fn restore_intent_pins_once_after_write_ownership_without_a_deferred_read_ "one admitted intent must persist exactly once, with no operation retry" ); } + +fn install_remote_fixture( + fixture: &Fixture, + runtime_id: &str, + base_url: &str, + signed: bool, +) -> crate::store::WorkspaceRuntimeBinding { + let identity = worker_runtime::auth::RuntimeIdentityMaterial::generate(runtime_id).unwrap(); + fixture + .store + .upsert_workspace_runtime_binding( + crate::store::WorkspaceRuntimeBinding { + workspace_id: WORKSPACE.into(), + runtime_id: runtime_id.into(), + display_name: "Held Runtime".into(), + base_url: base_url.into(), + public_key: identity.public_key, + public_key_fingerprint: String::new(), + binding_id: String::new(), + authentication_mode: if signed { + crate::store::WorkspaceRuntimeAuthenticationMode::WorkspaceIdentity + } else { + crate::store::WorkspaceRuntimeAuthenticationMode::LegacyServerIssuer + }, + created_at: "2026-01-01T00:00:00Z".into(), + updated_at: "2026-01-01T00:00:00Z".into(), + revoked_at: None, + }, + false, + ) + .unwrap(); + let binding = fixture + .store + .get_workspace_runtime_binding(WORKSPACE, runtime_id) + .unwrap() + .unwrap(); + // This narrow fixture exercises the provider's own live authority callback; + // it does not start the unrelated subscription/projection reconnect producer. + fixture.api.runtime.set_runtime_binding_gate(|_| true); + let mut config = crate::hosts::RemoteRuntimeConfig::new( + runtime_id, + "Held Runtime", + base_url, + Some("must-not-fall-back".into()), + ) + .with_timeout(WATCHDOG); + if signed { + fixture + .api + .signing_identities + .provision_existing(WORKSPACE, &format!("account-{WORKSPACE}")) + .unwrap(); + config.workspace_authorization = Some(WorkspaceRuntimeAuthorization::new( + fixture.store.clone(), + fixture.api.signing_identities.clone(), + "http://localhost:8787", + binding.clone(), + )); + } + fixture.api.runtime.register_or_replace( + RemoteWorkerRuntime::new(config, WORKSPACE.into(), "http://localhost:8787".into()).unwrap(), + ); + binding +} + + + +// Loopback fake at the Session/Worker Runtime HTTP boundary. It owns one reply, +// released by an explicit event; Drop unblocks accept/read and joins on failure. +struct HeldRuntimeReply { + address: std::net::SocketAddr, + entered: Option>, + release: Option>, + thread: Option>, +} +impl HeldRuntimeReply { + fn new() -> Self { + use std::io::{Read, Write}; + let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); + let address = listener.local_addr().unwrap(); + let (entered, received) = oneshot::channel(); + let (release, permit) = mpsc::channel(); + let thread = std::thread::spawn(move || { + let Ok((mut stream, _)) = listener.accept() else { + return; + }; + stream.set_read_timeout(Some(WATCHDOG)).unwrap(); + let mut buf = [0; 4096]; + let Ok(n) = stream.read(&mut buf) else { return }; + if n == 0 { + return; + } + let _ = entered.send( + String::from_utf8_lossy(&buf[..n]) + .lines() + .next() + .unwrap() + .to_owned(), + ); + let _ = permit.recv_timeout(WATCHDOG); + let _ = stream.write_all(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + }); + Self { + address, + entered: Some(received), + release: Some(release), + thread: Some(thread), + } + } + fn finish(&mut self) { + self.release.take(); + let _ = std::net::TcpStream::connect(self.address); + if let Some(thread) = self.thread.take() { + thread.join().unwrap(); + } + } +} +impl Drop for HeldRuntimeReply { + fn drop(&mut self) { + self.finish(); + } +} + +struct CountJobLifetime(Arc); +struct CountJobLease(Arc); +impl DbOperationLifetime for CountJobLifetime { + fn retain(&self) -> Box { + self.0.fetch_add(1, Ordering::SeqCst); + Box::new(CountJobLease(self.0.clone())) + } +} +impl Drop for CountJobLease { + fn drop(&mut self) { + self.0.fetch_sub(1, Ordering::SeqCst); + } +} + +#[tokio::test(flavor = "current_thread")] +async fn subjektiv_runtime_waits_leave_db_and_control_free_and_cancel_retains_the_actual_host_lease() + { + // Different mixed helpers carry this rule: Session Read/Search, subject + // projection, and legacy consolidation scope authorization (even refusal). + for kind in ["read", "search", "subject", "legacy_scope"] { + let fixture = Fixture::new().await; + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "subject-runtime", + &format!("http://{}", reply.address), + false, + ); + let Json(subject) = scoped_create_subjektiv_subject( + State(fixture.api.clone()), + AxumPath(ScopedWorkspacePath { + workspace_id: WORKSPACE.into(), + }), + Json(server_api::SubjektivSubjectCreateRequest { + role: "network subject".into(), + behavior_md: String::new(), + }), + ) + .await + .unwrap() + .1; + let singleton = if kind == "legacy_scope" { + format!("{SUBJEKTIV_CONSOLIDATION_SINGLETON_PREFIX}{}", subject.id) + } else { + crate::subjektiv::subject_worker_singleton_key(&subject.id).unwrap() + }; + let memory = fixture + .store + .get_workspace_memory_settings(WORKSPACE) + .unwrap(); + let reservation = fixture + .store + .reserve_worker_create( + WORKSPACE, + "subject-runtime", + "network-fixture", + "network-fixture-fingerprint", + Some(&singleton), + &memory, + ) + .unwrap(); + let worker = RuntimeWorkerRef::new("subject-runtime", reservation.worker_id.to_string()); + fixture + .store + .upsert_worker_registry(&WorkerRegistryRecord { + workspace_id: WORKSPACE.into(), + worker: worker.clone(), + display_name: "network fixture".into(), + profile: None, + retention_state: "normal".into(), + transcript_ref: None, + session_ref: None, + summary_ref: None, + diagnostics_ref: None, + created_at: "2026-01-01T00:00:00Z".into(), + updated_at: "2026-01-01T00:00:00Z".into(), + }) + .unwrap(); + fixture + .store + .complete_worker_create_reservation(WORKSPACE, reservation.worker_id) + .unwrap(); + open_subjektiv_store(&fixture.api) + .unwrap() + .record_session_attribution( + crate::subjektiv::SubjectSessionAttribution::new( + subject.id.clone(), + worker.runtime_id.clone(), + worker.worker_id.clone(), + "session", + ) + .unwrap(), + ) + .unwrap(); + fixture + .store + .create_worker_control_grant(&WorkerControlGrantRecord { + workspace_id: WORKSPACE.into(), + grant_id: "session-observe".into(), + controller: worker.clone(), + subject: worker.clone(), + relation: "subject_session".into(), + origin: "test".into(), + permissions: vec!["observe".into()], + operation_id: "session-observe".into(), + created_at: "2026-01-01T00:00:00Z".into(), + revoked_at: None, + }) + .unwrap(); + let context = server_api::ServerRequestContext { + actor: None, + worker_source: None, + runtime_source: Some(server_api::ServerRuntimeSource { + authentication_binding_id: "fixture-binding".into(), + runtime_id: worker.runtime_id, + worker_id: Some(worker.worker_id), + }), + origin: None, + transport_headers: Vec::new(), + }; + let counts = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let lifetime = Arc::new(CountJobLifetime(counts.clone())); + let api = fixture.api.clone(); + let task = tokio::spawn(crate::db_executor::with_operation_lifetime( + lifetime, + async move { + if kind == "subject" { + scoped_get_subjektiv_subject( + State(api), + AxumPath(ScopedSubjektivSubjectPath { + workspace_id: WORKSPACE.into(), + subject_id: subject.id, + }), + ) + .await + .map(|_| ()) + } else { + let operation = match kind { + "read" => server_api::SubjektivSessionBackendOperation::Read( + server_api::SubjektivSessionReadRequest { + session_id: "session".into(), + segment_id: "segment".into(), + entry_ref: "E1".into(), + mode: Default::default(), + cursor: None, + }, + ), + "search" => server_api::SubjektivSessionBackendOperation::Search( + serde_json::from_value( + serde_json::json!({"session_id":"session","query":"needle"}), + ) + .unwrap(), + ), + _ => server_api::SubjektivSessionBackendOperation::List( + serde_json::from_value(serde_json::json!({})).unwrap(), + ), + }; + scoped_subjektiv_session_backend( + State(api), + AxumPath(ScopedWorkspacePath { + workspace_id: WORKSPACE.into(), + }), + context, + Json(server_api::SubjektivSessionBackendRequest { operation }), + ) + .await + .map(|_| ()) + } + }, + )); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!( + request.contains(if kind == "read" || kind == "search" { + "/v1/session-public/" + } else { + "/v1/workers/" + }), + "{kind}: {request}" + ); + let read_store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || read_store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; // timer contract, not a wait guess + assert!( + !task.is_finished(), + "{kind} completed before upstream release" + ); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + assert_eq!( + counts.load(Ordering::SeqCst), + 1, + "{kind}: cancellation lost actual Host authority" + ); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + assert_eq!( + counts.load(Ordering::SeqCst), + 0, + "{kind}: completed Host job leaked its lease" + ); + fixture.executor.drain().await.unwrap(); + } +} From c0223a0dea847af626fa800e4e6da286b535eec0 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 08:58:34 +0900 Subject: [PATCH 02/14] fix: await owned cold protocol signing and propagate authorization failure --- crates/workspace-server/src/hosts.rs | 53 +++++++++------ crates/workspace-server/src/server.rs | 46 +++++++++---- .../src/server_db_execution_tests.rs | 65 ++++++++++++++++++- 3 files changed, 128 insertions(+), 36 deletions(-) diff --git a/crates/workspace-server/src/hosts.rs b/crates/workspace-server/src/hosts.rs index 0b41a9a47..429ece684 100644 --- a/crates/workspace-server/src/hosts.rs +++ b/crates/workspace-server/src/hosts.rs @@ -1297,8 +1297,8 @@ pub trait WorkspaceWorkerRuntime: Send + Sync { fn observation_source( &self, _worker_id: &str, - ) -> Option { - None + ) -> Result, RuntimeDiagnostic> { + Ok(None) } fn send_input(&self, worker_id: &str, _request: WorkerInputRequest) -> WorkerInputResult { @@ -2236,6 +2236,11 @@ impl RuntimeRegistry { let runtime = self.runtime(runtime_id)?; runtime .observation_source(worker_id) + .map_err(|diagnostic| RuntimeRegistryError::RuntimeOperationFailed { + runtime_id: runtime_id.to_owned(), + code: diagnostic.code, + message: diagnostic.message, + })? .ok_or_else(|| RuntimeRegistryError::UnknownWorker { worker: worker.clone(), }) @@ -3203,17 +3208,21 @@ impl WorkspaceWorkerRuntime for EmbeddedWorkerRuntime { fn observation_source( &self, worker_id: &str, - ) -> Option { - let worker_ref = self.worker_ref(worker_id)?; + ) -> Result, RuntimeDiagnostic> { + let Some(worker_ref) = self.worker_ref(worker_id) else { + return Ok(None); + }; if self.runtime.worker_detail(&worker_ref).is_err() { - return None; + return Ok(None); } - Some(crate::observation::RuntimeObservationSource::embedded( - crate::observation::EmbeddedRuntimeObservationSource { - worker: RuntimeWorkerRef::new(&self.runtime_id, worker_id), - runtime: self.runtime.clone(), - worker_ref, - }, + Ok(Some( + crate::observation::RuntimeObservationSource::embedded( + crate::observation::EmbeddedRuntimeObservationSource { + worker: RuntimeWorkerRef::new(&self.runtime_id, worker_id), + runtime: self.runtime.clone(), + worker_ref, + }, + ), )) } @@ -5324,20 +5333,22 @@ impl WorkspaceWorkerRuntime for RemoteWorkerRuntime { fn observation_source( &self, worker_id: &str, - ) -> Option { + ) -> Result, RuntimeDiagnostic> { let path = format!("/v1/workers/{worker_id}/protocol/ws"); let bearer_token = match &self.workspace_authorization { - Some(authorization) => authorization - .issue("GET", &path, "workers:protocol", Some(worker_id), &[]) - .ok(), + Some(authorization) => { + Some(authorization.issue("GET", &path, "workers:protocol", Some(worker_id), &[])?) + } None => self.bearer_token.clone(), }; - Some(crate::observation::RuntimeObservationSource::remote_ws( - crate::observation::RuntimeObservationSourceConfig { - worker: RuntimeWorkerRef::new(&self.runtime_id, worker_id), - endpoint: self.ws_endpoint(worker_id), - bearer_token, - }, + Ok(Some( + crate::observation::RuntimeObservationSource::remote_ws( + crate::observation::RuntimeObservationSourceConfig { + worker: RuntimeWorkerRef::new(&self.runtime_id, worker_id), + endpoint: self.ws_endpoint(worker_id), + bearer_token, + }, + ), )) } diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 46e52a8e6..8376cdbfc 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -31468,20 +31468,7 @@ pub(crate) async fn connect_workspace_worker_protocol( worker: &RuntimeWorkerRef, input_source: Option<&protocol::AuthenticatedInputSource>, ) -> Result { - let source = match api.observation_proxy.source(worker) { - Ok(source) => source, - Err(ObservationProxyError::WorkerNotFound(_)) => api - .runtime - .observation_source(worker) - .map_err(|error| error.into_error())?, - Err(error) => { - return Err(Error::RuntimeOperationFailed { - runtime_id: worker.runtime_id.clone(), - code: error.code().to_string(), - message: error.message().to_string(), - }); - } - }; + let source = workspace_worker_protocol_source(api, worker).await?; match source { RuntimeObservationSource::RemoteWs(config) => { connect_remote_worker_protocol(config, input_source).await @@ -31494,6 +31481,37 @@ pub(crate) async fn connect_workspace_worker_protocol( } } +// Cold source discovery includes live authority/signing reads. Keep the whole +// synchronous callback on its admitted DB owner, not the Tokio WS task. +async fn workspace_worker_protocol_source( + api: &WorkspaceApi, + worker: &RuntimeWorkerRef, +) -> Result { + let source = match api.observation_proxy.source(worker) { + Ok(source) => source, + Err(ObservationProxyError::WorkerNotFound(_)) => { + let runtime = api.runtime.clone(); + let worker = worker.clone(); + api.store + .db_executor() + .run(move || { + runtime + .observation_source(&worker) + .map_err(|error| error.into_error()) + }) + .await? + } + Err(error) => { + return Err(Error::RuntimeOperationFailed { + runtime_id: worker.runtime_id.clone(), + code: error.code().to_string(), + message: error.message().to_string(), + }); + } + }; + Ok(source) +} + fn insert_authenticated_input_source_header( headers: &mut HeaderMap, input_source: Option<&protocol::AuthenticatedInputSource>, diff --git a/crates/workspace-server/src/server_db_execution_tests.rs b/crates/workspace-server/src/server_db_execution_tests.rs index 24e613341..53d94da0f 100644 --- a/crates/workspace-server/src/server_db_execution_tests.rs +++ b/crates/workspace-server/src/server_db_execution_tests.rs @@ -848,7 +848,70 @@ fn install_remote_fixture( binding } - +#[tokio::test(flavor = "current_thread")] +async fn cold_protocol_source_signing_waits_off_tokio_and_never_falls_back_on_overload_or_revocation() + { + let fixture = Fixture::new().await; + let binding = install_remote_fixture(&fixture, "protocol-runtime", "http://127.0.0.1:9", true); + let worker = RuntimeWorkerRef::new(&binding.runtime_id, "source-worker"); + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let writer = fixture.store.clone(); + fixture + .executor + .dispatch(move || { + writer + .mark_api_token_used("missing-token", "2026-01-01T00:00:00Z") + .unwrap() + }) + .unwrap(); + observed(waiting).await.unwrap(); // SQL holds the actual shared connection mutex. + let mut source = Box::pin(workspace_worker_protocol_source(&fixture.api, &worker)); + assert!(futures::poll!(&mut source).is_pending()); + runtime_progress_while_locked(&lock).await; + assert!(futures::poll!(&mut source).is_pending()); + assert!( + matches!( + workspace_worker_protocol_source(&fixture.api, &worker).await, + Err(Error::DbCapacityExceeded) + ), + "overload must not return an unsigned or legacy-bearer source" + ); + lock.release(); + busy.resume(); + let RuntimeObservationSource::RemoteWs(config) = observed(source).await.unwrap() else { + panic!("remote source expected") + }; + let first = config + .bearer_token + .expect("fresh Workspace capability is required"); + assert_ne!(first, "must-not-fall-back"); + let RuntimeObservationSource::RemoteWs(second) = + workspace_worker_protocol_source(&fixture.api, &worker) + .await + .unwrap() + else { + panic!("remote source expected") + }; + assert_ne!( + first, + second.bearer_token.unwrap(), + "cold lookup must sign a fresh request, not cache authentication success" + ); + fixture + .store + .revoke_workspace_runtime_binding(WORKSPACE, &binding.runtime_id, "2026-01-02T00:00:00Z") + .unwrap(); + let error = workspace_worker_protocol_source(&fixture.api, &worker) + .await + .err() + .expect("revoked authority cannot produce any source"); + assert!( + matches!(error, Error::RuntimeOperationFailed { ref code, .. } if code == "workspace_runtime_authorization_stale"), + "{error:?}" + ); + fixture.executor.drain().await.unwrap(); +} // Loopback fake at the Session/Worker Runtime HTTP boundary. It owns one reply, // released by an explicit event; Drop unblocks accept/read and joins on failure. From b10b731531ac3661dcf7f6f9a8906ae72af03958 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 09:06:36 +0900 Subject: [PATCH 03/14] docs: record review boundary fixes and exact-source verification --- docs/development/backend-db-execution.md | 16 +++++-- .../report/2026-10-12-t748-suite-diagnosis.md | 45 ++++++++++++++++++- 2 files changed, 57 insertions(+), 4 deletions(-) diff --git a/docs/development/backend-db-execution.md b/docs/development/backend-db-execution.md index 8dd00b478..7fd4a58d6 100644 --- a/docs/development/backend-db-execution.md +++ b/docs/development/backend-db-execution.md @@ -58,6 +58,11 @@ owned workflow needs async resource locks, `run_host_workflow` polls it on a Hos worker using the original Tokio handle; do not build a replacement runtime for cached services and do not submit to and await the Host lane from inside itself. Control/session guards needed by an accepted mutation belong to that actual job. +Subjektiv session Read/Search, subject response projection, memory staging and +legacy consolidation-scope helpers also use Host ownership: they can synchronously +call Runtime/network services outside their SQL transactions. Pure DB-resident +Subjektiv scope and memory projection helpers stay on the DB lane. Host admission +must not be acquired again from an already-owned Host workflow. Backend Job cleanup has a third finite lane (two workers, 16 queued jobs), so a long Run teardown wait cannot starve ordinary Host control/stop requests. It is @@ -69,9 +74,14 @@ unknown DB outcome. Runtime teardown is still its own lifecycle fence, not proof of model processing. Remote subscription and Workdir authorization are async callbacks: their complete -live binding check and fresh request signing are offloaded together. A queue error -must not fall back to a configured bearer token. Synchronous CLI/migration callers -and existing synchronous Runtime adapters retain their explicit blocking boundary. +live binding check and fresh request signing are offloaded together. Cold protocol +source lookup in `connect_workspace_worker_protocol` is also awaited as an owned +DB stage, rather than invoking its synchronous signing callback on Tokio. +`WorkspaceWorkerRuntime::observation_source` is fallible: live binding/signing +failure and bounded admission errors propagate explicitly, not as a missing +source or a fallback to a configured bearer token. Synchronous CLI/migration +callers and existing synchronous Runtime adapters retain their explicit blocking +boundary. ## Caller recipe (including assignment APIs) diff --git a/docs/report/2026-10-12-t748-suite-diagnosis.md b/docs/report/2026-10-12-t748-suite-diagnosis.md index 3e73e67c2..ff78dc17f 100644 --- a/docs/report/2026-10-12-t748-suite-diagnosis.md +++ b/docs/report/2026-10-12-t748-suite-diagnosis.md @@ -148,7 +148,7 @@ The fresh candidate build listed the expected executor tests and total test coun ## Validation and remaining composition boundary -Final code source `49bfbfb2` passed the full default-parallel Server suite, +Pre-review code source `49bfbfb2` passed the full default-parallel Server suite, including the command-overload case: 960 passed, zero failures, one existing ignored migration fixture; 11 binary tests, library duration 175.29 seconds. The only captured panics were the three deliberate unknown-outcome tests. @@ -159,6 +159,49 @@ These commands used the isolated candidate artifact directory; libtest default parallelism and the production budgets were unchanged. The MR records the exact published source, which also includes this report and the boundary document. +## Post-publication independent review and causal fixes + +MR `01a12d3f-e90c-7b02-9ed9-cf3bd2e822a1` seq3 independently reviewed published +source `66bb9d0b` and requested changes despite the successful original suite. +Its two major findings exposed boundaries the earlier tests did not prove: + +- `T748-DB-NETWORK-LANE`: mixed Subjektiv helpers could wait for synchronous + Runtime HTTP on the sole DB worker. Fix `d3fcfb29` places session read/search, + subject projection, memory staging, and legacy consolidation-scope helpers on + the existing finite Host owner; pure DB-resident operations keep DB placement. + Whole transaction helpers remain intact, and no Runtime I/O is introduced + inside a SQL transaction. +- `T748-ASYNC-PROTOCOL-SIGNING`: cold protocol source lookup called live binding + checks and signing directly on Tokio. Fix `c0223a0d` awaits the owned DB stage. + The source API now propagates failure explicitly: an invalid live binding or + signing/admission failure cannot be hidden as `None` or a legacy bearer fallback. + +Permanent real loopback HTTP regressions hold upstream Runtime replies during +session read/search, subject and legacy-scope requests, while independently +observing SQL, timer and control progress. Cancelling waiters retains the actual +Host lifecycle lease until completion. Cold protocol source tests control another +SQLite writer and shared connection mutex wait, and verify bounded overload, +fresh signing and revocation without fallback. Existing queue/cancellation and +deletion evidence now observes the actual mutation owner; the mixed fixtures +explicitly inject a small finite Host budget. Production budgets are unchanged. + +Focused boundary tests passed (10). Restoring the old placement produced a +failing regression, and fixed placement passed; an earlier syntax-rejected +instrumentation attempt is excluded from evidence. Diagnostic artifacts under +`target/` are not production source or replacement domain implementations. + +Exact code source `c0223a0d` then passed root `cargo check` and the original +**default-parallel** Server suite: **962 passed, zero failed, one pre-existing +ignored**, 11 binary tests; library duration **173.36 seconds**. Only the three +intentional executor panic diagnostics appeared, with no residual test processes. +Workdir default (153) and `http-client` (156) full suites, format/diff checks and +state-contract naming (558 matching lines, zero unapproved, zero stale) passed. +All used the isolated `target/t748-candidate-build` artifact directory. Logs: +`target/t748-review-fix-{root-check,full-suite,workdir-default,workdir-http,format,names}.log`. +The following documentation-only commit does not change that tested code. +Fresh independent review of the normally advanced same MR source is still +required; previous `request_changes` evidence is not approval of this fix. + No latency benchmark, production incident measurement, target integration, Ticket conclusion, or running-environment update is claimed. T743 has now published its caller inventory/source; its owner will compose the T748 facade and From a2487b575df1f348251214c53da61595860fcb40 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 09:41:50 +0900 Subject: [PATCH 04/14] fix: keep mandatory Subject callbacks independent of waiting Host parents --- crates/workspace-server/src/server.rs | 221 +++++++++----- .../src/server/tests/subject_spawn_tests.rs | 288 ++++++++++++++---- 2 files changed, 371 insertions(+), 138 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 8376cdbfc..e20636a53 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -20372,11 +20372,26 @@ fn require_dedicated_subjektiv_consolidation_worker( } } -fn subjektiv_subject_scope( +// Store-only authority discovery. A mandatory Session attribution callback must +// reject consolidation roles without consulting Runtime: its create/restore parent +// can already hold a Host worker while Runtime waits for that callback to finish. +enum SubjektivStoredScopeAuthority { + Subject, + JobConsolidation, + LegacyConsolidation { singleton_key: String }, +} + +struct SubjektivStoredWorkerScope { + subject_id: String, + worker: RuntimeWorkerRef, + authority: SubjektivStoredScopeAuthority, +} + +fn subjektiv_worker_scope_from_store( api: &WorkspaceApi, workspace_id: &str, context: &server_api::ServerRequestContext, -) -> ApiResult<(String, RuntimeWorkerRef, SubjektivWorkerAuthority)> { +) -> ApiResult { let source = context.runtime_source.as_ref().ok_or_else(|| { Error::WorkspacePermissionDenied( "subjektiv operations require authenticated Runtime-owned source authority".to_string(), @@ -20389,11 +20404,11 @@ fn subjektiv_subject_scope( })?; let worker = RuntimeWorkerRef::new(&source.runtime_id, worker_id); if let Some(grant) = subjektiv_job::active_grant(api, workspace_id, &worker)? { - return Ok(( - grant.subject_id, + return Ok(SubjektivStoredWorkerScope { + subject_id: grant.subject_id, worker, - SubjektivWorkerAuthority::Consolidation, - )); + authority: SubjektivStoredScopeAuthority::JobConsolidation, + }); } let lease = api .store @@ -20404,15 +20419,47 @@ fn subjektiv_subject_scope( ) })?; let (subject_id, authority) = subjektiv_scope_from_singleton_key(&lease.key)?; - if authority == SubjektivWorkerAuthority::Consolidation { - let mut summary = api - .runtime - .worker(&worker) - .map_err(|error| error.into_error())?; - summary.singleton_key = Some(lease.key.clone()); - require_dedicated_subjektiv_consolidation_worker(&summary, workspace_id, &lease.key)?; - } - Ok((subject_id.to_string(), worker, authority)) + let authority = match authority { + SubjektivWorkerAuthority::Subject => SubjektivStoredScopeAuthority::Subject, + SubjektivWorkerAuthority::Consolidation => { + SubjektivStoredScopeAuthority::LegacyConsolidation { + singleton_key: lease.key.clone(), + } + } + }; + Ok(SubjektivStoredWorkerScope { + subject_id: subject_id.to_owned(), + worker, + authority, + }) +} + +// Mixed callers run this complete policy boundary on Host. Only legacy +// consolidation authority needs synchronous Runtime profile/tag verification. +fn subjektiv_subject_scope( + api: &WorkspaceApi, + workspace_id: &str, + context: &server_api::ServerRequestContext, +) -> ApiResult<(String, RuntimeWorkerRef, SubjektivWorkerAuthority)> { + let scope = subjektiv_worker_scope_from_store(api, workspace_id, context)?; + let authority = match scope.authority { + SubjektivStoredScopeAuthority::Subject => SubjektivWorkerAuthority::Subject, + SubjektivStoredScopeAuthority::JobConsolidation => SubjektivWorkerAuthority::Consolidation, + SubjektivStoredScopeAuthority::LegacyConsolidation { singleton_key } => { + let mut summary = api + .runtime + .worker(&scope.worker) + .map_err(|error| error.into_error())?; + summary.singleton_key = Some(singleton_key.clone()); + require_dedicated_subjektiv_consolidation_worker( + &summary, + workspace_id, + &singleton_key, + )?; + SubjektivWorkerAuthority::Consolidation + } + }; + Ok((scope.subject_id, scope.worker, authority)) } fn require_subjektiv_worker_authority( @@ -20436,8 +20483,8 @@ fn subjektiv_session_attribution( context: &server_api::ServerRequestContext, session_id: &str, ) -> ApiResult { - let (subject_id, worker, authority) = subjektiv_subject_scope(api, workspace_id, context)?; - if authority != SubjektivWorkerAuthority::Subject { + let scope = subjektiv_worker_scope_from_store(api, workspace_id, context)?; + if !matches!(scope.authority, SubjektivStoredScopeAuthority::Subject) { return Err(Error::WorkspacePermissionDenied( "subject Session attribution requires the current subject Worker".into(), ) @@ -20448,9 +20495,9 @@ fn subjektiv_session_attribution( return Err(Error::InvalidInput("committed session_id is invalid".to_string()).into()); } crate::subjektiv::SubjectSessionAttribution::new( - subject_id, - worker.runtime_id, - worker.worker_id, + scope.subject_id, + scope.worker.runtime_id, + scope.worker.worker_id, session_id, ) .map_err(|error| Error::InvalidInput(error.to_string()).into()) @@ -20885,8 +20932,11 @@ async fn scoped_record_subjektiv_session( context: server_api::ServerRequestContext, Json(request): Json, ) -> ApiResult> { - api.host_executors - .host()? + // Runtime create/restore waits for this mandatory callback while holding + // Host capacity. Its Subject-only authority and whole attribution SQL must + // use the independent DB lane, never the parent's waiting Host lane. + api.store + .db_executor() .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let attribution = if request.create_if_missing { @@ -22251,63 +22301,86 @@ async fn scoped_subjektiv_memory_backend( context: server_api::ServerRequestContext, Json(request): Json, ) -> ApiResult> { - api.host_executors - .host()? + use server_api::SubjektivMemoryBackendOperation as Op; + // Subject-only operations are complete DB boundaries. This includes resident + // context callbacks during Runtime create/restore; they must not queue behind + // the Host parents waiting for them. Other roles can need legacy Runtime checks. + let (expected, label) = match &request.operation { + Op::ResidentSummary(_) => ( + Some(SubjektivWorkerAuthority::Subject), + "resident surface read", + ), + Op::ResidentContext(_) => ( + Some(SubjektivWorkerAuthority::Subject), + "resident subject context read", + ), + Op::ValidateProposal(_) => ( + Some(SubjektivWorkerAuthority::Subject), + "Memory proposal validation", + ), + Op::ReceiptStatus(_) => ( + Some(SubjektivWorkerAuthority::Subject), + "explicit candidate receipt lookup", + ), + Op::StageExplicit(_) => ( + Some(SubjektivWorkerAuthority::Subject), + "explicit candidate staging", + ), + Op::ListCandidates(_) => ( + Some(SubjektivWorkerAuthority::Consolidation), + "candidate listing", + ), + Op::ReadCandidate(_) => ( + Some(SubjektivWorkerAuthority::Consolidation), + "candidate read", + ), + Op::DecideCandidate(_) => ( + Some(SubjektivWorkerAuthority::Consolidation), + "candidate decision", + ), + Op::PrepareSurface(_) => ( + Some(SubjektivWorkerAuthority::Consolidation), + "surface generation preparation", + ), + Op::PublishSurface(_) => ( + Some(SubjektivWorkerAuthority::Consolidation), + "surface publication", + ), + Op::FailSurface(_) => ( + Some(SubjektivWorkerAuthority::Consolidation), + "surface generation failure", + ), + Op::Query(_) | Op::Read(_) | Op::ListChanges(_) => (None, "recall"), + }; + let executor = if expected == Some(SubjektivWorkerAuthority::Subject) { + api.store.db_executor() + } else { + api.host_executors.host()? + }; + executor .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; - let (subject_id, source_worker, authority) = - subjektiv_subject_scope(&api, &path.workspace_id, &context)?; + let (subject_id, source_worker, authority) = if expected + == Some(SubjektivWorkerAuthority::Subject) + { + let scope = subjektiv_worker_scope_from_store(&api, &path.workspace_id, &context)?; + if !matches!(scope.authority, SubjektivStoredScopeAuthority::Subject) { + return Err(Error::WorkspacePermissionDenied(format!( + "subjektiv {label} is not authorized for this Worker role" + )) + .into()); + } + ( + scope.subject_id, + scope.worker, + SubjektivWorkerAuthority::Subject, + ) + } else { + subjektiv_subject_scope(&api, &path.workspace_id, &context)? + }; let store = open_subjektiv_store(&api)?; - use server_api::SubjektivMemoryBackendOperation as Op; // Authentication, live Worker/Job authority and committed Session provenance // remain Backend-owned. Only the domain operation/projection is shared. - let (expected, label) = match &request.operation { - Op::ResidentSummary(_) => ( - Some(SubjektivWorkerAuthority::Subject), - "resident surface read", - ), - Op::ResidentContext(_) => ( - Some(SubjektivWorkerAuthority::Subject), - "resident subject context read", - ), - Op::ValidateProposal(_) => ( - Some(SubjektivWorkerAuthority::Subject), - "Memory proposal validation", - ), - Op::ReceiptStatus(_) => ( - Some(SubjektivWorkerAuthority::Subject), - "explicit candidate receipt lookup", - ), - Op::StageExplicit(_) => ( - Some(SubjektivWorkerAuthority::Subject), - "explicit candidate staging", - ), - Op::ListCandidates(_) => ( - Some(SubjektivWorkerAuthority::Consolidation), - "candidate listing", - ), - Op::ReadCandidate(_) => ( - Some(SubjektivWorkerAuthority::Consolidation), - "candidate read", - ), - Op::DecideCandidate(_) => ( - Some(SubjektivWorkerAuthority::Consolidation), - "candidate decision", - ), - Op::PrepareSurface(_) => ( - Some(SubjektivWorkerAuthority::Consolidation), - "surface generation preparation", - ), - Op::PublishSurface(_) => ( - Some(SubjektivWorkerAuthority::Consolidation), - "surface publication", - ), - Op::FailSurface(_) => ( - Some(SubjektivWorkerAuthority::Consolidation), - "surface generation failure", - ), - Op::Query(_) | Op::Read(_) | Op::ListChanges(_) => (None, "recall"), - }; if let Some(expected) = expected { require_subjektiv_worker_authority(authority, expected, label)?; } diff --git a/crates/workspace-server/src/server/tests/subject_spawn_tests.rs b/crates/workspace-server/src/server/tests/subject_spawn_tests.rs index 37519b22c..f22bc4a9d 100644 --- a/crates/workspace-server/src/server/tests/subject_spawn_tests.rs +++ b/crates/workspace-server/src/server/tests/subject_spawn_tests.rs @@ -1,34 +1,45 @@ use super::*; -#[tokio::test] -async fn subject_session_callback_completes_while_create_and_deletion_are_waiting() { +#[tokio::test(flavor = "current_thread")] +async fn subject_callbacks_finish_before_waiting_host_parents_release_capacity() { let workspace = tempfile::tempdir().unwrap(); let mut api = test_api(workspace.path()).await; let identity = RuntimeIdentityMaterial::generate("runtime-test").unwrap(); configure_runtime_request_auth(&mut api, &identity, "runtime-test"); let subjektiv = open_subjektiv_store(&api).unwrap(); - let subject = subjektiv - .create_subject(crate::subjektiv::SubjectRole::new("companion").unwrap()) - .unwrap(); - let singleton_key = crate::subjektiv::subject_worker_singleton_key(&subject.id).unwrap(); let memory = api .config_store .get_workspace_memory_settings(TEST_WORKSPACE_ID) .unwrap(); - let reserved = api - .config_store - .reserve_worker_create( - TEST_WORKSPACE_ID, - "runtime-test", - "subject-start", - &"f".repeat(64), - Some(&singleton_key), - &memory, - ) - .unwrap(); - let worker_id = reserved.worker_id.to_string(); + let mut callbacks = Vec::new(); + for index in 0..2 { + let subject = subjektiv + .create_subject( + crate::subjektiv::SubjectRole::new(format!("companion-{index}")).unwrap(), + ) + .unwrap(); + let singleton_key = crate::subjektiv::subject_worker_singleton_key(&subject.id).unwrap(); + let reserved = api + .config_store + .reserve_worker_create( + TEST_WORKSPACE_ID, + "runtime-test", + &format!("subject-start-{index}"), + &"f".repeat(64), + Some(&singleton_key), + &memory, + ) + .unwrap(); + callbacks.push(( + subject.id, + reserved.worker_id.to_string(), + session_store::new_session_id().to_string(), + )); + } let server = WorkspaceServerApi::new_for_test(api.config.clone(), api.store.clone()); - // Reuse the fixture API, but exercise the real outer dispatcher and both auth layers. + // Real dispatcher, source proof authentication, live singleton authority and + // attribution SQL. Only Runtime's create/restore wait is modeled below; no + // product process or complete create endpoint is spawned by this route test. server .apis .lock() @@ -39,61 +50,210 @@ async fn subject_session_callback_completes_while_create_and_deletion_are_waitin build_inner_router(api.clone()), ); let gate = server.admission_for_workspace(TEST_WORKSPACE_ID).await; - let create = gate.admit(false).unwrap(); - let mut deletion = Box::pin(gate.lock_deletion()); - assert!(futures::poll!(&mut deletion).is_pending()); + let host = api.host_executors.host().unwrap(); // production 2 workers/16 queued + let mut parents = Vec::new(); + let mut finish_callbacks = Vec::new(); + for _ in 0..2 { + let create = gate.admit(false).unwrap(); + let (entered, ready) = tokio::sync::oneshot::channel(); + let (finish_callback, callback_reply) = std::sync::mpsc::channel(); + finish_callbacks.push(finish_callback); + parents.push(tokio::spawn(run_host_workflow(host.clone(), async move { + let _create = create; + entered.send(()).unwrap(); + // The actual Host parent cannot finish until its mandatory callbacks + // return. Timeout/disconnection is failure cleanup, never success. + callback_reply + .recv_timeout(std::time::Duration::from_secs(5)) + .map_err(|error| { + Error::InvalidInput(format!("Runtime callback did not complete: {error}")) + })?; + Ok::<_, Error>(()) + }))); + tokio::time::timeout(std::time::Duration::from_secs(2), ready) + .await + .unwrap() + .unwrap(); + } let app = workspace_server_router(server); let path = format!("/api/w/{TEST_WORKSPACE_ID}/subjektiv/sessions"); - let session_id = session_store::new_session_id().to_string(); - let body = serde_json::to_vec(&json!({ "session_id": session_id, "create_if_missing": true })) - .unwrap(); - let response = tokio::time::timeout( - std::time::Duration::from_secs(2), - app.clone().oneshot(runtime_source_request( - &identity, - Some(&worker_id), - "POST", - &path, - body.clone(), - )), - ) - .await - .expect("Session attribution must not wait for its own Worker creation") - .unwrap(); - let status = response.status(); - let response_body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); - assert_eq!(status, StatusCode::OK, "{response_body:?}"); - let response: Value = serde_json::from_slice(&response_body).unwrap(); - assert_eq!(response["subject_id"], subject.id); - assert_eq!(response["session_id"], session_id); - let stored = subjektiv.session_attribution(&session_id).unwrap().unwrap(); - assert_eq!(stored.worker_id, worker_id); - assert_eq!(stored.subject_id, subject.id); - - // The callback lane does not bypass authentication. - let response = tokio::time::timeout( - std::time::Duration::from_secs(2), - app.clone().oneshot( - Request::builder() - .method("POST") - .uri(&path) - .header(CONTENT_TYPE, "application/json") - .body(Body::from(body)) + let memory_path = format!("/api/w/{TEST_WORKSPACE_ID}/subjektiv/memory"); + for (_, worker_id, _) in &callbacks { + // Resident context is also a pure Subject callback used by Runtime + // bootstrap/restore. It must not reenter the waiting Host lane either. + let response = tokio::time::timeout( + std::time::Duration::from_secs(2), + app.clone().oneshot(runtime_source_request( + &identity, + Some(worker_id), + "POST", + &memory_path, + serde_json::to_vec(&server_api::SubjektivMemoryBackendRequest { + operation: server_api::SubjektivMemoryBackendOperation::ResidentContext( + Default::default(), + ), + }) .unwrap(), - ), - ) - .await - .unwrap() - .unwrap(); - assert_eq!(response.status(), StatusCode::UNAUTHORIZED); + )), + ) + .await + .expect("resident context must not wait for its Host parents") + .unwrap(); + let status = response.status(); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!(status, StatusCode::OK, "{body:?}"); + let resident: server_api::SubjektivMemoryBackendResponse = + serde_json::from_slice(&body).unwrap(); + assert!(matches!( + resident, + server_api::SubjektivMemoryBackendResponse::ResidentContext(_) + )); + } + // Resident memory is not an allowed deletion-drain callback. Preserve that + // admission fence; only the exact Session attribution route reenters below. + let mut deletion = Box::pin(gate.lock_deletion()); assert!(futures::poll!(&mut deletion).is_pending()); - drop(create); + for (subject_id, worker_id, session_id) in &callbacks { + let body = + serde_json::to_vec(&json!({ "session_id": session_id, "create_if_missing": true })) + .unwrap(); + let response = tokio::time::timeout( + std::time::Duration::from_secs(2), + app.clone().oneshot(runtime_source_request( + &identity, + Some(worker_id), + "POST", + &path, + body, + )), + ) + .await + .expect("Session attribution must not wait for its Host parents") + .unwrap(); + let status = response.status(); + let response_body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!(status, StatusCode::OK, "{response_body:?}"); + let response: Value = serde_json::from_slice(&response_body).unwrap(); + assert_eq!(response["subject_id"], *subject_id); + assert_eq!(response["session_id"], *session_id); + let stored = subjektiv.session_attribution(session_id).unwrap().unwrap(); + assert_eq!(stored.worker_id, *worker_id); + assert_eq!(stored.subject_id, *subject_id); + } + // Neither actual parent nor its admission lease was released to make these + // callbacks succeed. Only durable attribution completion releases them. + assert!(parents.iter().all(|parent| !parent.is_finished())); + assert!(futures::poll!(&mut deletion).is_pending()); + for finish_callback in finish_callbacks { + finish_callback.send(()).unwrap(); + } + for parent in parents { + parent.await.unwrap().unwrap(); + } + host.drain().await.unwrap(); + api.store.db_executor().drain().await.unwrap(); let _deletion = tokio::time::timeout(std::time::Duration::from_secs(2), deletion) .await .unwrap(); assert!(gate.admit(true).is_none()); } +#[tokio::test] +async fn consolidation_subject_callbacks_are_denied_without_runtime_lookup() { + let workspace = tempfile::tempdir().unwrap(); + let mut api = test_api(workspace.path()).await; + let identity = RuntimeIdentityMaterial::generate("runtime-test").unwrap(); + configure_runtime_request_auth(&mut api, &identity, "runtime-test"); + let subjektiv = open_subjektiv_store(&api).unwrap(); + let subject = subjektiv + .create_subject(crate::subjektiv::SubjectRole::new("companion").unwrap()) + .unwrap(); + let key = format!("{SUBJEKTIV_CONSOLIDATION_SINGLETON_PREFIX}{}", subject.id); + let memory = api + .config_store + .get_workspace_memory_settings(TEST_WORKSPACE_ID) + .unwrap(); + let reservation = api + .config_store + .reserve_worker_create( + TEST_WORKSPACE_ID, + "runtime-test", + "consolidation-start", + &"f".repeat(64), + Some(&key), + &memory, + ) + .unwrap(); + // runtime-test has no Worker adapter: authority denial must not try to look + // up its profile/tags through Runtime before rejecting this disallowed role. + let server = WorkspaceServerApi::new_for_test(api.config.clone(), api.store.clone()); + server + .apis + .lock() + .await + .insert(TEST_WORKSPACE_ID.into(), api.clone()); + server + .routers + .lock() + .await + .insert(TEST_WORKSPACE_ID.into(), build_inner_router(api.clone())); + let app = workspace_server_router(server); + let path = format!("/api/w/{TEST_WORKSPACE_ID}/subjektiv/sessions"); + let worker_id = reservation.worker_id.to_string(); + let session_id = session_store::new_session_id().to_string(); + for (callback_path, callback_body) in [ + ( + path.clone(), + serde_json::to_vec(&json!({"session_id": session_id, "create_if_missing": true})) + .unwrap(), + ), + ( + format!("/api/w/{TEST_WORKSPACE_ID}/subjektiv/memory"), + serde_json::to_vec(&server_api::SubjektivMemoryBackendRequest { + operation: server_api::SubjektivMemoryBackendOperation::ResidentContext( + Default::default(), + ), + }) + .unwrap(), + ), + ] { + let response = app + .clone() + .oneshot(runtime_source_request( + &identity, + Some(&worker_id), + "POST", + &callback_path, + callback_body, + )) + .await + .unwrap(); + let status = response.status(); + let body = to_bytes(response.into_body(), usize::MAX).await.unwrap(); + assert_eq!(status, StatusCode::FORBIDDEN, "{callback_path}: {body:?}"); + } + assert!( + subjektiv + .session_attribution(&session_id) + .unwrap() + .is_none() + ); + let response = app + .oneshot( + Request::builder() + .method("POST") + .uri(path) + .header(CONTENT_TYPE, "application/json") + .body(Body::from( + json!({"session_id": session_id, "create_if_missing": true}).to_string(), + )) + .unwrap(), + ) + .await + .unwrap(); + assert_eq!(response.status(), StatusCode::UNAUTHORIZED); +} + #[tokio::test] async fn workspace_deletion_waits_for_session_attribution_and_fences_both_lanes() { let gate = Arc::new(WorkspaceAdmission::default()); From 27b6c688e48e4061340d475b227d76b019cc462a Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 09:47:06 +0900 Subject: [PATCH 05/14] docs: explain independent mandatory callback ownership and causal regressions --- docs/development/backend-db-execution.md | 22 +++++-- .../report/2026-10-12-t748-suite-diagnosis.md | 64 ++++++++++++++++++- 2 files changed, 78 insertions(+), 8 deletions(-) diff --git a/docs/development/backend-db-execution.md b/docs/development/backend-db-execution.md index 7fd4a58d6..45c85f39d 100644 --- a/docs/development/backend-db-execution.md +++ b/docs/development/backend-db-execution.md @@ -58,11 +58,23 @@ owned workflow needs async resource locks, `run_host_workflow` polls it on a Hos worker using the original Tokio handle; do not build a replacement runtime for cached services and do not submit to and await the Host lane from inside itself. Control/session guards needed by an accepted mutation belong to that actual job. -Subjektiv session Read/Search, subject response projection, memory staging and -legacy consolidation-scope helpers also use Host ownership: they can synchronously -call Runtime/network services outside their SQL transactions. Pure DB-resident -Subjektiv scope and memory projection helpers stay on the DB lane. Host admission -must not be acquired again from an already-owned Host workflow. +Subjektiv session Read/Search, subject response projection, general memory staging +and legacy consolidation-scope helpers also use Host ownership: they can +synchronously call Runtime/network services outside their SQL transactions. +Pure DB-resident Subjektiv scope and memory projection helpers stay on the DB lane. +Host admission must not be acquired again from an already-owned Host workflow, +including indirectly through a Runtime's mandatory Backend callback. + +Mandatory Session attribution has Subject-only authority and whole attribution +SQL on the independent DB lane. Store-only scope discovery distinguishes Subject, +current Job consolidation, and legacy consolidation authority; attribution rejects +both consolidation roles **before** any Runtime profile/tag lookup. Legacy +consolidation checks for mixed callers remain on Host. Subject-only memory +operations (resident context/summary, proposal validation, receipt lookup, explicit +staging) use the same store-only authority boundary and DB ownership. Runtime +bootstrap/restore can request resident context while its Host parent is waiting. +This is not permission to bypass Workspace deletion admission: only the already +allowed exact callback endpoints may reenter deletion drain. Backend Job cleanup has a third finite lane (two workers, 16 queued jobs), so a long Run teardown wait cannot starve ordinary Host control/stop requests. It is diff --git a/docs/report/2026-10-12-t748-suite-diagnosis.md b/docs/report/2026-10-12-t748-suite-diagnosis.md index ff78dc17f..b8f3bdd22 100644 --- a/docs/report/2026-10-12-t748-suite-diagnosis.md +++ b/docs/report/2026-10-12-t748-suite-diagnosis.md @@ -198,9 +198,67 @@ Workdir default (153) and `http-client` (156) full suites, format/diff checks an state-contract naming (558 matching lines, zero unapproved, zero stale) passed. All used the isolated `target/t748-candidate-build` artifact directory. Logs: `target/t748-review-fix-{root-check,full-suite,workdir-default,workdir-http,format,names}.log`. -The following documentation-only commit does not change that tested code. -Fresh independent review of the normally advanced same MR source is still -required; previous `request_changes` evidence is not approval of this fix. +The documentation-only publication `b10b7315` did not change that tested code. +MR seq5 independently confirmed the two old fixes and repeated the checks above, +but requested changes for a new mandatory-callback cycle; see the next section. + +## Mandatory callback must be independent of its waiting Host parent + +MR seq5 finding `T748-HOST-CALLBACK-CYCLE` traced Host create/restore -> Runtime +bootstrap -> mandatory Session attribution -> the same finite Host lane. Two +waiting parents could consume both workers while successfully admitted callbacks +could not run. Cancelling a waiter correctly retains the actual parent, so it is +not a solution. The Reviewer's compiled executor artifact proved this placement +cycle, not a complete create endpoint/E2E reproduction. + +Fix `a2487b57` separates store-only scope discovery into typed Subject, current +Job consolidation and legacy consolidation authority. Session attribution accepts +only Subject authority and executes its whole existing SQL operation on the +independent DB lane. It rejects consolidation roles without calling Runtime. +Mixed read/search, subject projections and allowed legacy consolidation operations +keep their Runtime checks on Host; the previous DB/network fix is not undone. + +The same dependency audit identified Subject resident context reads used by +Runtime bootstrap/restore. All Subject-only memory operations use store-only +Subject authority and DB ownership too (resident summary/context, proposal +validation, receipt lookup, explicit staging). Other memory operations retain +Host ownership because legacy authority can require synchronous Runtime lookup. +No transaction is split across an await and no production budget is increased. + +The strengthened signed route regression holds both actual production-sized Host +workers (2/16) in `run_host_workflow` parents and retains their create admission +leases until callbacks finish. Resident and attribution routes authenticate real +Runtime source proofs, check current singleton ownership, respond successfully, +and persist both distinct Subject session attributions **before** either parent +releases capacity. It also preserves Session callback reentry during deletion +drain. Runtime's wait is modeled at this policy boundary; no product process or +complete create endpoint is spawned. Separate denial coverage proves legacy +consolidation callbacks return 403 without Runtime lookup, leave no attribution, +and do not bypass authentication (401). + +Only Session attribution is an allowed deletion-drain callback; that endpoint +allowlist was not expanded. An initial fixture sent resident reads during deletion +and correctly received 409. The fixture now tests those reads before deletion, +then Session attribution while deletion waits. This is alignment with the existing +fence, not a relaxed product gate or success-picking retry. A diagnostic Python +invocation was unavailable before making any source change; only the subsequent +Deno lane-restoration runs are causal red evidence. + +Restoring just Session's old Host placement fails at its expected callback +watchdog; restoring just resident memory's old Host placement fails at its own +watchdog. Fixed placement succeeds without parent bailout. Evidence: +`target/t748-callback-old-{session,resident}-placement-red.log`, +`target/t748-callback-restored-green.log`. Final focused signed callback tests (5), +DB boundaries (10), and Subjektiv regressions (25) passed. + +Exact code `a2487b575df1f348251214c53da61595860fcb40` passed root `cargo check` and +original default-parallel Server **963 passed / 0 failed / 1 existing ignored**, +11 binary tests; library duration **173.01 seconds**. Only the three intentional +executor panics appeared, with no residual test processes. Workdir default153 / +http-client156, fmt/diff and naming558/0/0 passed. The isolated candidate cache +remains in use. Logs: `target/t748-callback-{root-check,full-suite,workdir-default,workdir-http}.log`. +The following docs-only publication changes no tested code. Its exact normally +advanced MR source still needs a fresh independent verdict; seq5 is not approval. No latency benchmark, production incident measurement, target integration, Ticket conclusion, or running-environment update is claimed. T743 has now From dbcea6c34ba55453ce588d68272df238cde19f47 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 10:25:59 +0900 Subject: [PATCH 06/14] fix: await bounded Worker discovery and Runtime resource summary owners --- crates/workspace-server/src/server.rs | 13 +- .../src/server_db_execution_tests.rs | 148 ++++++++++++++++++ 2 files changed, 159 insertions(+), 2 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index e20636a53..2e7fb889a 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -23710,7 +23710,11 @@ async fn discover_workspace_workers( "invalid_workspace_worker_discovery_cursor".to_string(), )) })?; - let workers = workers_response(api)?; + let workers = api + .store + .db_executor() + .run(move || workers_response(api)) + .await?; Ok(workspace_worker_discovery_page( workers.items, query, @@ -32159,7 +32163,12 @@ async fn workspace_runtime_resources_response( workspace_id: &str, ) -> ApiResult { let limit = api.config.max_records.min(200); - let runtimes = api.runtime.list_runtimes(limit); + let runtime = api.runtime.clone(); + let runtimes = api + .host_executors + .host()? + .run(move || Ok::<_, ApiError>(runtime.list_runtimes(limit))) + .await?; let bindings = api .store .list_workspace_runtime_bindings(workspace_id, true) diff --git a/crates/workspace-server/src/server_db_execution_tests.rs b/crates/workspace-server/src/server_db_execution_tests.rs index 53d94da0f..cf6c84389 100644 --- a/crates/workspace-server/src/server_db_execution_tests.rs +++ b/crates/workspace-server/src/server_db_execution_tests.rs @@ -919,6 +919,7 @@ struct HeldRuntimeReply { address: std::net::SocketAddr, entered: Option>, release: Option>, + upstream_released: Arc, thread: Option>, } impl HeldRuntimeReply { @@ -928,6 +929,8 @@ impl HeldRuntimeReply { let address = listener.local_addr().unwrap(); let (entered, received) = oneshot::channel(); let (release, permit) = mpsc::channel(); + let upstream_released = Arc::new(AtomicBool::new(false)); + let flag = upstream_released.clone(); let thread = std::thread::spawn(move || { let Ok((mut stream, _)) = listener.accept() else { return; @@ -946,12 +949,14 @@ impl HeldRuntimeReply { .to_owned(), ); let _ = permit.recv_timeout(WATCHDOG); + flag.store(true, Ordering::SeqCst); let _ = stream.write_all(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); }); Self { address, entered: Some(received), release: Some(release), + upstream_released, thread: Some(thread), } } @@ -1182,3 +1187,146 @@ async fn subjektiv_runtime_waits_leave_db_and_control_free_and_cancel_retains_th fixture.executor.drain().await.unwrap(); } } + +#[tokio::test(flavor = "current_thread")] +async fn runtime_resource_summary_waits_off_tokio_and_db_and_retains_cancelled_host_ownership() { + let fixture = Fixture::new().await; + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "summary-runtime", + &format!("http://{}", reply.address), + true, + ); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let api = fixture.api.clone(); + let task = tokio::spawn(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + async move { workspace_runtime_resources_response(&api, WORKSPACE).await }, + )); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!(request.contains("/v1/runtime"), "{request}"); + assert!( + !reply.upstream_released.load(Ordering::SeqCst), + "upstream watchdog released before Tokio could observe the request" + ); + let read_store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || read_store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; // timer contract only + assert!(!reply.upstream_released.load(Ordering::SeqCst)); + assert!(!task.is_finished()); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "cancel released actual Runtime summary owner" + ); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + fixture.executor.drain().await.unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn worker_discovery_sql_waits_off_tokio_and_keeps_admission_until_actual_cleanup() { + // Exercise the exact post-auth handler boundary: signed source verification + // has separate route coverage. Auth itself also awaits DB and would otherwise + // hide a handler's direct mutex acquisition until after the writer is released. + for cancel_waiter in [false, true] { + let fixture = Fixture::new().await; + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let writer = fixture.store.clone(); + fixture + .executor + .dispatch(move || { + writer + .mark_api_token_used("missing-token", "2026-01-01T00:00:00Z") + .unwrap(); + }) + .unwrap(); + observed(waiting).await.unwrap(); // Actual SQL writer holds shared connection mutex. + let query = || server_api::WorkspaceWorkerDiscoveryQuery { + limit: Some(1), + cursor: None, + query: Some("missing-peer".into()), + }; + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let lifetime = Arc::new(CountJobLifetime(leases.clone())); + let mut discovery = Box::pin(crate::db_executor::with_operation_lifetime( + lifetime, + discover_workspace_workers(fixture.api.clone(), WORKSPACE.into(), query(), true), + )); + assert!( + futures::poll!(&mut discovery).is_pending(), + "discovery must queue, not synchronously wait for the SQL mutex" + ); + assert_eq!(leases.load(Ordering::SeqCst), 1); + runtime_progress_while_locked(&lock).await; + assert_eq!( + observed(fixture.host_executor.run(|| Ok::<_, Error>("Host control"))) + .await + .unwrap(), + "Host control" + ); + assert!(futures::poll!(&mut discovery).is_pending()); + let rejected = + discover_workspace_workers(fixture.api.clone(), WORKSPACE.into(), query(), true) + .await + .unwrap_err() + .into_response(); + let body = response_json(rejected, StatusCode::SERVICE_UNAVAILABLE).await; + assert!(body.to_string().contains("db_capacity_exceeded"), "{body}"); + let denied = + discover_workspace_workers(fixture.api.clone(), WORKSPACE.into(), query(), false) + .await + .unwrap_err() + .into_response(); + response_json(denied, StatusCode::FORBIDDEN).await; + // Failure cleanup never makes the progress assertions above true. + assert!(!lock.released.load(Ordering::SeqCst)); + if cancel_waiter { + drop(discovery); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "cancel released accepted discovery authority" + ); + let rejected = + discover_workspace_workers(fixture.api.clone(), WORKSPACE.into(), query(), true) + .await + .unwrap_err() + .into_response(); + response_json(rejected, StatusCode::SERVICE_UNAVAILABLE).await; + lock.release(); + busy.resume(); + } else { + lock.release(); + busy.resume(); + let page = observed(discovery).await.unwrap(); + assert!( + page.workers.is_empty(), + "empty filtered projection must remain empty" + ); + assert!(page.next_cursor.is_none()); + } + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!( + leases.load(Ordering::SeqCst), + 0, + "actual discovery cleanup leaked its lease" + ); + fixture.host_executor.drain().await.unwrap(); + } +} From b63bdb4402b68d97d4d5cc190b435ef5105fc92d Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 10:31:48 +0900 Subject: [PATCH 07/14] docs: record discovery and Runtime metadata boundary verification --- docs/development/backend-db-execution.md | 9 +++ .../report/2026-10-12-t748-suite-diagnosis.md | 58 ++++++++++++++++++- 2 files changed, 65 insertions(+), 2 deletions(-) diff --git a/docs/development/backend-db-execution.md b/docs/development/backend-db-execution.md index 45c85f39d..4bb0ac804 100644 --- a/docs/development/backend-db-execution.md +++ b/docs/development/backend-db-execution.md @@ -112,6 +112,15 @@ whole transaction helpers. Additional assignment diagnostics, answer mutations, and Ticket queries use the same owned caller boundary; do not add a second ledger or reconstruct processing evidence from Worker liveness. +Worker discovery uses the same whole `workers_response` DB stage as Worker list +and resource detail: projection snapshot, Workdir registry/links, and authority +projection never acquire the shared connection on Tokio. Source-proof requirement +and pure query/cursor validation still precede admission; overload remains 503. +Runtime resource list/detail use a finite Host stage for synchronous Runtime +summary I/O, then await the independent DB binding read. Do not confuse a Runtime +summary callback with the pure stored Worker projection or run network waits on +the sole DB worker. + ## Regression evidence - `db_executor::tests`: real SQLite IMMEDIATE write contention while a current-thread diff --git a/docs/report/2026-10-12-t748-suite-diagnosis.md b/docs/report/2026-10-12-t748-suite-diagnosis.md index b8f3bdd22..9f7730463 100644 --- a/docs/report/2026-10-12-t748-suite-diagnosis.md +++ b/docs/report/2026-10-12-t748-suite-diagnosis.md @@ -257,8 +257,62 @@ original default-parallel Server **963 passed / 0 failed / 1 existing ignored**, executor panics appeared, with no residual test processes. Workdir default153 / http-client156, fmt/diff and naming558/0/0 passed. The isolated candidate cache remains in use. Logs: `target/t748-callback-{root-check,full-suite,workdir-default,workdir-http}.log`. -The following docs-only publication changes no tested code. Its exact normally -advanced MR source still needs a fresh independent verdict; seq5 is not approval. +The docs-only publication `27b6c688` changed no tested code. MR seq7 independently +confirmed the callback fix and earlier fixes, but identified another direct async +caller; its independent verdict was `request_changes`, not approval. + +## Worker discovery and adjacent metadata caller audit + +MR seq7 major `T748-ASYNC-WORKER-DISCOVERY` traced async discovery directly into +synchronous `workers_response`. That helper acquires the shared SQL connection +for the Worker projection snapshot, Workdir registry/links and authority projection. +The Reviewer's independently compiled public snapshot helper reproduced a +300.246ms shared-mutex stall while its Tokio timer could not progress. This was +helper-boundary evidence, not an authenticated endpoint/E2E run. + +Fix `dbcea6c3` awaits the **whole existing** `workers_response` on the independent +bounded DB owner. Source-proof requirement, Workspace/query/cursor validation, +visibility filtering, pagination and control-grant behavior are unchanged. The +other production callers were traced: Worker list/resource detail already use DB; +mixed Runtime cleanup plans use Host. No network work is added to the DB owner. + +The adjacent metadata audit found async `workspace_runtime_resources_response` +also directly calling synchronous Runtime summaries (including remote signed HTTP +and its awaited DB authorization). This call now uses finite Host ownership, +then the existing independent async binding read. Runtime list/detail share this +boundary. Whole SQL helpers, admission budgets and authentication fallback rules +are not changed. + +The permanent post-auth discovery handler regression uses a separate SQLite +IMMEDIATE writer and busy observer to hold the actual shared connection mutex. +While the accepted discovery read is queued, current-thread timer and independent +Host control progress. A second read receives 503/`db_capacity_exceeded`; a source +without proof receives 403 before admission. Cancellation retains the actual +queued read's lease/budget until result cleanup; ordinary completion preserves the +empty filtered catalog response. Signed route proof verification remains covered +by the existing dedicated-permission test, separately from this post-auth policy +boundary so middleware's own DB wait cannot hide the handler bug. + +Restoring the exact old discovery call fails its first-poll assertion after the +fixture watchdog releases the synchronous mutex wait. Restoring only the old +Runtime-summary placement fails because Tokio cannot observe the real loopback +request until the upstream watchdog releases it. Fixed summary placement lets +SQL/timer/control progress during the held signed upstream request and keeps the +actual Host lease after cancellation. Explicit upstream-release observation +prevents watchdog failure cleanup from masquerading as progress. Logs: +`target/t748-discovery-old-placement-red.log`, +`target/t748-runtime-summary-old-placement-red.log`, +`target/t748-discovery-summary-fixed-boundaries.log` (12 passed), +`target/t748-discovery-summary-policy.log` (4), and callbacks (5). + +Exact code `dbcea6c34ba55453ce588d68272df238cde19f47` passed root `cargo check` and +original default-parallel Server **965 passed / 0 failed / 1 existing ignored**, +11 binary tests; library duration **175.00 seconds**. Only three deliberate +executor panics appeared, with no residual test processes. Workdir default153 / +http-client156 and fmt/diff/naming558/0/0 passed. Logs use the same isolated +candidate cache: `target/t748-discovery-summary-{root-check,full-suite,workdir-default,workdir-http}.log`. +The following docs-only publication changes no tested code; fresh exact-source +independent review is required before an approved handoff. No latency benchmark, production incident measurement, target integration, Ticket conclusion, or running-environment update is claimed. T743 has now From 925bce2a405b6d3255887fba88867ab8d23882dc Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 11:24:12 +0900 Subject: [PATCH 08/14] fix: own Runtime management authority stages and commit publication workflows --- crates/workspace-server/src/server.rs | 259 +++++++++------ .../src/server_db_execution_tests.rs | 299 ++++++++++++++++-- 2 files changed, 451 insertions(+), 107 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 2e7fb889a..becd6fd80 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -27608,30 +27608,44 @@ async fn scoped_update_remote_runtime( .as_deref() .map(str::trim) .filter(|value| !value.is_empty()) - .unwrap_or(&path.runtime_id); - let binding = api - .store - .update_workspace_runtime_binding_metadata( - &path.workspace_id, - &path.runtime_id, - display_name, - endpoint.as_str().trim_end_matches('/'), - &Utc::now().to_rfc3339(), - ) - .await?; - api.runtime_binding_expectations - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .insert( - (path.workspace_id.clone(), path.runtime_id.clone()), - binding.clone(), - ); - api.runtime_subscription_broker - .unregister_runtime(&path.runtime_id); - api.register_workspace_runtime_binding(binding)?; - Ok(Json( - workspace_runtime_detail(&api, &path.workspace_id, &path.runtime_id).await?, - )) + .unwrap_or(&path.runtime_id) + .to_owned(); + update_remote_runtime_metadata_owned( + api, + path.workspace_id, + path.runtime_id, + display_name, + endpoint.as_str().trim_end_matches('/').to_owned(), + ) + .await +} + +// Post-auth mutation boundary: cancellation cannot split SQL completion from +// publication or discard the accepted workflow's request/resource authority. +async fn update_remote_runtime_metadata_owned( + api: WorkspaceApi, + workspace_id: String, + runtime_id: String, + display_name: String, + endpoint: String, +) -> ApiResult> { + run_host_workflow(api.host_executors.host()?, async move { + let binding = api + .store + .update_workspace_runtime_binding_metadata( + &workspace_id, + &runtime_id, + &display_name, + &endpoint, + &Utc::now().to_rfc3339(), + ) + .await?; + publish_workspace_runtime_binding(&api, binding)?; + Ok(Json( + workspace_runtime_detail_on_host(&api, &workspace_id, &runtime_id).await?, + )) + }) + .await } async fn scoped_reveal_runtime_trust_key( @@ -29116,6 +29130,75 @@ async fn list_workers( .await } +async fn require_active_runtime_registration_identity(api: &WorkspaceApi) -> ApiResult<()> { + let identities = api.signing_identities.clone(); + let workspace_id = api.config.workspace_id.clone(); + api.store + .db_executor() + .run(move || { + let identity = identities.get_validated(&workspace_id)?; + if identity.state != "active" { + return Err(settings_bad_request( + "workspace_signing_identity_unavailable", + "an active Workspace signing identity is required before registering a Runtime", + )); + } + Ok(()) + }) + .await +} + +async fn scan_runtime_registration_workers(api: &WorkspaceApi, runtime_id: &str) -> ApiResult<()> { + let runtime = api.runtime.clone(); + let runtime_id = runtime_id.to_owned(); + let limit = api.config.max_records.min(200); + api.host_executors.host()?.run(move || { + match runtime.list_workers_for_runtime(&runtime_id, limit) { + Ok(workers) if !workers.items.is_empty() => Err(Error::RuntimeBindingConflict(format!( + "Runtime `{runtime_id}` still has {} active worker(s) and cannot replace its connection", + workers.items.len() + )).into()), + Ok(_) | Err(RuntimeRegistryError::UnknownRuntime(_)) => Ok(()), + Err(error) => Err(error.into_error().into()), + } + }).await +} + +fn publish_workspace_runtime_binding( + api: &WorkspaceApi, + binding: WorkspaceRuntimeBinding, +) -> Result<()> { + api.runtime_binding_expectations + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .insert( + (binding.workspace_id.clone(), binding.runtime_id.clone()), + binding.clone(), + ); + api.runtime_subscription_broker + .unregister_runtime(&binding.runtime_id); + api.register_workspace_runtime_binding(binding) +} + +async fn require_current_runtime_test_binding( + api: &WorkspaceApi, + binding: WorkspaceRuntimeBinding, +) -> ApiResult { + let store = api.store.clone(); + api.store + .db_executor() + .run(move || { + if !store.workspace_runtime_binding_matches(&binding)? { + return Err(Error::RuntimeBindingConflict( + "Runtime connection changed while the connection test was running".to_string(), + ) + .into()); + } + Ok(binding) + }) + .await +} + async fn create_remote_runtime( State(api): State, Extension(actor): Extension, @@ -29136,33 +29219,13 @@ async fn create_remote_runtime( "the embedded Runtime is built in and cannot be managed as a remote Runtime", )); } - let identity = api - .signing_identities - .get_validated(&api.config.workspace_id)?; - if identity.state != "active" { - return Err(settings_bad_request( - "workspace_signing_identity_unavailable", - "an active Workspace signing identity is required before registering a Runtime", - )); - } + require_active_runtime_registration_identity(&api).await?; // Protect observed active Workers without removing the current connection. // This scan is not proof of idleness: an unavailable/auth-failing endpoint // must remain repairable by an owner using binding-ID CAS below. Replacement // neither stops nor deletes Workers; Runtime removal has separate preflight. // Rejected key/CAS mutations must leave the current registration usable. - match api - .runtime - .list_workers_for_runtime(&runtime_id, api.config.max_records.min(200)) - { - Ok(workers) if !workers.items.is_empty() => { - return Err(Error::RuntimeBindingConflict(format!( - "Runtime `{runtime_id}` still has {} active worker(s) and cannot replace its connection", - workers.items.len() - )).into()); - } - Ok(_) | Err(RuntimeRegistryError::UnknownRuntime(_)) => {} - Err(error) => return Err(error.into_error().into()), - } + scan_runtime_registration_workers(&api, &runtime_id).await?; let now = Utc::now().to_rfc3339(); let record = WorkspaceRuntimeBinding { workspace_id: api.config.workspace_id.clone(), @@ -29183,30 +29246,32 @@ async fn create_remote_runtime( updated_at: now, revoked_at: None, }; - let (mutation, binding) = api - .store - .put_workspace_runtime_binding_key( - record, - request.expected_binding_id.as_deref(), - &actor.account_id, + let mutation_api = api.clone(); + // One accepted owner keeps SQL commit, local registration and its response + // observation together even when the HTTP waiter disappears. Resource reads + // call the on-Host helper directly, never re-admit to the occupied Host lane. + let (mutation, resource) = run_host_workflow(api.host_executors.host()?, async move { + let (mutation, binding) = mutation_api + .store + .put_workspace_runtime_binding_key( + record, + request.expected_binding_id.as_deref(), + &actor.account_id, + ) + .await?; + publish_workspace_runtime_binding(&mutation_api, binding)?; + let resource = workspace_runtime_resources_response_on_host( + &mutation_api, + mutation_api.workspace_id(), ) - .await?; - api.runtime_binding_expectations - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .insert( - (api.config.workspace_id.clone(), runtime_id.clone()), - binding.clone(), - ); - api.runtime_subscription_broker - .unregister_runtime(&runtime_id); - api.register_workspace_runtime_binding(binding)?; - let resource = workspace_runtime_resources_response(&api, &api.config.workspace_id) .await? .items .into_iter() .find(|resource| resource.runtime.runtime_id == runtime_id) .ok_or_else(|| Error::UnknownRuntime(runtime_id.clone()))?; + Ok::<_, ApiError>((mutation, resource)) + }) + .await?; let status = if mutation == WorkspaceRuntimeBindingMutation::Created { StatusCode::CREATED } else { @@ -29563,22 +29628,15 @@ async fn test_runtime_connection( let checked_at = Utc::now().to_rfc3339(); let runtime = api.runtime.clone(); let ping_runtime_id = runtime_id.clone(); - let ping = tokio::task::spawn_blocking(move || runtime.ping(&ping_runtime_id)) - .await - .map_err(|_| Error::RuntimeOperationFailed { - runtime_id: runtime_id.clone(), - code: "runtime_connection_test_unavailable".to_string(), - message: "Runtime connection test could not be completed".to_string(), - })?; + let ping = api + .host_executors + .host()? + .run(move || Ok::<_, ApiError>(runtime.ping(&ping_runtime_id))) + .await?; - // The ping runs outside the store lock. A replaced/revoked connection must - // not be reported as an observation of the earlier binding. - if !api.store.workspace_runtime_binding_matches(&binding)? { - return Err(Error::RuntimeBindingConflict( - "Runtime connection changed while the connection test was running".to_string(), - ) - .into()); - } + // The ping runs outside the store lock. Recheck the entire live binding on + // the independent DB owner, never acquire its mutex on the Tokio waiter. + let binding = require_current_runtime_test_binding(&api, binding).await?; let mut result = runtime_connection_test_response(api.workspace_id(), &runtime_id, checked_at, ping); // This result describes this attempt, not permission for a future request. @@ -29590,8 +29648,10 @@ async fn test_runtime_connection( async fn get_worker_launch_options( State(api): State, ) -> ApiResult> { - api.store - .db_executor() + // Runtime summaries can call back into DB signing/authority. Profile config + // reads are synchronous too, but this mixed helper must not occupy sole DB. + api.host_executors + .host()? .run(move || Ok(Json(worker_launch_options_response(&api)?))) .await } @@ -32161,14 +32221,23 @@ fn workers_response(api: WorkspaceApi) -> ApiResult ApiResult { + let api = api.clone(); + let workspace_id = workspace_id.to_owned(); + run_host_workflow(api.host_executors.host()?, async move { + workspace_runtime_resources_response_on_host(&api, &workspace_id).await + }) + .await +} + +// Already-owned Runtime workflows call this directly; re-admission to Host would +// wait behind the same parents. DB binding helpers still use their own DB owner. +async fn workspace_runtime_resources_response_on_host( + api: &WorkspaceApi, + workspace_id: &str, ) -> ApiResult { let limit = api.config.max_records.min(200); - let runtime = api.runtime.clone(); - let runtimes = api - .host_executors - .host()? - .run(move || Ok::<_, ApiError>(runtime.list_runtimes(limit))) - .await?; + let runtimes = api.runtime.list_runtimes(limit); let bindings = api .store .list_workspace_runtime_bindings(workspace_id, true) @@ -32267,12 +32336,26 @@ async fn workspace_runtime_detail( api: &WorkspaceApi, workspace_id: &str, runtime_id: &str, +) -> ApiResult { + let api = api.clone(); + let workspace_id = workspace_id.to_owned(); + let runtime_id = runtime_id.to_owned(); + run_host_workflow(api.host_executors.host()?, async move { + workspace_runtime_detail_on_host(&api, &workspace_id, &runtime_id).await + }) + .await +} + +async fn workspace_runtime_detail_on_host( + api: &WorkspaceApi, + workspace_id: &str, + runtime_id: &str, ) -> ApiResult { let binding = api .store .get_workspace_runtime_binding(workspace_id, runtime_id) .await?; - let mut resource = workspace_runtime_resources_response(api, workspace_id) + let mut resource = workspace_runtime_resources_response_on_host(api, workspace_id) .await? .items .into_iter() diff --git a/crates/workspace-server/src/server_db_execution_tests.rs b/crates/workspace-server/src/server_db_execution_tests.rs index cf6c84389..4d287776c 100644 --- a/crates/workspace-server/src/server_db_execution_tests.rs +++ b/crates/workspace-server/src/server_db_execution_tests.rs @@ -1189,12 +1189,12 @@ async fn subjektiv_runtime_waits_leave_db_and_control_free_and_cancel_retains_th } #[tokio::test(flavor = "current_thread")] -async fn runtime_resource_summary_waits_off_tokio_and_db_and_retains_cancelled_host_ownership() { +async fn runtime_registration_scan_keeps_sql_and_tokio_free_and_retains_cancelled_host_budget() { let fixture = Fixture::new().await; let mut reply = HeldRuntimeReply::new(); install_remote_fixture( &fixture, - "summary-runtime", + "scan-runtime", &format!("http://{}", reply.address), true, ); @@ -1202,42 +1202,303 @@ async fn runtime_resource_summary_waits_off_tokio_and_db_and_retains_cancelled_h let api = fixture.api.clone(); let task = tokio::spawn(crate::db_executor::with_operation_lifetime( Arc::new(CountJobLifetime(leases.clone())), - async move { workspace_runtime_resources_response(&api, WORKSPACE).await }, + async move { scan_runtime_registration_workers(&api, "scan-runtime").await }, )); let request = observed(reply.entered.take().unwrap()).await.unwrap(); - assert!(request.contains("/v1/runtime"), "{request}"); + assert!(request.contains("/v1/workers"), "{request}"); assert!( !reply.upstream_released.load(Ordering::SeqCst), - "upstream watchdog released before Tokio could observe the request" + "registration scan blocked Tokio until watchdog bailout" ); - let read_store = fixture.store.clone(); + let store = fixture.store.clone(); observed( fixture .executor - .run(move || read_store.get_workspace_memory_settings(WORKSPACE)), + .run(move || store.get_workspace_memory_settings(WORKSPACE)), ) .await .unwrap(); - assert_eq!( - observed(tokio::spawn(async { "control" })).await.unwrap(), - "control" - ); - observed(tokio::time::sleep(StdDuration::from_millis(1))).await; // timer contract only - assert!(!reply.upstream_released.load(Ordering::SeqCst)); - assert!(!task.is_finished()); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; + let mut queued_control = Box::pin(fixture.host_executor.run(|| Ok::<_, Error>(()))); + assert!(futures::poll!(&mut queued_control).is_pending()); + let rejected = scan_runtime_registration_workers(&fixture.api, "scan-runtime") + .await + .unwrap_err(); + response_json(rejected.into_response(), StatusCode::SERVICE_UNAVAILABLE).await; task.abort(); assert!(task.await.unwrap_err().is_cancelled()); - assert_eq!( - leases.load(Ordering::SeqCst), - 1, - "cancel released actual Runtime summary owner" - ); + assert_eq!(leases.load(Ordering::SeqCst), 1); + assert!(!reply.upstream_released.load(Ordering::SeqCst)); reply.finish(); + observed(queued_control).await.unwrap(); observed(fixture.host_executor.drain()).await.unwrap(); assert_eq!(leases.load(Ordering::SeqCst), 0); fixture.executor.drain().await.unwrap(); } +#[tokio::test(flavor = "current_thread")] +async fn runtime_management_authority_waits_off_tokio_and_retains_cancelled_db_admission() { + for stage in ["identity", "binding"] { + let fixture = Fixture::new().await; + let binding = + install_remote_fixture(&fixture, "authority-runtime", "http://127.0.0.1:9", true); + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let writer = fixture.store.clone(); + fixture + .executor + .dispatch(move || { + writer + .mark_api_token_used("missing-token", "2026-01-01T00:00:00Z") + .unwrap(); + }) + .unwrap(); + observed(waiting).await.unwrap(); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let api = fixture.api.clone(); + let expected = binding.clone(); + let mut authority = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + async move { + if stage == "identity" { + require_active_runtime_registration_identity(&api).await + } else { + require_current_runtime_test_binding(&api, expected) + .await + .map(|_| ()) + } + }, + )); + assert!( + futures::poll!(&mut authority).is_pending(), + "{stage}: authority must queue without blocking Tokio on the shared mutex" + ); + runtime_progress_while_locked(&lock).await; + let overload = if stage == "identity" { + require_active_runtime_registration_identity(&fixture.api) + .await + .unwrap_err() + } else { + require_current_runtime_test_binding(&fixture.api, binding.clone()) + .await + .unwrap_err() + }; + response_json(overload.into_response(), StatusCode::SERVICE_UNAVAILABLE).await; + drop(authority); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "{stage}: cancelled DB authority lost its lease" + ); + lock.release(); + busy.resume(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + require_active_runtime_registration_identity(&fixture.api) + .await + .unwrap(); + assert_eq!( + require_current_runtime_test_binding(&fixture.api, binding.clone()) + .await + .unwrap() + .binding_id, + binding.binding_id + ); + fixture + .store + .revoke_workspace_runtime_binding( + WORKSPACE, + &binding.runtime_id, + "2026-01-02T00:00:00Z", + ) + .unwrap(); + let stale = require_current_runtime_test_binding(&fixture.api, binding) + .await + .unwrap_err(); + response_json(stale.into_response(), StatusCode::CONFLICT).await; + } +} + +#[tokio::test(flavor = "current_thread")] +async fn runtime_connection_post_ping_sql_waits_off_tokio_while_host_control_progresses() { + let fixture = Fixture::new().await; + let mut reply = HeldRuntimeReply::new(); + let binding = install_remote_fixture( + &fixture, + "ping-runtime", + &format!("http://{}", reply.address), + true, + ); + let mut test = Box::pin(test_runtime_connection( + State(fixture.api.clone()), + AxumPath(binding.runtime_id.clone()), + )); + assert!(futures::poll!(&mut test).is_pending()); + observed(fixture.executor.drain()).await.unwrap(); + assert!(futures::poll!(&mut test).is_pending()); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!(request.contains("/ping"), "{request}"); + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let writer = fixture.store.clone(); + fixture + .executor + .dispatch(move || { + writer + .mark_api_token_used("missing-token", "2026-01-01T00:00:00Z") + .unwrap(); + }) + .unwrap(); + observed(waiting).await.unwrap(); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + assert!( + futures::poll!(&mut test).is_pending(), + "post-ping binding fence must await DB without a synchronous mutex wait" + ); + runtime_progress_while_locked(&lock).await; + assert_eq!( + observed(fixture.host_executor.run(|| Ok::<_, Error>("control"))) + .await + .unwrap(), + "control" + ); + lock.release(); + busy.resume(); + let Json(result) = observed(test).await.unwrap(); + assert_eq!(result.binding_id, binding.binding_id); + fixture.executor.drain().await.unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn cancelled_runtime_metadata_mutation_keeps_commit_and_registration_publication_owned() { + let fixture = Fixture::new().await; + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let endpoint = format!("http://{}", listener.local_addr().unwrap()); + let server = tokio::spawn(async move { + axum::serve( + listener, + Router::new().fallback(|| async { StatusCode::SERVICE_UNAVAILABLE }), + ) + .await + .unwrap(); + }); + install_remote_fixture(&fixture, "metadata-runtime", &endpoint, true); + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let mut update = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + update_remote_runtime_metadata_owned( + fixture.api.clone(), + WORKSPACE.into(), + "metadata-runtime".into(), + "updated metadata".into(), + endpoint, + ), + )); + assert!(futures::poll!(&mut update).is_pending()); + observed(waiting).await.unwrap(); // The real metadata SQL mutation is waiting. + runtime_progress_while_locked(&lock).await; + drop(update); + assert_eq!( + leases.load(Ordering::SeqCst), + 2, + "actual Host workflow and its accepted SQL both retain authority" + ); + lock.release(); + busy.resume(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + let saved = fixture + .store + .get_workspace_runtime_binding(WORKSPACE, "metadata-runtime") + .unwrap() + .unwrap(); + assert_eq!(saved.display_name, "updated metadata"); + let published = fixture + .api + .runtime_binding_expectations + .read() + .unwrap() + .get(&(WORKSPACE.into(), "metadata-runtime".into())) + .cloned() + .unwrap(); + assert_eq!( + published, saved, + "SQL completion must publish even after waiter cancellation" + ); + fixture + .api + .runtime_subscription_broker + .unregister_runtime("metadata-runtime"); + fixture.api.worker_projection.shutdown_async().await; + fixture.executor.drain().await.unwrap(); + server.abort(); + let _ = server.await; +} + +#[tokio::test(flavor = "current_thread")] +async fn runtime_metadata_reads_wait_off_tokio_and_db_and_retain_cancelled_host_ownership() { + for read in ["resources", "launch_options"] { + let fixture = Fixture::new().await; + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "summary-runtime", + &format!("http://{}", reply.address), + true, + ); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let api = fixture.api.clone(); + let task = tokio::spawn(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + async move { + if read == "resources" { + workspace_runtime_resources_response(&api, WORKSPACE) + .await + .map(|_| ()) + } else { + get_worker_launch_options(State(api)).await.map(|_| ()) + } + }, + )); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!(request.contains("/v1/runtime"), "{request}"); + assert!( + !reply.upstream_released.load(Ordering::SeqCst), + "upstream watchdog released before Tokio could observe the request" + ); + let read_store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || read_store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; // timer contract only + assert!(!reply.upstream_released.load(Ordering::SeqCst)); + assert!(!task.is_finished()); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "cancel released actual Runtime summary owner" + ); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + fixture.executor.drain().await.unwrap(); + } +} + #[tokio::test(flavor = "current_thread")] async fn worker_discovery_sql_waits_off_tokio_and_keeps_admission_until_actual_cleanup() { // Exercise the exact post-auth handler boundary: signed source verification From 6d8c49b72da6b5d1b57dcc952b730556bc7b5589 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 11:30:03 +0900 Subject: [PATCH 09/14] docs: record Runtime authority and cancellation-safe publication stages --- docs/development/backend-db-execution.md | 17 +++-- .../report/2026-10-12-t748-suite-diagnosis.md | 67 ++++++++++++++++++- 2 files changed, 78 insertions(+), 6 deletions(-) diff --git a/docs/development/backend-db-execution.md b/docs/development/backend-db-execution.md index 4bb0ac804..5126c4735 100644 --- a/docs/development/backend-db-execution.md +++ b/docs/development/backend-db-execution.md @@ -116,10 +116,19 @@ Worker discovery uses the same whole `workers_response` DB stage as Worker list and resource detail: projection snapshot, Workdir registry/links, and authority projection never acquire the shared connection on Tokio. Source-proof requirement and pure query/cursor validation still precede admission; overload remains 503. -Runtime resource list/detail use a finite Host stage for synchronous Runtime -summary I/O, then await the independent DB binding read. Do not confuse a Runtime -summary callback with the pure stored Worker projection or run network waits on -the sole DB worker. +Runtime resource list/detail use an accepted Host workflow for synchronous Runtime +summary I/O and await independent DB binding/audit reads. Launch options are also +mixed Runtime/config helpers and use Host, not the sole DB worker. Runtime +registration validates its signing identity in a whole DB stage, scans existing +Workers on Host, then owns SQL key/CAS mutation, local registration publication +and response observation in one accepted workflow. Metadata updates likewise keep +commit and publication together after waiter cancellation. Public resource/detail +helpers admit Host ownership; already-owned workflows call their `_on_host` +variants directly, never await another admission to the same occupied lane. +Connection tests ping on finite Host ownership and then await the whole current +binding fence on DB. A replaced/revoked binding cannot attest the earlier ping. +Do not confuse a Runtime summary callback with a pure stored Worker projection +or run network waits on the sole DB worker. ## Regression evidence diff --git a/docs/report/2026-10-12-t748-suite-diagnosis.md b/docs/report/2026-10-12-t748-suite-diagnosis.md index 9f7730463..47410b47c 100644 --- a/docs/report/2026-10-12-t748-suite-diagnosis.md +++ b/docs/report/2026-10-12-t748-suite-diagnosis.md @@ -311,8 +311,71 @@ original default-parallel Server **965 passed / 0 failed / 1 existing ignored**, executor panics appeared, with no residual test processes. Workdir default153 / http-client156 and fmt/diff/naming558/0/0 passed. Logs use the same isolated candidate cache: `target/t748-discovery-summary-{root-check,full-suite,workdir-default,workdir-http}.log`. -The following docs-only publication changes no tested code; fresh exact-source -independent review is required before an approved handoff. +Docs-only publication `b63bdb44` changed no tested code. MR seq9 independently +confirmed these fixes, then requested changes for Runtime management's residual +synchronous authority calls; it was not an approval of the complete source. + +## Runtime management: authority, network, and accepted publication ownership + +Seq9 major `T748-ASYNC-RUNTIME-MANAGEMENT` identified registration's direct +`get_validated` signing identity read and post-ping direct live-binding fence. +The Reviewer reproduced both exact compiled helpers waiting approximately 300ms +on the shared SQL mutex with no Tokio timer progress (helper-boundary, not endpoint +or E2E proof). Registration's synchronous Runtime Worker scan also requires a +network/authority boundary; ping previously used an unbounded blocking admission. + +Fix `925bce2a` introduces whole owned DB stages for validated active signing +identity and the post-ping current binding fence. Runtime Worker scans and ping +use existing finite Host admission. A changed/revoked binding still produces +conflict rather than attesting the earlier connection; owner/key/CAS validation +and existing whole store transactions are retained. + +The adjacent management audit also closes cancellation separation between SQL +commit and local Runtime publication. Registration and metadata updates now own +mutation, expectations/registry/subscription publication, and response observation +in one accepted Host workflow, with their SQL helper awaited on the independent +DB owner. Public Runtime resource/detail helpers admit ownership; the already-owned +workflows call explicit `_on_host` variants directly instead of reacquiring the +same lane. There is no Runtime/network await inside a SQL transaction or nested +Host admission. A cancelled waiter cannot strand a committed metadata change +without publishing the matching binding. + +Launch options were another mixed Runtime summary/config helper placed on sole +DB. Its signed Runtime authorization could enqueue DB work behind its own waiting +parent. It now uses Host, preserving SQL/control progress and finite ownership. +The audit traced remaining signing validation/binding-fence calls: UI signing +operations already use DB, Runtime gate calls occur on Host, and removal already +has an owned Host workflow. DNS validation stays async; pure local registration +construction/notification stays in the accepted publication owner. + +Permanent regressions cover both authority stages under a real separate SQLite +IMMEDIATE writer and shared-mutex busy observer: current-thread timer/control +progress, overload503, cancelled actual DB lease retention/cleanup, positive +validation and revoked binding409. An actual signed loopback ping is held while +SQL contention starts; after its reply the real handler's post-ping fence queues +without blocking Tokio or Host control. Another signed loopback scan proves SQL/ +timer progress and retained cancelled finite Host admission. The metadata mutation +case cancels while its own SQL is waiting, then observes one durable update and +an equal published binding after actual Host/DB drain. Producers are stopped and +drained while the original fixture runtime remains alive. + +Old identity, binding, scan and launch placements each fail their expected +contention/watchdog boundary; fixed code passes. Logs: +`target/t748-management-old-{identity,binding,scan,launch}-placement-red.log`, +`target/t748-management-final-restored-green.log` (16 boundaries), registration +policy11 and connection policy13. A zero-match launch-options exploration is not +counted as proof; its real held-summary case is in the 16 boundaries. The first +fixture teardown named an unavailable method and failed compilation; it was +corrected to the existing `shutdown_async`, not counted as a semantic test run. + +Exact code `925bce2a405b6d3255887fba88867ab8d23882dc` passed root `cargo check` and +original default-parallel Server **969 passed / 0 failed / 1 existing ignored**, +11 binary tests; library duration **172.25 seconds**. Only three deliberate +executor panics appeared, with no residual test processes. Workdir default153 / +http-client156, fmt/diff/naming558/0/0 passed. Same isolated cache; evidence logs: +`target/t748-management-{root-check,full-suite,workdir-default,workdir-http}.log`. +The following docs-only publication changes no tested code and still requires a +fresh exact-source independent verdict before an approved handoff. No latency benchmark, production incident measurement, target integration, Ticket conclusion, or running-environment update is claimed. T743 has now From 1a065db0fc5d07e2dcc1b9dac2fcfc0607b9a23c Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 12:12:50 +0900 Subject: [PATCH 10/14] fix(server): retain trust-key revoke publication after cancellation --- crates/workspace-server/src/server.rs | 122 +++++----- .../src/server_db_execution_tests.rs | 222 ++++++++++++++++++ 2 files changed, 290 insertions(+), 54 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index becd6fd80..3283f13da 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -9403,62 +9403,70 @@ impl server_api::ServerApi for ServerApiContractService { "expected_binding_id must contain between 1 and 256 bytes", ))); } - let now = Utc::now().to_rfc3339(); - if let Err(error) = api - .store - .revoke_workspace_runtime_binding_key( - &workspace_id, - &runtime_id, - &request.expected_binding_id, - &actor.account_id, - &now, - ) - .await - { - let kind = match error { - Error::RuntimeBindingIdConflict { .. } => { - Some(server_api::RuntimeTrustConflictKind::StaleBinding) - } - Error::RuntimeBindingFingerprintConflict { .. } => { - Some(server_api::RuntimeTrustConflictKind::FingerprintInUse) - } - _ => None, - }; - if let Some(kind) = kind { - let current = api - .store - .get_workspace_runtime_binding(&workspace_id, &runtime_id) - .await - .map_err(|error| map_error(ApiError::from(error)))?; - return Err(server_api::RuntimeManagementApiError::trust_conflict( - server_api::RuntimeTrustConflictResponse { - error: kind, - message: match kind { - server_api::RuntimeTrustConflictKind::StaleBinding => { - "the Runtime trust binding changed; reload before retrying".to_string() - } - server_api::RuntimeTrustConflictKind::FingerprintInUse => { - "the public key is already bound to another Runtime in this Workspace".to_string() - } + // Own the whole revoke outcome through local publication and response. + // The accepted DB mutation outlives a cancelled HTTP waiter; its Host + // parent must also remove expectations and terminate subscriptions. + // SQL keeps its independent owner, and response reads stay on this Host + // workflow rather than attempting nested admission to the same lane. + run_host_workflow(api.host_executors.host()?, async move { + let now = Utc::now().to_rfc3339(); + if let Err(error) = api + .store + .revoke_workspace_runtime_binding_key( + &workspace_id, + &runtime_id, + &request.expected_binding_id, + &actor.account_id, + &now, + ) + .await + { + let kind = match error { + Error::RuntimeBindingIdConflict { .. } => { + Some(server_api::RuntimeTrustConflictKind::StaleBinding) + } + Error::RuntimeBindingFingerprintConflict { .. } => { + Some(server_api::RuntimeTrustConflictKind::FingerprintInUse) + } + _ => None, + }; + if let Some(kind) = kind { + let current = api + .store + .get_workspace_runtime_binding(&workspace_id, &runtime_id) + .await + .map_err(|error| map_error(ApiError::from(error)))?; + return Err(server_api::RuntimeManagementApiError::trust_conflict( + server_api::RuntimeTrustConflictResponse { + error: kind, + message: match kind { + server_api::RuntimeTrustConflictKind::StaleBinding => { + "the Runtime trust binding changed; reload before retrying".to_string() + } + server_api::RuntimeTrustConflictKind::FingerprintInUse => { + "the public key is already bound to another Runtime in this Workspace".to_string() + } + }, + current_binding_id: current.as_ref().map(|binding| binding.binding_id.clone()), + current_fingerprint: current + .as_ref() + .map(|binding| binding.public_key_fingerprint.clone()), }, - current_binding_id: current.as_ref().map(|binding| binding.binding_id.clone()), - current_fingerprint: current - .as_ref() - .map(|binding| binding.public_key_fingerprint.clone()), - }, - )); + )); + } + return Err(map_error(ApiError::from(error))); } - return Err(map_error(ApiError::from(error))); - } - api.runtime_binding_expectations - .write() - .unwrap_or_else(std::sync::PoisonError::into_inner) - .remove(&(workspace_id.clone(), runtime_id.clone())); - api.runtime_subscription_broker - .unregister_runtime(&runtime_id); - workspace_runtime_detail(&api, &workspace_id, &runtime_id) - .await - .map_err(map_error) + api.runtime_binding_expectations + .write() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .remove(&(workspace_id.clone(), runtime_id.clone())); + api.runtime_subscription_broker + .unregister_runtime(&runtime_id); + workspace_runtime_detail_on_host(&api, &workspace_id, &runtime_id) + .await + .map_err(map_error) + }) + .await } async fn runtime_connection_test( @@ -34715,6 +34723,12 @@ impl From for server_api::RepositoryApiError { } } +impl From for server_api::RuntimeManagementApiError { + fn from(error: Error) -> Self { + Self::from(server_api::RepositoryApiError::from(error)) + } +} + impl From for ApiError { fn from(error: Error) -> Self { let diagnostics = match &error { diff --git a/crates/workspace-server/src/server_db_execution_tests.rs b/crates/workspace-server/src/server_db_execution_tests.rs index 4d287776c..6cfa24529 100644 --- a/crates/workspace-server/src/server_db_execution_tests.rs +++ b/crates/workspace-server/src/server_db_execution_tests.rs @@ -1370,6 +1370,228 @@ async fn runtime_connection_post_ping_sql_waits_off_tokio_while_host_control_pro fixture.executor.drain().await.unwrap(); } +#[tokio::test(flavor = "current_thread")] +async fn saturated_host_rejects_runtime_key_revoke_without_mutating_trust() { + let fixture = Fixture::new().await; + fixture.api.worker_projection.shutdown_async().await; + observed(fixture.executor.drain()).await.unwrap(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let binding = install_remote_fixture( + &fixture, + "revoke-runtime", + &format!("http://{}", listener.local_addr().unwrap()), + true, + ); + fixture + .api + .runtime_binding_expectations + .write() + .unwrap() + .insert( + (WORKSPACE.into(), binding.runtime_id.clone()), + binding.clone(), + ); + let initial_registrations = fixture.api.runtime_subscription_broker.runtime_ids(); + // Sender Drop also releases each controlled job if a route assertion panics. + let mut releases: Vec> = Vec::new(); + let (started, running) = oneshot::channel(); + let (release, permit) = mpsc::channel(); + releases.push(release); + fixture + .host_executor + .dispatch(move || { + let _ = started.send(()); + let _ = permit.recv_timeout(WATCHDOG); + }) + .unwrap(); + observed(running).await.unwrap(); + let (release, permit) = mpsc::channel(); + releases.push(release); + fixture + .host_executor + .dispatch(move || { + let _ = permit.recv_timeout(WATCHDOG); + }) + .unwrap(); + let body = response_json( + observed(fixture.dispatch(Fixture::request( + "DELETE", + "/api/w/db-execution/runtimes/revoke-runtime/trust-key", + serde_json::json!({"expected_binding_id": binding.binding_id}), + ))) + .await + .unwrap(), + StatusCode::SERVICE_UNAVAILABLE, + ) + .await; + assert!(body.to_string().contains("db_capacity_exceeded"), "{body}"); + assert_eq!( + fixture + .store + .get_workspace_runtime_binding(WORKSPACE, &binding.runtime_id) + .unwrap() + .unwrap(), + binding + ); + assert!( + fixture + .store + .list_workspace_runtime_binding_audit(WORKSPACE, &binding.runtime_id, 10) + .unwrap() + .is_empty() + ); + assert_eq!( + fixture.api.runtime_subscription_broker.runtime_ids(), + initial_registrations + ); + assert_eq!( + fixture + .api + .runtime_binding_expectations + .read() + .unwrap() + .get(&(WORKSPACE.into(), binding.runtime_id.clone())), + Some(&binding) + ); + drop(releases); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn cancelled_runtime_key_revoke_commits_audit_and_removes_broker_registration() { + let fixture = Fixture::new().await; + // Stop unrelated projection writes before observing this mutation's SQLite + // wait. Keep the original runtime alive through actual workflow cleanup. + fixture.api.worker_projection.shutdown_async().await; + observed(fixture.executor.drain()).await.unwrap(); + let listener = tokio::net::TcpListener::bind("127.0.0.1:0").await.unwrap(); + let binding = install_remote_fixture( + &fixture, + "revoke-runtime", + &format!("http://{}", listener.local_addr().unwrap()), + true, + ); + fixture + .api + .runtime_binding_expectations + .write() + .unwrap() + .insert( + (WORKSPACE.into(), binding.runtime_id.clone()), + binding.clone(), + ); + let mut config = remote_runtime_config_from_binding(&binding).unwrap(); + config.workspace_authorization = Some(WorkspaceRuntimeAuthorization::new( + fixture.store.clone(), + fixture.api.signing_identities.clone(), + "http://localhost:8787", + binding.clone(), + )); + fixture + .api + .runtime_subscription_broker + .register_remote_runtime(config); + let mut subscription = fixture + .api + .runtime_subscription_broker + .subscribe( + &binding.runtime_id, + protocol::subscription::EventSubscriptionSelector::RuntimeWorkers, + ) + .unwrap(); + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let server = fixture.server.clone(); + let request = Fixture::request( + "DELETE", + "/api/w/db-execution/runtimes/revoke-runtime/trust-key", + serde_json::json!({"expected_binding_id": binding.binding_id}), + ); + let revoke = + tokio::spawn(async move { dispatch_workspace_request(State(server), request).await }); + observed(waiting).await.unwrap(); // Actual authenticated revoke reached BEGIN IMMEDIATE. + runtime_progress_while_locked(&lock).await; + revoke.abort(); + assert!(revoke.await.unwrap_err().is_cancelled()); + let admission = fixture.server.admission_for_workspace(WORKSPACE).await; + let mut deletion = Box::pin(admission.lock_deletion()); + assert!( + futures::poll!(&mut deletion).is_pending(), + "cancelled HTTP must retain actual revoke ownership until publication" + ); + assert!( + fixture + .api + .runtime_subscription_broker + .runtime_ids() + .contains(&binding.runtime_id) + ); + lock.release(); + busy.resume(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + let _deletion = observed(deletion).await; + let saved = fixture + .store + .get_workspace_runtime_binding(WORKSPACE, &binding.runtime_id) + .unwrap() + .unwrap(); + assert!(saved.revoked_at.is_some()); + assert_ne!(saved.binding_id, binding.binding_id); + let audit = fixture + .store + .list_workspace_runtime_binding_audit(WORKSPACE, &binding.runtime_id, 10) + .unwrap(); + assert_eq!( + audit.len(), + 1, + "cancelled mutation must commit one audit without retry" + ); + assert_eq!(audit[0].action, "revoked"); + assert_eq!(audit[0].actor_account_id, format!("account-{WORKSPACE}")); + assert_eq!(audit[0].binding_id, saved.binding_id); + assert_eq!(audit[0].at, saved.revoked_at.unwrap()); + assert!( + !fixture + .api + .runtime_subscription_broker + .runtime_ids() + .contains(&binding.runtime_id), + "committed revoke must unregister even after HTTP waiter cancellation" + ); + assert!( + !fixture + .api + .runtime_binding_expectations + .read() + .unwrap() + .contains_key(&(WORKSPACE.into(), binding.runtime_id.clone())) + ); + // Exercise downstream termination at the broker protocol boundary, not a + // product process or complete browser/Runtime E2E flow. + observed(async { + loop { + match subscription.recv().await { + Some(crate::runtime_subscription::BrokerSubscriptionEvent::Closed { + code, .. + }) => { + assert_eq!( + code, + protocol::subscription::SubscriptionTerminationCode::ServerShutdown + ); + break; + } + Some(crate::runtime_subscription::BrokerSubscriptionEvent::Disconnected { + .. + }) => {} + event => panic!("expected revoke shutdown, got {event:?}"), + } + } + }) + .await; +} + #[tokio::test(flavor = "current_thread")] async fn cancelled_runtime_metadata_mutation_keeps_commit_and_registration_publication_owned() { let fixture = Fixture::new().await; From ebce7ab7b1230654b8eb5340afca8de835f24f12 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 12:18:31 +0900 Subject: [PATCH 11/14] docs: record cancellation-safe Runtime trust revocation evidence --- docs/development/backend-db-execution.md | 8 ++- .../report/2026-10-12-t748-suite-diagnosis.md | 58 ++++++++++++++++++- 2 files changed, 63 insertions(+), 3 deletions(-) diff --git a/docs/development/backend-db-execution.md b/docs/development/backend-db-execution.md index 5126c4735..5eea19479 100644 --- a/docs/development/backend-db-execution.md +++ b/docs/development/backend-db-execution.md @@ -122,8 +122,12 @@ mixed Runtime/config helpers and use Host, not the sole DB worker. Runtime registration validates its signing identity in a whole DB stage, scans existing Workers on Host, then owns SQL key/CAS mutation, local registration publication and response observation in one accepted workflow. Metadata updates likewise keep -commit and publication together after waiter cancellation. Public resource/detail -helpers admit Host ownership; already-owned workflows call their `_on_host` +commit and publication together after waiter cancellation. Trust-key revoke also +owns whole CAS/audit SQL, expectation removal, broker unregister/Shutdown and +response observation in one accepted Host workflow. Future request proof denial +alone does not terminate existing subscriptions. Host admission must precede the +mutation, so overload cannot commit a revoke then fail on response admission. +Public resource/detail helpers admit Host ownership; already-owned workflows call their `_on_host` variants directly, never await another admission to the same occupied lane. Connection tests ping on finite Host ownership and then await the whole current binding fence on DB. A replaced/revoked binding cannot attest the earlier ping. diff --git a/docs/report/2026-10-12-t748-suite-diagnosis.md b/docs/report/2026-10-12-t748-suite-diagnosis.md index 47410b47c..af3f9a9c1 100644 --- a/docs/report/2026-10-12-t748-suite-diagnosis.md +++ b/docs/report/2026-10-12-t748-suite-diagnosis.md @@ -374,7 +374,63 @@ original default-parallel Server **969 passed / 0 failed / 1 existing ignored**, executor panics appeared, with no residual test processes. Workdir default153 / http-client156, fmt/diff/naming558/0/0 passed. Same isolated cache; evidence logs: `target/t748-management-{root-check,full-suite,workdir-default,workdir-http}.log`. -The following docs-only publication changes no tested code and still requires a +Docs-only publication `6d8c49b7` changed no tested code. MR seq11 independently +confirmed those fixes and repeated the full validation, but requested changes for +cancelled trust-key revocation; it did not approve the complete snapshot. + +## Trust-key revocation must own post-commit subscription termination + +Seq11 major `T748-RUNTIME-REVOKE-CANCEL-PUBLICATION` reproduced the authenticated +public DELETE trust-key route waiting for an actual SQLite writer. Cancelling its +HTTP waiter did not cancel accepted SQL: revoke, binding-ID replacement and audit +committed, but waiter-owned expectation removal and broker unregister were lost. +The independent artifact used the normal public Server library's inner router, +not outer Workspace dispatch or live-stream/E2E. New signed requests still fail +the live binding fence; that does not shut down an already connected subscription. + +Fix `1a065db0` gives the whole key-revoke outcome one accepted finite Host owner: +intact CAS/audit SQL on the independent DB lane, local expectation removal, broker +unregister/Shutdown, and response observation. Owner/scope/input preflight, typed +trust conflicts and current-binding error responses remain unchanged. Response +reads use the already-owned `_on_host` detail helper, with no nested Host admission. +Admission/unknown-outcome errors retain the normal typed availability mapping; +there is no transaction split, new queue budget or automatic mutation retry. +The adjacent key-mutation caller audit confirms registration and metadata already +own publication and this is the sole production key-revoke caller. + +The permanent cancellation regression exercises the **real outer Workspace +dispatch**, owner browser cookie, generated DELETE route, whole store mutation and +broker. Projection producers stop before the write lock is acquired, so the +fixture-local busy observer identifies this actual revoke transaction, not startup +SQL. Timer/control progress before the watchdog releases SQLite. After waiter +cancellation, the real admitted RequestLease keeps deletion Pending. Releasing the +writer and draining actual Host/DB owners persists exactly one revoke audit, +changes binding ID, removes expectations and broker registration, and delivers +`Closed/ServerShutdown` to the downstream. The loopback endpoint holds a WebSocket +handshake; this proves broker termination, not an established upstream socket or +complete product E2E. Existing broker protocol tests cover socket replacement. + +A separate full-Host route regression observes 503 before mutation and unchanged +binding, audit and publication. Restoring only the exact old revoke method fails +both permanent tests: the cancelled job leaves broker registration behind, and +under overload the old handler commits revoke before its response helper returns +503. Fixed code passes both without retry or reduced parallelism. Evidence: +`target/t748-revoke-final-old-publication-red.log`, +`target/t748-revoke-fixed-boundaries.log` (18), `target/t748-revoke-policy.log` (3), +`target/t748-revoke-binding-policy.log` (16). +An initial counter probe outside dispatch was invalid because dispatch replaces it +with the real RequestLease; it was corrected to observe actual deletion drain. +A test-only overload channel type-inference compile rejection was corrected with +an explicit unit channel. Neither setup error is counted as semantic evidence. + +Exact code `1a065db0fc5d07e2dcc1b9dac2fcfc0607b9a23c` passed root `cargo check` and +the original default-parallel Server **971 passed / 0 failed / 1 existing ignored**, +11 binary tests; library duration **173.05 seconds**. Only three deliberate executor +panic diagnostics appeared, and no retained test/repro processes were observed. +Workdir default153/http-client156, fmt/diff and naming558/0/0 passed. All Cargo +checks/tests use the isolated candidate cache. Logs: +`target/t748-revoke-{root-check,full-suite,workdir-default,workdir-http,fmt,names}.log`. +The following documentation-only publication changes no tested code and needs a fresh exact-source independent verdict before an approved handoff. No latency benchmark, production incident measurement, target integration, From 5873f250736fb06c35176db71097e6168064cbc7 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 14:46:05 +0900 Subject: [PATCH 12/14] fix(server): own residual Ticket MR and worker request stages --- crates/workspace-server/src/server.rs | 468 ++++++---- .../src/server_db_execution_tests.rs | 72 +- .../src/server_residual_execution_tests.rs | 843 ++++++++++++++++++ 3 files changed, 1196 insertions(+), 187 deletions(-) create mode 100644 crates/workspace-server/src/server_residual_execution_tests.rs diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 3283f13da..4d75d5985 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -13555,8 +13555,8 @@ async fn scoped_get_ticket( State(api): State, AxumPath(path): AxumPath, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; browser_ticket_detail(&api, &path.id) @@ -13569,8 +13569,8 @@ async fn scoped_query_tickets( AxumPath(path): AxumPath, Json(query): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; Ok(Json(api.authority.query_tickets(query)?)) @@ -13583,8 +13583,8 @@ async fn scoped_show_ticket( AxumPath(path): AxumPath, Json(query): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let backend = browser_ticket_backend(&api)?; @@ -13668,8 +13668,8 @@ async fn scoped_list_ticket_assignments( State(api): State, AxumPath(path): AxumPath, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let ticket = api.authority.ticket(&path.id)?; @@ -14337,8 +14337,8 @@ async fn apply_set_ticket_assignment( let prep_api = api.clone(); let prep_workspace = workspace_id.clone(); let (ticket, role, operation_id, expected_assignment_id, record, workdir_bindings) = api - .store - .db_executor() + .host_executors + .host()? .run(move || { let api = &prep_api; let workspace_id = prep_workspace; @@ -14488,8 +14488,8 @@ async fn apply_clear_ticket_assignment( author: String, source: Option, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &workspace_id)?; let ticket = api.authority.ticket(&id)?; @@ -14560,60 +14560,65 @@ async fn scoped_cancel_ticket_implementation( Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let ticket = api.authority.ticket(&path.id)?; - let operation_id = require_ticket_assignment_value("operation_id", request.operation_id)?; - let assignment_id = require_ticket_assignment_value("assignment_id", request.assignment_id)?; - let reason = require_ticket_assignment_value("reason", request.reason)?; - if reason.len() > 512 { - return Err(Error::InvalidInput( - "implementation cancellation reason must be at most 512 bytes".to_string(), - ) - .into()); - } + // Runtime cancel and session cleanup are mixed work. One accepted Host + // owner retains authority through the whole cancellation transaction/detail. + run_host_workflow(api.host_executors.host()?, async move { + let ticket = api.authority.ticket(&path.id)?; + let operation_id = require_ticket_assignment_value("operation_id", request.operation_id)?; + let assignment_id = require_ticket_assignment_value("assignment_id", request.assignment_id)?; + let reason = require_ticket_assignment_value("reason", request.reason)?; + if reason.len() > 512 { + return Err(Error::InvalidInput( + "implementation cancellation reason must be at most 512 bytes".to_string(), + ) + .into()); + } - let current = api.store.get_active_ticket_role_assignment( - &path.workspace_id, - &ticket.id, - TicketAssignmentRole::Worker, - )?; - if let Some(assignment) = current.filter(|value| value.assignment_id == assignment_id) - && let TicketAssignmentPrincipal::Worker { - runtime_id, - worker_id, - } = assignment.principal - { - if api - .store - .get_ticket_assignment_operation(&path.workspace_id, &operation_id)? - .is_some() + let current = api.store.get_active_ticket_role_assignment( + &path.workspace_id, + &ticket.id, + TicketAssignmentRole::Worker, + )?; + if let Some(assignment) = current.filter(|value| value.assignment_id == assignment_id) + && let TicketAssignmentPrincipal::Worker { + runtime_id, + worker_id, + } = assignment.principal { + if api + .store + .get_ticket_assignment_operation(&path.workspace_id, &operation_id)? + .is_some() + { + return Err(Error::TicketAssignmentConflict(format!( + "operation `{operation_id}` was already used for another Ticket assignment mutation" + )) + .into()); + } + let worker = RuntimeWorkerRef::new(runtime_id, worker_id); + cancel_ticket_worker(&api, &worker, &reason).await?; + } + + let cancelled = api.store.cancel_current_ticket_worker_assignment( + &path.workspace_id, + &ticket.id, + &assignment_id, + &new_id("tasev"), + &new_id("tev"), + &operation_id, + "workspace-web", + &Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true), + &reason, + )?; + if !cancelled { return Err(Error::TicketAssignmentConflict(format!( - "operation `{operation_id}` was already used for another Ticket assignment mutation" + "assignment `{assignment_id}` is not the current Coder implementation" )) .into()); } - let worker = RuntimeWorkerRef::new(runtime_id, worker_id); - cancel_ticket_worker(&api, &worker, &reason).await?; - } - - let cancelled = api.store.cancel_current_ticket_worker_assignment( - &path.workspace_id, - &ticket.id, - &assignment_id, - &new_id("tasev"), - &new_id("tev"), - &operation_id, - "workspace-web", - &Utc::now().to_rfc3339_opts(SecondsFormat::Secs, true), - &reason, - )?; - if !cancelled { - return Err(Error::TicketAssignmentConflict(format!( - "assignment `{assignment_id}` is not the current Coder implementation" - )) - .into()); - } - browser_ticket_detail(&api, &ticket.id) + browser_ticket_detail(&api, &ticket.id) + }) + .await } async fn cancel_ticket_worker( @@ -15070,8 +15075,8 @@ async fn scoped_edit_ticket_item( AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let mut targets = request.target.map(|target| match target { @@ -15125,8 +15130,8 @@ async fn scoped_append_ticket_event( AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let role = match request.role { @@ -15154,14 +15159,32 @@ async fn scoped_mark_ticket_ready_from_browser( Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let _ = scoped_mark_ticket_ready( - State(api.clone()), - AxumPath((path.workspace_id.clone(), path.id.clone())), - headers, - Json(request), - ) - .await?; - browser_ticket_detail(&api, &path.id) + api.host_executors + .host()? + .run(move || { + let result = execute_ticket_rest_operation_on_host( + &api, + &path.workspace_id, + headers, + None, + TicketBackendOperation::MarkReady { + id: TicketIdOrSlug::Query(path.id.clone()), + request: ticket::TicketMarkReady { + operation_key: request.operation_key, + reason: request.reason, + author: None, + intake_summary: request.intake_summary, + }, + }, + )?; + // Preserve result validation before projecting the committed detail. + let _ = ticket_rest_result(result, |result| match result { + TicketBackendOperationResult::Ticket(ticket) => Some(ticket), + _ => None, + })?; + browser_ticket_detail(&api, &path.id) + }) + .await } async fn scoped_queue_ticket( @@ -15357,7 +15380,7 @@ async fn execute_ticket_rest_operation_with_context( workspace_id: &str, headers: HeaderMap, context: Option<&server_api::ServerRequestContext>, - mut operation: TicketBackendOperation, + operation: TicketBackendOperation, ) -> ApiResult { validate_workspace_scope(api, workspace_id)?; if let TicketBackendOperation::UpdateCase { ticket, request } = operation { @@ -15379,10 +15402,25 @@ async fn execute_ticket_rest_operation_with_context( let job_api = api.clone(); let workspace_id = workspace_id.to_owned(); let context = context.cloned(); - api.host_executors.host()?.run(move || { - let api = &job_api; - let workspace_id = workspace_id.as_str(); - let context = context.as_ref(); + api.host_executors + .host()? + .run(move || { + let api = &job_api; + let workspace_id = workspace_id.as_str(); + let context = context.as_ref(); + execute_ticket_rest_operation_on_host(api, workspace_id, headers, context, operation) + }) + .await +} + +// Already-owned callers must not re-admit to the occupied Host lane. +fn execute_ticket_rest_operation_on_host( + api: &WorkspaceApi, + workspace_id: &str, + headers: HeaderMap, + context: Option<&server_api::ServerRequestContext>, + mut operation: TicketBackendOperation, +) -> ApiResult { validate_ticket_operation_repository_keys(api, workspace_id, &operation)?; let mut backend = SqliteTicketBackend::open_verified( api.config.database_path.clone(), @@ -15580,7 +15618,6 @@ async fn execute_ticket_rest_operation_with_context( } } Ok(result) - }).await } #[cfg(test)] @@ -17044,37 +17081,46 @@ async fn scoped_repair_merge_request_selector( if !input.explicit_confirmation { return Err(Error::BrowserReopenConfirmationRequired.into()); } - let store = merge_request_store(&api, &workspace_id)?; - let mr = store.get_by_id(&workspace_id, &merge_request_id)?; - let ticket_id = merge_request_ticket_id(&mr)?; - let assignment = api - .store - .get_active_ticket_worker_assignment(&workspace_id, &ticket_id)? - .ok_or_else(|| { - Error::TicketAssignmentConflict("Ticket has no active Coder work assignment".into()) - })?; - let resolved_subject_ref = observe_published_source_ref( - &api, - &workspace_id, - &assignment.worker.runtime_id, - &mr.repository_id, - &input.selector_from, - )? - .resolved_ref; - let repaired = store.repair_selector_from(merge_request::RepairSelectorFrom { - workspace_id: workspace_id.clone(), - merge_request_id, - ticket_id, - selector_from: input.selector_from, - resolved_subject_ref, - repaired_by: merge_request::WorkerIdentity { - runtime_id: "browser".into(), - worker_id: "authenticated-user".into(), - }, - reason: input.reason, - now: Utc::now(), - })?; - Ok(Json(public_merge_request(&api, &workspace_id, repaired)?)) + // Provider observation and intact repair/audit SQL share an accepted owner; + // authentication's earlier DB await does not offload this mixed stage. + api.host_executors + .host()? + .run(move || { + let store = merge_request_store(&api, &workspace_id)?; + let mr = store.get_by_id(&workspace_id, &merge_request_id)?; + let ticket_id = merge_request_ticket_id(&mr)?; + let assignment = api + .store + .get_active_ticket_worker_assignment(&workspace_id, &ticket_id)? + .ok_or_else(|| { + Error::TicketAssignmentConflict( + "Ticket has no active Coder work assignment".into(), + ) + })?; + let resolved_subject_ref = observe_published_source_ref( + &api, + &workspace_id, + &assignment.worker.runtime_id, + &mr.repository_id, + &input.selector_from, + )? + .resolved_ref; + let repaired = store.repair_selector_from(merge_request::RepairSelectorFrom { + workspace_id: workspace_id.clone(), + merge_request_id, + ticket_id, + selector_from: input.selector_from, + resolved_subject_ref, + repaired_by: merge_request::WorkerIdentity { + runtime_id: "browser".into(), + worker_id: "authenticated-user".into(), + }, + reason: input.reason, + now: Utc::now(), + })?; + Ok(Json(public_merge_request(&api, &workspace_id, repaired)?)) + }) + .await } async fn scoped_register_reviewer_child_session( @@ -17473,8 +17519,8 @@ async fn scoped_ticket_assignment_tracking_update_with_context( Json(update): Json, ) -> ApiResult> { let context = context.clone(); - api.store - .db_executor() + api.host_executors + .host()? .run(move || { let context = &context; @@ -17623,7 +17669,7 @@ async fn apply_ticket_case_update( Json(mut request): Json, ) -> ApiResult> { let actor = actor.cloned(); - api.store.db_executor().run(move || { + api.host_executors.host()?.run(move || { let actor = actor.as_ref(); validate_workspace_scope(&api, &workspace_id)?; @@ -18224,6 +18270,20 @@ fn authenticate_worker_mutation_source( Ok(worker) } +async fn authenticate_worker_mutation_source_owned( + api: &WorkspaceApi, + workspace_id: &str, + headers: &HeaderMap, +) -> Result { + let api = api.clone(); + let workspace_id = workspace_id.to_owned(); + let headers = headers.clone(); + api.host_executors + .host()? + .run(move || authenticate_worker_mutation_source(&api, &workspace_id, &headers)) + .await +} + fn current_worker_identity( api: &WorkspaceApi, workspace_id: &str, @@ -24031,7 +24091,8 @@ async fn send_known_worker_input( Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let source = authenticate_worker_mutation_source(&api, &path.workspace_id, &headers)?; + let source = + authenticate_worker_mutation_source_owned(&api, &path.workspace_id, &headers).await?; let context = WorkerOperationContext::WorkerControl { controller: RuntimeWorkerRef::new(&source.runtime_id, &source.worker_id), }; @@ -24069,7 +24130,8 @@ async fn cancel_known_worker( Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let source = authenticate_worker_mutation_source(&api, &path.workspace_id, &headers)?; + let source = + authenticate_worker_mutation_source_owned(&api, &path.workspace_id, &headers).await?; let context = WorkerOperationContext::WorkerControl { controller: RuntimeWorkerRef::new(&source.runtime_id, &source.worker_id), }; @@ -24090,7 +24152,8 @@ async fn stop_known_worker( Json(request): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let source = authenticate_worker_mutation_source(&api, &path.workspace_id, &headers)?; + let source = + authenticate_worker_mutation_source_owned(&api, &path.workspace_id, &headers).await?; let context = WorkerOperationContext::WorkerControl { controller: RuntimeWorkerRef::new(&source.runtime_id, &source.worker_id), }; @@ -24111,7 +24174,8 @@ async fn restore_known_worker( request: server_api::WorkerRestoreRequest, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let source = authenticate_worker_mutation_source(&api, &path.workspace_id, &headers)?; + let source = + authenticate_worker_mutation_source_owned(&api, &path.workspace_id, &headers).await?; let context = WorkerOperationContext::WorkerControl { controller: RuntimeWorkerRef::new(&source.runtime_id, &source.worker_id), }; @@ -24179,7 +24243,8 @@ async fn scoped_capture_worker_observation_session( Json(subject): Json, ) -> ApiResult> { validate_workspace_scope(&api, &path.workspace_id)?; - let source = authenticate_worker_mutation_source(&api, &path.workspace_id, &headers)?; + let source = + authenticate_worker_mutation_source_owned(&api, &path.workspace_id, &headers).await?; let server_api::WorkerObservationSubjectRef::RuntimeWorker { runtime_id, worker_id, @@ -24191,20 +24256,47 @@ async fn scoped_capture_worker_observation_session( }; let target = RuntimeWorkerRef::new(runtime_id, worker_id); let controller = RuntimeWorkerRef::new(&source.runtime_id, &source.worker_id); - let grant = authorize_known_worker_permission( - &api, - &path.workspace_id, - &controller, - &target, - "observe", - )?; + let grant_api = api.clone(); + let grant_controller = controller.clone(); + let grant_target = target.clone(); + let workspace_id = path.workspace_id.clone(); + let grant = api + .store + .db_executor() + .run(move || { + authorize_known_worker_permission( + &grant_api, + &workspace_id, + &grant_controller, + &grant_target, + "observe", + ) + }) + .await?; let lock = worker_control_lock(&api, &grant.grant_id); - let _guard = lock.lock().await; - authorize_known_worker_permission(&api, &path.workspace_id, &controller, &target, "observe")?; - let target_summary = api - .runtime - .worker(&target) - .map_err(|error| error.into_error())?; + let guard = lock.lock_owned().await; + let lookup_api = api.clone(); + let lookup_target = target.clone(); + let (_guard, target_summary) = api + .host_executors + .host()? + .run(move || { + // Retain the control guard during the actual re-fence and live lookup, + // even if the observation caller cancels its wait. + authorize_known_worker_permission( + &lookup_api, + &path.workspace_id, + &controller, + &lookup_target, + "observe", + )?; + let target_summary = lookup_api + .runtime + .worker(&lookup_target) + .map_err(|error| error.into_error())?; + Ok::<_, Error>((guard, target_summary)) + }) + .await?; if matches!( target_summary.state.as_str(), "stopped" | "failed" | "rejected" | "disconnected" @@ -28973,8 +29065,8 @@ async fn get_ticket( State(api): State, AxumPath(id): AxumPath, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || Ok(Json(api.authority.ticket(&id)?))) .await } @@ -30705,45 +30797,81 @@ async fn restore_runtime_worker_with_context( request: server_api::WorkerRestoreRequest, ) -> ApiResult> { let worker = WorkspaceWorker::resolve_async(&api, &runtime_id, &worker_id).await?; - let mut result = worker.restore(&context, request).await?; - let projected_worker = if let Some(worker) = result.worker.as_ref() { - // A replayed terminal receipt is historical evidence, not a new Runtime observation. - let record = api - .store - .get_worker_registry(api.workspace_id(), &worker.worker)? - .ok_or_else(|| Error::UnknownWorker { - worker: worker.worker.clone(), - })?; - let links = api - .store - .list_worker_workdir_links(&api.config.workspace_id, &record.worker)?; - let workdirs = api - .store - .list_workdir_registry(&api.config.workspace_id, 500)?; - match project_worker_registry_projection(&api, Some(worker), &record, links, &workdirs) { - Ok(worker) => Some(worker), - Err(error) => { - result.diagnostics.push(RuntimeDiagnostic::new( - "worker_restore_current_projection_unavailable", - "warning", - sanitize_backend_error(&error.error.to_string()), - )); + let result = worker.restore(&context, request).await?; + project_worker_restore_response_owned(api, runtime_id, worker_id, result).await +} + +// Post-restore stored projection is its own whole DB stage. Historical terminal +// receipts are never turned into a fresh Runtime observation or registry write. +async fn project_worker_restore_response_owned( + api: WorkspaceApi, + runtime_id: String, + worker_id: String, + mut result: crate::hosts::InternalWorkerRestoreResult, +) -> ApiResult> { + // A terminal result without a Worker has no stored projection to read. + // Do not turn that pure response into a new availability failure on DB overload. + if result.worker.is_none() { + return Ok(Json(server_api::WorkerRestoreResponse { + workspace_id: api.workspace_id().to_string(), + runtime_id, + worker_id, + result: server_api::WorkerRestoreResult { + state: result.state, + worker: None, + diagnostics: result.diagnostics.into_iter().map(Into::into).collect(), + }, + })); + } + api.store + .db_executor() + .run(move || { + let projected_worker = if let Some(worker) = result.worker.as_ref() { + // A replayed terminal receipt is historical evidence, not a new Runtime observation. + let record = api + .store + .get_worker_registry(api.workspace_id(), &worker.worker)? + .ok_or_else(|| Error::UnknownWorker { + worker: worker.worker.clone(), + })?; + let links = api + .store + .list_worker_workdir_links(&api.config.workspace_id, &record.worker)?; + let workdirs = api + .store + .list_workdir_registry(&api.config.workspace_id, 500)?; + match project_worker_registry_projection( + &api, + Some(worker), + &record, + links, + &workdirs, + ) { + Ok(worker) => Some(worker), + Err(error) => { + result.diagnostics.push(RuntimeDiagnostic::new( + "worker_restore_current_projection_unavailable", + "warning", + sanitize_backend_error(&error.error.to_string()), + )); + None + } + } + } else { None - } - } - } else { - None - }; - Ok(Json(server_api::WorkerRestoreResponse { - workspace_id: api.workspace_id().to_string(), - runtime_id: runtime_id.clone(), - worker_id: worker_id.clone(), - result: server_api::WorkerRestoreResult { - state: result.state, - worker: projected_worker, - diagnostics: result.diagnostics.into_iter().map(Into::into).collect(), - }, - })) + }; + Ok(Json(server_api::WorkerRestoreResponse { + workspace_id: api.workspace_id().to_string(), + runtime_id: runtime_id.clone(), + worker_id: worker_id.clone(), + result: server_api::WorkerRestoreResult { + state: result.state, + worker: projected_worker, + diagnostics: result.diagnostics.into_iter().map(Into::into).collect(), + }, + })) + }) + .await } fn reject_workdir_for_embedded_runtime(runtime_id: &str, has_workdir: bool) -> ApiResult<()> { diff --git a/crates/workspace-server/src/server_db_execution_tests.rs b/crates/workspace-server/src/server_db_execution_tests.rs index 6cfa24529..d52ffacd6 100644 --- a/crates/workspace-server/src/server_db_execution_tests.rs +++ b/crates/workspace-server/src/server_db_execution_tests.rs @@ -1,4 +1,6 @@ //! Real SQLite waits at the HTTP/authority boundary (not mock executor jobs). +#[path = "server_residual_execution_tests.rs"] +mod residual; use super::*; use crate::SqliteWorkspaceStore; use crate::db_executor::{DbExecutor, DbExecutorLimits, DbOperationLifetime}; @@ -36,6 +38,10 @@ struct Fixture { impl Fixture { async fn new() -> Self { + Self::with_repositories(Vec::new()).await + } + + async fn with_repositories(repositories: Vec) -> Self { let dir = tempfile::tempdir().unwrap(); let executor = DbExecutor::new(DbExecutorLimits { workers: NonZeroUsize::new(1).unwrap(), @@ -59,7 +65,7 @@ impl Fixture { cookie_name: "db_execution_session".into(), }, max_records: 100, - repositories: Vec::new(), + repositories, runtime_event_sources: Vec::new(), remote_runtime_sources: Vec::new(), backend_base_url: Some("http://localhost:8787".into()), @@ -72,6 +78,24 @@ impl Fixture { seed_test_registered_workspace(store.as_ref(), &config) .await .unwrap(); + for repository in &config.repositories { + store + .upsert_repository(&crate::store::RepositoryRecord { + workspace_id: WORKSPACE.into(), + repository_id: repository.id.clone(), + repository_key: repository.repository_key.clone(), + kind: "git".into(), + provider: Some(repository.provider.clone()), + source: repository.source.clone(), + default_ref: repository.default_selector.clone(), + source_fingerprint: repository.source_fingerprint.clone(), + observed_status: repository.observed_status.clone(), + observed_at: repository.observed_at.clone(), + created_at: config.workspace_created_at.clone(), + updated_at: config.workspace_created_at.clone(), + }) + .unwrap(); + } store .upsert_user(&UserRecord { user_id: "fixture-user".into(), @@ -924,6 +948,12 @@ struct HeldRuntimeReply { } impl HeldRuntimeReply { fn new() -> Self { + Self::with_held_path(None) + } + fn for_path(path: &str) -> Self { + Self::with_held_path(Some(path.to_owned())) + } + fn with_held_path(path: Option) -> Self { use std::io::{Read, Write}; let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let address = listener.local_addr().unwrap(); @@ -932,25 +962,33 @@ impl HeldRuntimeReply { let upstream_released = Arc::new(AtomicBool::new(false)); let flag = upstream_released.clone(); let thread = std::thread::spawn(move || { - let Ok((mut stream, _)) = listener.accept() else { - return; - }; - stream.set_read_timeout(Some(WATCHDOG)).unwrap(); - let mut buf = [0; 4096]; - let Ok(n) = stream.read(&mut buf) else { return }; - if n == 0 { - return; - } - let _ = entered.send( - String::from_utf8_lossy(&buf[..n]) + loop { + let Ok((mut stream, _)) = listener.accept() else { + return; + }; + stream.set_read_timeout(Some(WATCHDOG)).unwrap(); + let mut buf = [0; 4096]; + let Ok(n) = stream.read(&mut buf) else { return }; + if n == 0 { + return; + } + let request = String::from_utf8_lossy(&buf[..n]) .lines() .next() .unwrap() - .to_owned(), - ); - let _ = permit.recv_timeout(WATCHDOG); - flag.store(true, Ordering::SeqCst); - let _ = stream.write_all(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + .to_owned(); + if path.as_ref().is_some_and(|path| !request.contains(path)) { + // Ready's best-effort notification lookup precedes its detail + // source observation. Deny it promptly; hold only the named stage. + let _ = stream.write_all(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + continue; + } + let _ = entered.send(request); + let _ = permit.recv_timeout(WATCHDOG); + flag.store(true, Ordering::SeqCst); + let _ = stream.write_all(b"HTTP/1.1 503 Service Unavailable\r\nContent-Length: 0\r\nConnection: close\r\n\r\n"); + break; + } }); Self { address, diff --git a/crates/workspace-server/src/server_residual_execution_tests.rs b/crates/workspace-server/src/server_residual_execution_tests.rs new file mode 100644 index 000000000..8b0e3620a --- /dev/null +++ b/crates/workspace-server/src/server_residual_execution_tests.rs @@ -0,0 +1,843 @@ +use super::*; + +fn registry_record(worker: RuntimeWorkerRef) -> WorkerRegistryRecord { + WorkerRegistryRecord { + workspace_id: WORKSPACE.into(), + worker, + display_name: "current stored label".into(), + profile: Some("builtin:coder".into()), + retention_state: "normal".into(), + transcript_ref: None, + session_ref: None, + summary_ref: None, + diagnostics_ref: None, + created_at: "2026-01-01T00:00:00Z".into(), + updated_at: "2026-01-01T00:00:00Z".into(), + } +} + +fn assigned_ticket(fixture: &Fixture) -> String { + let record = registry_record(RuntimeWorkerRef::new( + EMBEDDED_WORKER_RUNTIME_ID, + Uuid::now_v7().to_string(), + )); + fixture.store.upsert_worker_registry(&record).unwrap(); + let mut input = ticket::NewTicket::new("cancel this implementation"); + input.workflow_state = Some(TicketState::Open); + let ticket = browser_ticket_backend(&fixture.api) + .unwrap() + .create(input) + .unwrap(); + fixture + .store + .set_current_ticket_worker_assignment( + &TicketWorkerAssignmentRecord { + workspace_id: WORKSPACE.into(), + ticket_id: ticket.id.clone(), + assignment_id: "current-coder".into(), + worker: record.worker, + assigned_by: "fixture-user".into(), + assigned_at: "2026-01-01T00:00:00Z".into(), + }, + None, + "assign-event", + "assign-operation", + false, + ) + .unwrap(); + ticket.id +} + +async fn fixture() -> Fixture { + let fixture = Fixture::new().await; + fixture.api.worker_projection.shutdown_async().await; + observed(fixture.executor.drain()).await.unwrap(); + fixture +} + +fn cancel_request() -> serde_json::Value { + serde_json::json!({ + "operation_id": "cancel-operation", "assignment_id": "current-coder", "reason": "use the corrected design" + }) +} + +#[tokio::test(flavor = "current_thread")] +async fn cancelled_ticket_implementation_keeps_assignment_commit_owned() { + let fixture = fixture().await; + let ticket = assigned_ticket(&fixture); + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let mut response = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + fixture.inner_router().oneshot(Fixture::request( + "POST", + &format!("/api/w/{WORKSPACE}/tickets/{ticket}/implementation-cancellations"), + cancel_request(), + )), + )); + assert!( + futures::poll!(&mut response).is_pending(), + "cancellation must admit an owned stage, not run SQL on Tokio" + ); + observed(waiting).await.unwrap(); // Actual cancellation transaction, after Runtime/session cleanup. + runtime_progress_while_locked(&lock).await; + drop(response); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "actual mixed cancellation must keep its authority after the waiter drops" + ); + lock.release(); + busy.resume(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + assert!( + fixture + .store + .get_active_ticket_worker_assignment(WORKSPACE, &ticket) + .unwrap() + .is_none() + ); + assert!( + fixture + .store + .get_ticket_assignment_operation(WORKSPACE, "cancel-operation") + .unwrap() + .is_some() + ); + let detail = browser_ticket_detail(&fixture.api, &ticket).unwrap().0; + assert!(detail.current_worker.is_none()); + assert_eq!(detail.state.as_str(), TicketState::Open.as_str()); +} + +async fn saturated_host(fixture: &Fixture) -> Vec> { + let mut releases = Vec::new(); + let (started, running) = oneshot::channel(); + let (release, permit) = mpsc::channel::<()>(); + releases.push(release); + fixture + .host_executor + .dispatch(move || { + let _ = started.send(()); + let _ = permit.recv_timeout(WATCHDOG); + }) + .unwrap(); + observed(running).await.unwrap(); + let (release, permit) = mpsc::channel::<()>(); + releases.push(release); + fixture + .host_executor + .dispatch(move || { + let _ = permit.recv_timeout(WATCHDOG); + }) + .unwrap(); + releases +} + +#[tokio::test(flavor = "current_thread")] +async fn cancellation_host_overload_leaves_the_coder_assignment_unchanged() { + let fixture = fixture().await; + let ticket = assigned_ticket(&fixture); + let before = fixture + .store + .get_active_ticket_worker_assignment(WORKSPACE, &ticket) + .unwrap() + .unwrap(); + let releases = saturated_host(&fixture).await; + let body = response_json( + observed(fixture.inner_router().oneshot(Fixture::request( + "POST", + &format!("/api/w/{WORKSPACE}/tickets/{ticket}/implementation-cancellations"), + cancel_request(), + ))) + .await + .unwrap(), + StatusCode::SERVICE_UNAVAILABLE, + ) + .await; + assert!(body.to_string().contains("db_capacity_exceeded"), "{body}"); + assert_eq!( + fixture + .store + .get_active_ticket_worker_assignment(WORKSPACE, &ticket) + .unwrap() + .unwrap(), + before + ); + assert!( + fixture + .store + .get_ticket_assignment_operation(WORKSPACE, "cancel-operation") + .unwrap() + .is_none() + ); + drop(releases); + observed(fixture.host_executor.drain()).await.unwrap(); +} + +fn seed_unrepaired_mr(fixture: &Fixture) { + let ticket = browser_ticket_backend(&fixture.api) + .unwrap() + .create(ticket::NewTicket::new("repair source selector")) + .unwrap(); + fixture + .store + .upsert_repository(&crate::store::RepositoryRecord { + workspace_id: WORKSPACE.into(), + repository_id: "fixture-repo".into(), + repository_key: "fixture-repo".into(), + kind: "git".into(), + provider: Some("git".into()), + source: server_api::RepositorySource { + kind: server_api::RepositorySourceKind::LocalPath, + uri: fixture + ._dir + .path() + .join("unused-repository") + .to_string_lossy() + .into_owned(), + }, + default_ref: Some("develop".into()), + source_fingerprint: "sha256:fixture".into(), + observed_status: server_api::RepositoryObservedStatus::Unverified, + observed_at: None, + created_at: "2026-01-01T00:00:00Z".into(), + updated_at: "2026-01-01T00:00:00Z".into(), + }) + .unwrap(); + // Persist the external unrepaired-selector shape. No active assignment: + // the real handler must reach its assignment read, then reject without provider dispatch. + let conn = Connection::open(&fixture.api.config.database_path).unwrap(); + conn.execute("INSERT INTO merge_requests VALUES (?1,'repair-fixture','fixture-repo','open',NULL,'develop',?2,?2)", rusqlite::params![WORKSPACE, "2026-01-01T00:00:00Z"]).unwrap(); + conn.execute("INSERT INTO merge_request_ticket_relations VALUES (?1,'repair-fixture',?2,'implements',?3)", rusqlite::params![WORKSPACE, ticket.id, "2026-01-01T00:00:00Z"]).unwrap(); +} + +fn repair_headers() -> HeaderMap { + let mut headers = HeaderMap::new(); + headers.insert( + "cookie", + "db_execution_session=fixture-browser-token" + .parse() + .unwrap(), + ); + headers +} + +fn repair_input() -> Json { + Json(server_api::RepairMergeRequestSelectorRequest { + selector_from: "work/fixed-selector".into(), + reason: "repair the imported selector".into(), + explicit_confirmation: true, + }) +} + +#[tokio::test(flavor = "current_thread")] +async fn authenticated_selector_repair_waits_off_tokio_and_keeps_cancelled_host_ownership() { + let fixture = fixture().await; + seed_unrepaired_mr(&fixture); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let mut repair = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + scoped_repair_merge_request_selector( + State(fixture.api.clone()), + repair_headers(), + AxumPath((WORKSPACE.into(), "repair-fixture".into())), + repair_input(), + ), + )); + assert!(futures::poll!(&mut repair).is_pending()); // Browser authentication DB stage only. + observed(fixture.executor.drain()).await.unwrap(); + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let writer = fixture.store.clone(); + fixture + .executor + .dispatch(move || { + writer + .mark_api_token_used("missing", "2026-01-01T00:00:00Z") + .unwrap(); + }) + .unwrap(); + observed(waiting).await.unwrap(); + assert!( + futures::poll!(&mut repair).is_pending(), + "post-auth selector repair must not wait on the connection mutex on Tokio" + ); + runtime_progress_while_locked(&lock).await; + drop(repair); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "actual repair stage must retain authority after cancellation" + ); + lock.release(); + busy.resume(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + let mr = merge_request_store(&fixture.api, WORKSPACE) + .unwrap() + .get_by_id(WORKSPACE, "repair-fixture") + .unwrap(); + assert!(mr.selector_from.is_none()); + assert!( + merge_request_store(&fixture.api, WORKSPACE) + .unwrap() + .thread_page_by_id(WORKSPACE, "repair-fixture", None, 10) + .unwrap() + .is_empty() + ); +} + +#[tokio::test(flavor = "current_thread")] +async fn selector_repair_host_overload_preserves_confirmation_and_no_side_effects() { + let fixture = fixture().await; + seed_unrepaired_mr(&fixture); + let releases = saturated_host(&fixture).await; + let error = scoped_repair_merge_request_selector( + State(fixture.api.clone()), + repair_headers(), + AxumPath((WORKSPACE.into(), "repair-fixture".into())), + repair_input(), + ) + .await + .unwrap_err(); + let body = response_json(error.into_response(), StatusCode::SERVICE_UNAVAILABLE).await; + assert!(body.to_string().contains("db_capacity_exceeded"), "{body}"); + let mut input = repair_input(); + input.0.explicit_confirmation = false; + let error = scoped_repair_merge_request_selector( + State(fixture.api.clone()), + repair_headers(), + AxumPath((WORKSPACE.into(), "repair-fixture".into())), + input, + ) + .await + .unwrap_err(); + assert!( + !matches!(error.error, Error::DbCapacityExceeded), + "browser confirmation denial must precede Host admission" + ); + let mr = merge_request_store(&fixture.api, WORKSPACE) + .unwrap() + .get_by_id(WORKSPACE, "repair-fixture") + .unwrap(); + assert!(mr.selector_from.is_none()); + drop(releases); + observed(fixture.host_executor.drain()).await.unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn post_restore_projection_waits_off_tokio_and_preserves_historical_results() { + let fixture = fixture().await; + let record = registry_record(RuntimeWorkerRef::new( + EMBEDDED_WORKER_RUNTIME_ID, + Uuid::now_v7().to_string(), + )); + fixture.store.upsert_worker_registry(&record).unwrap(); + let mut historical = worker_summary_from_registry(&record); + historical.label = "historical label".into(); + historical.state = "paused".into(); + let result = || crate::hosts::InternalWorkerRestoreResult { + state: server_api::WorkerRestoreState::Rejected, + worker: Some(historical.clone()), + diagnostics: Vec::new(), + }; + let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let writer = fixture.store.clone(); + fixture + .executor + .dispatch(move || { + writer + .mark_api_token_used("missing", "2026-01-01T00:00:00Z") + .unwrap(); + }) + .unwrap(); + observed(waiting).await.unwrap(); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let mut projection = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + project_worker_restore_response_owned( + fixture.api.clone(), + record.worker.runtime_id.clone(), + record.worker.worker_id.clone(), + result(), + ), + )); + assert!( + futures::poll!(&mut projection).is_pending(), + "stored projection must await a whole DB stage after Host restore returns" + ); + runtime_progress_while_locked(&lock).await; + assert_eq!( + observed(fixture.host_executor.run(|| Ok::<_, Error>("control"))) + .await + .unwrap(), + "control" + ); + let error = project_worker_restore_response_owned( + fixture.api.clone(), + record.worker.runtime_id.clone(), + record.worker.worker_id.clone(), + result(), + ) + .await + .unwrap_err(); + response_json(error.into_response(), StatusCode::SERVICE_UNAVAILABLE).await; + let Json(no_worker) = project_worker_restore_response_owned( + fixture.api.clone(), + record.worker.runtime_id.clone(), + record.worker.worker_id.clone(), + crate::hosts::InternalWorkerRestoreResult { + state: server_api::WorkerRestoreState::Rejected, + worker: None, + diagnostics: Vec::new(), + }, + ) + .await + .unwrap(); + assert_eq!( + no_worker.result.state, + server_api::WorkerRestoreState::Rejected + ); + drop(projection); + assert_eq!(leases.load(Ordering::SeqCst), 1); + lock.release(); + busy.resume(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + let Json(response) = project_worker_restore_response_owned( + fixture.api.clone(), + record.worker.runtime_id.clone(), + record.worker.worker_id.clone(), + result(), + ) + .await + .unwrap(); + assert_eq!( + response.result.state, + server_api::WorkerRestoreState::Rejected + ); + let worker = response.result.worker.unwrap(); + assert_eq!(worker.label, record.display_name); + assert_eq!( + worker.state, "paused", + "a historical receipt is not a new Runtime observation" + ); + assert_eq!( + fixture + .store + .get_worker_registry(WORKSPACE, &record.worker) + .unwrap() + .unwrap(), + record, + "response projection must not write a new observation" + ); + observed(fixture.executor.drain()).await.unwrap(); +} + +#[tokio::test(flavor = "current_thread")] +async fn ticket_source_ref_reads_and_ready_publication_leave_sql_and_tokio_free() { + for operation in ["detail", "query", "ready"] { + let repository_dir = tempfile::tempdir().unwrap(); + let source = server_api::RepositorySource { + kind: server_api::RepositorySourceKind::LocalPath, + uri: repository_dir.path().to_string_lossy().into_owned(), + }; + let fixture = Fixture::with_repositories(vec![ConfiguredRepository { + id: "fixture-repo".into(), + repository_key: "fixture-repo".into(), + provider: "git".into(), + source_fingerprint: crate::repository_source::repository_source_fingerprint(&source), + source, + observed_status: server_api::RepositoryObservedStatus::Unverified, + observed_at: None, + path: Some(repository_dir.path().into()), + default_selector: Some("develop".into()), + }]) + .await; + fixture.api.worker_projection.shutdown_async().await; + observed(fixture.executor.drain()).await.unwrap(); + let mut reply = HeldRuntimeReply::for_path("/v1/repository-refs/observe"); + // Narrow Runtime HTTP/provider placement fixture, not a signed authority + // or complete Git/Runtime E2E test. Existing signed routes test that fence. + install_remote_fixture( + &fixture, + "ref-runtime", + &format!("http://{}", reply.address), + false, + ); + let worker = RuntimeWorkerRef::new("ref-runtime", Uuid::now_v7().to_string()); + fixture + .store + .upsert_worker_registry(®istry_record(worker.clone())) + .unwrap(); + let mut input = ticket::NewTicket::new("source ref projection"); + input.workflow_state = Some(TicketState::Open); + let ticket = browser_ticket_backend(&fixture.api) + .unwrap() + .create(input) + .unwrap(); + fixture + .store + .set_current_ticket_worker_assignment( + &TicketWorkerAssignmentRecord { + workspace_id: WORKSPACE.into(), + ticket_id: ticket.id.clone(), + assignment_id: "ref-assignment".into(), + worker: worker.clone(), + assigned_by: "fixture-user".into(), + assigned_at: "2026-01-01T00:00:00Z".into(), + }, + None, + "ref-event", + "ref-operation", + false, + ) + .unwrap(); + merge_request_store(&fixture.api, WORKSPACE) + .unwrap() + .open_merge_request(merge_request::OpenMergeRequest { + merge_request_id: "ref-mr".into(), + ticket_id: ticket.id.clone(), + repository_id: "fixture-repo".into(), + selector_from: "work/source".into(), + selector_to: "develop".into(), + summary: String::new(), + auth: merge_request::MergeRequestAuth { + workspace_id: WORKSPACE.into(), + repository_id: "fixture-repo".into(), + runtime_id: worker.runtime_id, + worker_id: worker.worker_id, + assignment_id: "ref-assignment".into(), + }, + now: Utc::now(), + }) + .unwrap(); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let api = fixture.api.clone(); + let ticket_id = ticket.id.clone(); + let mut response = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + async move { + match operation { + "detail" => scoped_get_ticket( + State(api), + AxumPath(ScopedRecordPath { + workspace_id: WORKSPACE.into(), + id: ticket_id, + }), + ) + .await + .map(|_| ()), + "query" => scoped_query_tickets( + State(api), + AxumPath(ScopedWorkspacePath { + workspace_id: WORKSPACE.into(), + }), + Json(TicketQueryRequest::default()), + ) + .await + .map(|_| ()), + _ => scoped_mark_ticket_ready_from_browser( + State(api), + HeaderMap::new(), + AxumPath(ScopedRecordPath { + workspace_id: WORKSPACE.into(), + id: ticket_id, + }), + Json(InternalTicketMarkReadyRequest { + operation_key: "ready-operation".into(), + reason: None, + intake_summary: None, + }), + ) + .await + .map(|_| ()), + } + }, + )); + assert!(futures::poll!(&mut response).is_pending()); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!( + request.contains("/v1/repository-refs/observe"), + "{operation}: {request}" + ); + let store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; + assert!( + !reply.upstream_released.load(Ordering::SeqCst), + "watchdog release is not provider progress" + ); + if operation == "ready" { + let saved = browser_ticket_backend(&fixture.api) + .unwrap() + .show(TicketIdOrSlug::Id(ticket.id)) + .unwrap(); + assert_eq!( + saved.meta.readiness.as_deref(), + Some("ready"), + "Ready mutation must be durable before the held detail source observation" + ); + assert_eq!( + saved + .events + .iter() + .filter( + |event| event.attributes.get("operation_key").map(String::as_str) + == Some("ready-operation") + ) + .count(), + 1 + ); + assert_eq!( + saved.meta.workflow_state, + TicketState::Open, + "Ready must commit before detail publication" + ); + } + drop(response); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "{operation}: cancelled provider stage lost actual ownership" + ); + fixture.host_executor.dispatch(|| {}).unwrap(); // Remaining finite queued slot. + let error = scoped_get_ticket( + State(fixture.api.clone()), + AxumPath(ScopedRecordPath { + workspace_id: WORKSPACE.into(), + id: "unused".into(), + }), + ) + .await + .unwrap_err(); + response_json(error.into_response(), StatusCode::SERVICE_UNAVAILABLE).await; + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + } +} + +#[tokio::test(flavor = "current_thread")] +async fn worker_control_source_lookup_waits_off_tokio_and_retains_finite_host_ownership() { + let fixture = fixture().await; + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "source-runtime", + &format!("http://{}", reply.address), + false, + ); + let source = RuntimeWorkerRef::new("source-runtime", Uuid::now_v7().to_string()); + let mut headers = HeaderMap::new(); + headers.insert("x-yoi-runtime-id", source.runtime_id.parse().unwrap()); + headers.insert("x-yoi-worker-id", source.worker_id.parse().unwrap()); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let request = || { + Json(server_api::RuntimeWorkerLifecycleRequest { + reason: None, + ticket_assignment: None, + }) + }; + let path = || { + AxumPath(ScopedRuntimeWorkerPath { + workspace_id: WORKSPACE.into(), + worker: source.clone(), + }) + }; + let mut control = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + cancel_known_worker( + State(fixture.api.clone()), + path(), + headers.clone(), + request(), + ), + )); + assert!( + futures::poll!(&mut control).is_pending(), + "control source lookup must not synchronously join Runtime HTTP on Tokio" + ); + let request_line = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!(request_line.contains("/v1/workers/"), "{request_line}"); + let store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; + assert!(!reply.upstream_released.load(Ordering::SeqCst)); + drop(control); + assert_eq!(leases.load(Ordering::SeqCst), 1); + fixture.host_executor.dispatch(|| {}).unwrap(); + let error = cancel_known_worker(State(fixture.api.clone()), path(), headers, request()) + .await + .unwrap_err(); + let body = response_json(error.into_response(), StatusCode::SERVICE_UNAVAILABLE).await; + assert!( + body.to_string().contains("db_capacity_exceeded"), + "Host overload is availability, not identity denial: {body}" + ); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); +} + +#[tokio::test(flavor = "current_thread")] +async fn ticket_tracking_and_case_authority_runtime_lookup_leave_db_and_tokio_free() { + for operation in ["tracking", "case"] { + let fixture = fixture().await; + let ticket = browser_ticket_backend(&fixture.api) + .unwrap() + .create(ticket::NewTicket::new("decision authority")) + .unwrap(); + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "orchestrator-runtime", + &format!("http://{}", reply.address), + false, + ); + let memory = fixture + .store + .get_workspace_memory_settings(WORKSPACE) + .unwrap(); + let reservation = fixture + .store + .reserve_worker_create( + WORKSPACE, + "orchestrator-runtime", + "orch-create", + "orch-create-fingerprint", + Some(crate::hosts::WORKSPACE_ORCHESTRATOR_SINGLETON_KEY), + &memory, + ) + .unwrap(); + let source = + RuntimeWorkerRef::new("orchestrator-runtime", reservation.worker_id.to_string()); + fixture + .store + .upsert_worker_registry(®istry_record(source.clone())) + .unwrap(); + fixture + .store + .complete_worker_create_reservation(WORKSPACE, reservation.worker_id) + .unwrap(); + let context = server_api::ServerRequestContext { + actor: None, + worker_source: None, + runtime_source: Some(server_api::ServerRuntimeSource { + authentication_binding_id: "verified-fixture-binding".into(), + runtime_id: source.runtime_id.clone(), + worker_id: Some(source.worker_id.clone()), + }), + origin: None, + transport_headers: Vec::new(), + }; + let backend = browser_ticket_backend(&fixture.api).unwrap(); + let before = backend.show(TicketIdOrSlug::Id(ticket.id.clone())).unwrap(); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let api = fixture.api.clone(); + let ticket_id = ticket.id.clone(); + let mut decision = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + async { + if operation == "tracking" { + scoped_ticket_assignment_tracking_update_with_context( + State(api), + HeaderMap::new(), + &context, + AxumPath((WORKSPACE.into(), ticket_id)), + Json(ticket::TicketAssignmentTrackingUpdate { + assignment_id: "no-delegation".into(), + expected_last_event_id: "baseline".into(), + operation_key: "track-operation".into(), + tracking: ticket::TicketAssignmentTracking { + status: ticket::TicketAssignmentTrackingStatus::Inprogress, + waiting_reason: None, + progress: None, + next_action: None, + result: None, + }, + }), + ) + .await + .map(|_| ()) + } else { + apply_ticket_case_update( + State(api), + Some(source), + None, + AxumPath((WORKSPACE.into(), ticket_id)), + Json(ticket::TicketCaseUpdate { + expected_content_digest: ticket::ticket_content_digest(&before), + expected_state: before.meta.workflow_state, + expected_closure_operation_key: None, + operation_key: "case-operation".into(), + action: ticket::TicketCaseAction::Close { + reason: ticket::TicketClosureReason::Completed, + }, + explanation: "check current authority".into(), + references: Vec::new(), + author: None, + }), + ) + .await + .map(|_| ()) + } + }, + )); + assert!(futures::poll!(&mut decision).is_pending()); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!(request.contains("/v1/workers/"), "{operation}: {request}"); + let store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; + assert!(!reply.upstream_released.load(Ordering::SeqCst)); + drop(decision); + assert_eq!(leases.load(Ordering::SeqCst), 1); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + assert_eq!( + backend.show(TicketIdOrSlug::Id(ticket.id)).unwrap(), + before, + "failed live authority must not make a decision" + ); + } +} From 4d9811944065b3a273f8b7582b2138ce5566a221 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 15:07:27 +0900 Subject: [PATCH 13/14] fix(server): bound embedded protocol and Feature shutdown ownership --- crates/workspace-server/src/server.rs | 138 ++++++++---- .../tests/embedded_owned_execution_tests.rs | 197 ++++++++++++++++++ .../tests/protocol_backpressure_tests.rs | 23 +- .../src/server_residual_execution_tests.rs | 61 ++++++ 4 files changed, 367 insertions(+), 52 deletions(-) create mode 100644 crates/workspace-server/src/server/tests/embedded_owned_execution_tests.rs diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 4d75d5985..1a5593d6d 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -2651,16 +2651,7 @@ impl WorkspaceServerApi { async fn shutdown_feature_storage(&self) -> Result<()> { let apis = self.apis.lock().await.values().cloned().collect::>(); for api in apis { - api.worker_projection.shutdown_async().await; - let storage = api.feature_storage.clone(); - api.store - .db_executor() - .run(move || { - storage.shutdown().map_err(|error| { - Error::Store(format!("failed to shut down Feature storage: {error}")) - }) - }) - .await?; + shutdown_workspace_feature_storage(&api).await?; } Ok(()) } @@ -12271,15 +12262,28 @@ pub async fn serve( let orchestrator_hook = tokio::spawn(run_orchestrator_turn_end_hook(api.clone())); let result = axum::serve(listener, build_router(api.clone())).await; orchestrator_hook.abort(); - let storage_result = api - .feature_storage - .shutdown() - .map_err(|error| Error::Store(format!("failed to shut down Feature storage: {error}"))); + let _ = orchestrator_hook.await; + let storage_result = shutdown_workspace_feature_storage(&api).await; result?; storage_result?; Ok(()) } +// Feature shutdown fences new operations and waits for synchronous connection +// holders. Accepted close retains ownership after its async waiter disappears. +async fn shutdown_workspace_feature_storage(api: &WorkspaceApi) -> Result<()> { + api.worker_projection.shutdown_async().await; + let storage = api.feature_storage.clone(); + api.store + .db_executor() + .run(move || { + storage.shutdown().map_err(|error| { + Error::Store(format!("failed to shut down Feature storage: {error}")) + }) + }) + .await +} + #[derive(Debug, Serialize, Deserialize)] pub struct ProjectListResponse { pub workspace_id: String, @@ -31749,7 +31753,7 @@ pub(crate) async fn connect_workspace_worker_protocol( RuntimeObservationSource::Embedded(source) => { let scope = worker_runtime::RuntimeWorkspaceScope::new(api.workspace_id(), "embedded-backend"); - connect_embedded_worker_protocol(source, scope).await + connect_embedded_worker_protocol(source, scope, api.host_executors.host()?).await } } } @@ -31845,18 +31849,25 @@ async fn connect_remote_worker_protocol( async fn connect_embedded_worker_protocol( source: crate::observation::EmbeddedRuntimeObservationSource, scope: worker_runtime::RuntimeWorkspaceScope, + executor: crate::db_executor::DbExecutor, ) -> Result { - // Capture one real execution endpoint, including its greeting/snapshot. - // The WorkerRef-wide Workspace observation bus is a separate catalogue - // channel and must not reconnect this operational session after Restore. - let mut transport = source - .runtime - .attach_worker_protocol_scoped(&scope, &source.worker_ref) - .map_err(|error| Error::RuntimeOperationFailed { - runtime_id: source.worker.runtime_id.clone(), - code: "embedded_worker_protocol_connect_failed".to_string(), - message: error.to_string(), - })?; + // Endpoint capture takes Runtime operation/state locks. Keep it, and each + // intact method (including fs-store commit/publication), off Tokio and the + // sole DB lane. A discarded reply retains the endpoint until cleanup. + let capture_source = source.clone(); + let capture_scope = scope.clone(); + let mut transport = executor + .run(move || { + capture_source + .runtime + .attach_worker_protocol_scoped(&capture_scope, &capture_source.worker_ref) + .map_err(|error| Error::RuntimeOperationFailed { + runtime_id: capture_source.worker.runtime_id.clone(), + code: "embedded_worker_protocol_connect_failed".to_string(), + message: error.to_string(), + }) + }) + .await?; Ok(spawn_worker_protocol_pump( move |mut methods, events| async move { if let Err(reason) = enqueue_worker_protocol_event(&events, transport.snapshot.clone()) @@ -31868,9 +31879,26 @@ async fn connect_embedded_worker_protocol( method = methods.recv() => { let Some(method) = method else { return WorkerProtocolEnd::Disconnected; }; let shutdown = matches!(&method, protocol::Method::Shutdown { .. }); - match source.runtime.send_connected_protocol_method_scoped( - &scope, &source.worker_ref, &transport, method, - ) { + let method_source = source.clone(); + let method_scope = scope.clone(); + let owned = executor.run(move || { + let result = method_source.runtime.send_connected_protocol_method_scoped( + &method_scope, &method_source.worker_ref, &transport, method, + ); + // Endpoint validation is synchronous too. + let terminal = result.as_ref().err().is_some_and(|error| + embedded_protocol_dispatch_is_terminal(error, &transport)); + Ok::<_, Error>((transport, result, terminal)) + }).await; + let (returned, result, terminal) = match owned { + Ok(value) => value, + Err(error) => { + let _ = enqueue_worker_protocol_event(&events, protocol_error_event(error.to_string())); + return WorkerProtocolEnd::Disconnected; + } + }; + transport = returned; + match result { Ok(direct_events) => { for event in direct_events { let terminal = matches!(event, protocol::Event::Shutdown); @@ -31879,7 +31907,6 @@ async fn connect_embedded_worker_protocol( } } Err(error) => { - let terminal = embedded_protocol_dispatch_is_terminal(&error, &transport); if let Err(reason) = enqueue_worker_protocol_event(&events, protocol_error_event(error.to_string())) { return reason; } if terminal { return WorkerProtocolEnd::Disconnected; } } @@ -31892,7 +31919,6 @@ async fn connect_embedded_worker_protocol( if let Err(reason) = enqueue_worker_protocol_event(&events, event) { return reason; } if terminal { return WorkerProtocolEnd::Disconnected; } } - // Stop, disconnect and upstream lag never cause an implicit reattach. None => return WorkerProtocolEnd::Disconnected, } } @@ -35184,6 +35210,7 @@ mod tests { mod drive_api_tests; mod drive_tests; mod drive_web_roundtrip_tests; + mod embedded_owned_execution_tests; mod external_checkout_tests; mod protocol_backpressure_tests; mod subject_spawn_tests; @@ -45338,6 +45365,7 @@ mod tests { // The receiver belongs to one mock execution, not to a Runtime WorkerRecord. // Bound protocol senders become terminal when Stop drops this execution. struct DeterministicWorkerExecution { + dispatch_hook: std::sync::Mutex>>, context: worker_runtime::execution::WorkerExecutionContext, state: std::sync::Mutex, method_tx: tokio::sync::mpsc::Sender, @@ -45357,6 +45385,7 @@ mod tests { let (method_tx, method_rx) = tokio::sync::mpsc::channel(32); let (events, _) = tokio::sync::broadcast::channel(32); Self { + dispatch_hook: std::sync::Mutex::new(None), context, state: std::sync::Mutex::new(protocol::WorkerStateSnapshot::initial()), method_tx, @@ -45380,6 +45409,9 @@ mod tests { Vec, worker_runtime::execution::WorkerExecutionResult, > { + if let Some(hook) = self.dispatch_hook.lock().unwrap().take() { + hook(); + } // Drive the actual mock receiver synchronously; no method is routed // through a WorkerRef lookup after a protocol connection is attached. let mut receiver = self.method_rx.lock().unwrap(); @@ -61492,10 +61524,13 @@ mod tests { for delayed in delayed_protocol_test_methods() { let (runtime, worker_ref, execution) = runtime_with_worker_backend(); let source = embedded_protocol_test_source(&runtime, &worker_ref); - let mut old = - connect_embedded_worker_protocol(source.clone(), embedded_protocol_test_scope()) - .await - .unwrap(); + let mut old = connect_embedded_worker_protocol( + source.clone(), + embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), + ) + .await + .unwrap(); assert_embedded_protocol_snapshot( next_embedded_protocol_event(&mut old).await, &worker_ref, @@ -61559,10 +61594,13 @@ mod tests { &[worker_ref.clone()] ); - let mut current = - connect_embedded_worker_protocol(source, embedded_protocol_test_scope()) - .await - .unwrap(); + let mut current = connect_embedded_worker_protocol( + source, + embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), + ) + .await + .unwrap(); assert_embedded_protocol_snapshot( next_embedded_protocol_event(&mut current).await, &worker_ref, @@ -61710,10 +61748,13 @@ mod tests { for lagged in [false, true] { let (runtime, worker_ref, execution) = runtime_with_worker_backend(); let source = embedded_protocol_test_source(&runtime, &worker_ref); - let mut bridge = - connect_embedded_worker_protocol(source.clone(), embedded_protocol_test_scope()) - .await - .unwrap(); + let mut bridge = connect_embedded_worker_protocol( + source.clone(), + embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), + ) + .await + .unwrap(); assert_embedded_protocol_snapshot( next_embedded_protocol_event(&mut bridge).await, &worker_ref, @@ -61755,6 +61796,7 @@ mod tests { let mut bridge = connect_embedded_worker_protocol( embedded_protocol_test_source(&runtime, &worker_ref), embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), ) .await .unwrap(); @@ -61815,9 +61857,13 @@ mod tests { let wrong_scope = worker_runtime::RuntimeWorkspaceScope::new("other-workspace", "local-token"); assert!( - connect_embedded_worker_protocol(source.clone(), wrong_scope.clone()) - .await - .is_err() + connect_embedded_worker_protocol( + source.clone(), + wrong_scope.clone(), + HostExecutors::default().host().unwrap() + ) + .await + .is_err() ); let (mut socket, server) = spawn_embedded_protocol_test_ws(source.clone(), wrong_scope, false).await; diff --git a/crates/workspace-server/src/server/tests/embedded_owned_execution_tests.rs b/crates/workspace-server/src/server/tests/embedded_owned_execution_tests.rs new file mode 100644 index 000000000..55444a031 --- /dev/null +++ b/crates/workspace-server/src/server/tests/embedded_owned_execution_tests.rs @@ -0,0 +1,197 @@ +use super::*; +use crate::db_executor::{ + DbExecutor, DbExecutorLimits, DbOperationLifetime, with_operation_lifetime, +}; +use std::{ + num::NonZeroUsize, + sync::{ + atomic::{AtomicUsize, Ordering}, + mpsc, + }, + time::Duration, +}; + +fn owner() -> DbExecutor { + DbExecutor::new(DbExecutorLimits { + workers: NonZeroUsize::new(1).unwrap(), + queued: NonZeroUsize::new(1).unwrap(), + }) + .unwrap() +} +async fn observed(future: impl std::future::Future) -> T { + tokio::time::timeout(Duration::from_secs(2), future) + .await + .expect("owned protocol stage did not progress") +} +struct Release(Option>); +impl Release { + fn release(&mut self) { + if let Some(tx) = self.0.take() { + let _ = tx.send(()); + } + } +} +impl Drop for Release { + fn drop(&mut self) { + self.release(); + } +} +struct Lifetime(Arc); +struct Retained(Arc); +impl Drop for Retained { + fn drop(&mut self) { + self.0.fetch_sub(1, Ordering::SeqCst); + } +} +impl DbOperationLifetime for Lifetime { + fn retain(&self) -> Box { + self.0.fetch_add(1, Ordering::SeqCst); + Box::new(Retained(self.0.clone())) + } +} + +#[tokio::test(flavor = "current_thread")] +async fn cancelled_embedded_endpoint_capture_keeps_actual_host_budget_off_tokio() { + let (runtime, worker, execution) = runtime_with_worker_backend(); + let (started, held) = tokio::sync::oneshot::channel(); + let (tx, rx) = mpsc::channel(); + let mut release = Release(Some(tx)); + let thread = std::thread::spawn(move || { + let _guard = execution.contexts.lock().unwrap(); + let _ = started.send(()); + let _ = rx.recv_timeout(Duration::from_secs(5)); + }); + observed(held).await.unwrap(); + let executor = owner(); + let leases = Arc::new(AtomicUsize::new(0)); + let mut capture = Box::pin(with_operation_lifetime( + Arc::new(Lifetime(leases.clone())), + connect_embedded_worker_protocol( + embedded_protocol_test_source(&runtime, &worker), + embedded_protocol_test_scope(), + executor.clone(), + ), + )); + assert!( + futures::poll!(&mut capture).is_pending(), + "endpoint Runtime lock must not run on Tokio" + ); + observed(tokio::time::sleep(Duration::from_millis(1))).await; + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + drop(capture); + assert_eq!(leases.load(Ordering::SeqCst), 1); + executor.dispatch(|| {}).unwrap(); // only the one queued slot is left + let error = executor.run(|| Ok::<_, Error>(())).await.unwrap_err(); + assert!(matches!(error, Error::DbCapacityExceeded)); + release.release(); + thread.join().unwrap(); + observed(executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); +} + +#[tokio::test(flavor = "current_thread")] +async fn cancelled_embedded_shutdown_commits_fs_receipt_after_the_pump_disappears() { + let dir = tempfile::tempdir().unwrap(); + let execution = Arc::new(DeterministicExecutionBackend::default()); + let options = worker_runtime::fs_store::FsRuntimeStoreOptions { + root: dir.path().join("runtime"), + runtime_id: "owned-protocol".into(), + display_name: None, + }; + let runtime = worker_runtime::Runtime::with_fs_store_and_execution_backend( + options.clone(), + execution.clone(), + ) + .unwrap(); + runtime.store_config_bundle(runtime_test_bundle()).unwrap(); + let worker = runtime + .create_worker_scoped(&embedded_protocol_test_scope(), runtime_create_request()) + .unwrap() + .worker_ref; + let executor = owner(); + let mut connection = connect_embedded_worker_protocol( + embedded_protocol_test_source(&runtime, &worker), + embedded_protocol_test_scope(), + executor.clone(), + ) + .await + .unwrap(); + assert!(matches!( + observed(connection.events.recv()).await.unwrap(), + protocol::Event::Snapshot { .. } + )); + let (entered, held) = tokio::sync::oneshot::channel(); + let (tx, rx) = mpsc::channel(); + let mut release = Release(Some(tx)); + let endpoint = execution + .contexts + .lock() + .unwrap() + .get(&worker) + .unwrap() + .clone(); + let gate_finished = Arc::new(std::sync::atomic::AtomicBool::new(false)); + let finished = gate_finished.clone(); + *endpoint.dispatch_hook.lock().unwrap() = Some(Box::new(move || { + let _ = entered.send(()); + let _ = rx.recv_timeout(Duration::from_secs(5)); + finished.store(true, Ordering::SeqCst); + })); + connection + .methods + .send(protocol::Method::Shutdown { + command: protocol::WorkerCommandEnvelope::new(700), + }) + .await + .unwrap(); + observed(held).await.unwrap(); // actual Runtime method, after its durable stop admission + observed(tokio::time::sleep(Duration::from_millis(1))).await; + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + assert!( + !gate_finished.load(Ordering::SeqCst), + "watchdog bailout is not Tokio progress" + ); + connection + .termination + .terminate(WorkerProtocolEnd::Disconnected); + drop(connection); + executor.dispatch(|| {}).unwrap(); + assert!(matches!( + executor.run(|| Ok::<_, Error>(())).await.unwrap_err(), + Error::DbCapacityExceeded + )); + release.release(); + observed(executor.drain()).await.unwrap(); + assert_eq!( + runtime.worker_detail(&worker).unwrap().status, + worker_runtime::catalog::WorkerStatus::Stopped + ); + // Read the actual fs-store aggregate written by the accepted method. The + // deterministic execution fixture has no restorable Profile archive, so + // restarting it would test archive recovery rather than this commit owner. + let record: serde_json::Value = serde_json::from_slice( + &std::fs::read( + options + .root + .join("workers") + .join(worker.worker_id.to_string()) + .join("worker.json"), + ) + .unwrap(), + ) + .unwrap(); + assert_eq!( + record["status"], "stopped", + "cancelled pump lost its fs-store commit: {record}" + ); + assert_ne!( + record["execution_state"]["state"], + "reconciliation_required" + ); +} diff --git a/crates/workspace-server/src/server/tests/protocol_backpressure_tests.rs b/crates/workspace-server/src/server/tests/protocol_backpressure_tests.rs index 358b9a7bb..06759a67e 100644 --- a/crates/workspace-server/src/server/tests/protocol_backpressure_tests.rs +++ b/crates/workspace-server/src/server/tests/protocol_backpressure_tests.rs @@ -11,6 +11,7 @@ async fn embedded_event_overflow_terminates_without_losing_the_admitted_prefix() let mut connection = connect_embedded_worker_protocol( embedded_protocol_test_source(&runtime, &worker_ref), embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), ) .await .unwrap(); @@ -69,6 +70,7 @@ async fn embedded_protocol_delivers_snapshot_then_ordered_replies_and_live_event let mut connection = connect_embedded_worker_protocol( embedded_protocol_test_source(&runtime, &worker_ref), embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), ) .await .unwrap(); @@ -105,9 +107,13 @@ async fn embedded_protocol_delivers_snapshot_then_ordered_replies_and_live_event async fn embedded_overflow_requires_explicit_reconnect_and_stop_ends_the_new_connection() { let (runtime, worker_ref, execution) = runtime_with_worker_backend(); let source = embedded_protocol_test_source(&runtime, &worker_ref); - let mut old = connect_embedded_worker_protocol(source.clone(), embedded_protocol_test_scope()) - .await - .unwrap(); + let mut old = connect_embedded_worker_protocol( + source.clone(), + embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), + ) + .await + .unwrap(); assert_embedded_protocol_snapshot(next_embedded_protocol_event(&mut old).await, &worker_ref); tokio::time::timeout(PROTOCOL_TEST_TIMEOUT, async { for index in 0..=WORKER_PROTOCOL_EVENT_CAPACITY { @@ -127,9 +133,13 @@ async fn embedded_overflow_requires_explicit_reconnect_and_stop_ends_the_new_con // Overflow ends only the attached connection, not its Worker. Reconnection // is explicit and starts from a fresh execution snapshot, not old backlog. - let mut fresh = connect_embedded_worker_protocol(source, embedded_protocol_test_scope()) - .await - .unwrap(); + let mut fresh = connect_embedded_worker_protocol( + source, + embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), + ) + .await + .unwrap(); assert_embedded_protocol_snapshot(next_embedded_protocol_event(&mut fresh).await, &worker_ref); fresh .methods @@ -797,6 +807,7 @@ async fn embedded_disconnect_and_last_owner_drop_release_the_execution_event_sub let mut connection = connect_embedded_worker_protocol( embedded_protocol_test_source(&runtime, &worker_ref), embedded_protocol_test_scope(), + HostExecutors::default().host().unwrap(), ) .await .unwrap(); diff --git a/crates/workspace-server/src/server_residual_execution_tests.rs b/crates/workspace-server/src/server_residual_execution_tests.rs index 8b0e3620a..abbe46393 100644 --- a/crates/workspace-server/src/server_residual_execution_tests.rs +++ b/crates/workspace-server/src/server_residual_execution_tests.rs @@ -841,3 +841,64 @@ async fn ticket_tracking_and_case_authority_runtime_lookup_leave_db_and_tokio_fr ); } } + +#[tokio::test(flavor = "current_thread")] +async fn cancelled_feature_shutdown_keeps_sql_close_owned_without_blocking_control() { + let fixture = fixture().await; + let (database, _) = fixture.feature_database(); + let mutation = database.clone(); + let (entered, waiting) = oneshot::channel(); + let (release, permit) = mpsc::channel(); + let released = Arc::new(AtomicBool::new(false)); + let flag = released.clone(); + let thread = std::thread::spawn(move || { + mutation + .with_connection(|conn| { + conn.execute_batch("BEGIN IMMEDIATE; CREATE TABLE close_owner_probe(id INTEGER)")?; + let _ = entered.send(()); + let _ = permit.recv_timeout(WATCHDOG); + flag.store(true, Ordering::SeqCst); + conn.execute_batch("ROLLBACK")?; + Ok(()) + }) + .unwrap(); + }); + let mut lock = WriteLock { + release: Some(release), + thread: Some(thread), + released, + }; + observed(waiting).await.unwrap(); // actual SQL transaction owns the Feature connection + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let mut close = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + shutdown_workspace_feature_storage(&fixture.api), + )); + assert!( + futures::poll!(&mut close).is_pending(), + "Feature connection close must not wait on Tokio" + ); + runtime_progress_while_locked(&lock).await; + assert_eq!( + observed(fixture.host_executor.run(|| Ok::<_, Error>("control"))) + .await + .unwrap(), + "control" + ); + fixture.executor.dispatch(|| {}).unwrap(); + assert!(matches!( + shutdown_workspace_feature_storage(&fixture.api) + .await + .unwrap_err(), + Error::DbCapacityExceeded + )); + drop(close); + assert_eq!(leases.load(Ordering::SeqCst), 1); + lock.release(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + assert!( + database.with_connection(|_| Ok(())).is_err(), + "accepted shutdown must fence later use" + ); +} From e436b2ae58e940db416a5d6a9c2edb1368ec7600 Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 17:36:08 +0900 Subject: [PATCH 14/14] fix(server): own remaining request callers and stop continuation --- crates/workspace-server/src/server.rs | 800 ++++++++++++------ .../src/server/worker_operations.rs | 72 +- .../src/server_db_execution_tests.rs | 134 ++- .../src/server_drive_grants.rs | 14 +- .../src/server_residual_execution_tests.rs | 479 ++++++++++- .../src/server_workspace_config.rs | 6 +- .../2026-10-12-t748-caller-inventory.md | 287 +++++++ .../report/2026-10-12-t748-suite-diagnosis.md | 8 +- ...6-10-12-worker-observation-followup-gap.md | 49 ++ 9 files changed, 1507 insertions(+), 342 deletions(-) create mode 100644 docs/report/2026-10-12-t748-caller-inventory.md create mode 100644 docs/report/2026-10-12-worker-observation-followup-gap.md diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 1a5593d6d..fa5b9a8cb 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -8699,16 +8699,28 @@ impl server_api::ServerApi for ServerApiContractService { )); } let worker = RuntimeWorkerRef::new(runtime_id.clone(), worker_id.clone()); - let observation = api - .runtime - .worker_session(&workspace_id, &worker) - .map_err(|_error| { - runtime_api::RuntimeApiError::new( - StatusCode::BAD_GATEWAY.as_u16(), - "worker_session_observation_failed", - "Worker Session observation failed", + let operation_api = api.clone(); + let operation_workspace = workspace_id.clone(); + let runtime_result = api + .host_executors + .host() + .map_err(runtime_execution_error)? + .run(move || { + Ok::<_, Error>( + operation_api + .runtime + .worker_session(&operation_workspace, &worker), ) - })?; + }) + .await + .map_err(runtime_execution_error)?; + let observation = runtime_result.map_err(|_error| { + runtime_api::RuntimeApiError::new( + StatusCode::BAD_GATEWAY.as_u16(), + "worker_session_observation_failed", + "Worker Session observation failed", + ) + })?; Ok(server_api::WorkspaceWorkerSessionResponse { subject: server_api::WorkspaceWorkerSubject::RuntimeWorker { runtime_id, @@ -8743,16 +8755,29 @@ impl server_api::ServerApi for ServerApiContractService { )); } let worker = RuntimeWorkerRef::new(runtime_id.clone(), worker_id.clone()); - let observation = api - .runtime - .worker_session_history(&workspace_id, &worker, query.cursor, query.limit) - .map_err(|_error| { - runtime_api::RuntimeApiError::new( - StatusCode::BAD_GATEWAY.as_u16(), - "worker_session_history_failed", - "Worker Session history request failed", - ) - })?; + let operation_api = api.clone(); + let operation_workspace = workspace_id.clone(); + let runtime_result = api + .host_executors + .host() + .map_err(runtime_execution_error)? + .run(move || { + Ok::<_, Error>(operation_api.runtime.worker_session_history( + &operation_workspace, + &worker, + query.cursor, + query.limit, + )) + }) + .await + .map_err(runtime_execution_error)?; + let observation = runtime_result.map_err(|_error| { + runtime_api::RuntimeApiError::new( + StatusCode::BAD_GATEWAY.as_u16(), + "worker_session_history_failed", + "Worker Session history request failed", + ) + })?; Ok(server_api::WorkspaceWorkerSessionHistoryResponse { subject: server_api::WorkspaceWorkerSubject::RuntimeWorker { runtime_id, @@ -11624,7 +11649,7 @@ impl server_api::ServerApi for ServerApiContractService { let api = self.workspace_api().map_err(|_| { workspace_config::pre_error(Error::Store("Workspace unavailable".into())) })?; - let worker = workspace_config::identity(api, &context, &workspace_id)?; + let worker = workspace_config::identity(api, &context, &workspace_id).await?; workspace_config::get(api, &worker) .await .map(server_api::WorkspaceConfigCurrentResponse) @@ -11641,7 +11666,7 @@ impl server_api::ServerApi for ServerApiContractService { let api = self.workspace_api().map_err(|_| { workspace_config::pre_error(Error::Store("Workspace unavailable".into())) })?; - let worker = workspace_config::identity(api, &context, &workspace_id)?; + let worker = workspace_config::identity(api, &context, &workspace_id).await?; workspace_config::attach(api, &worker, request).await } async fn current_worker_workspace_config_observe( @@ -11656,7 +11681,7 @@ impl server_api::ServerApi for ServerApiContractService { let api = self.workspace_api().map_err(|_| { workspace_config::pre_error(Error::Store("Workspace unavailable".into())) })?; - let worker = workspace_config::identity(api, &context, &workspace_id)?; + let worker = workspace_config::identity(api, &context, &workspace_id).await?; workspace_config::observe(api, &worker, request).await } async fn current_worker_workspace_config_read( @@ -11671,7 +11696,7 @@ impl server_api::ServerApi for ServerApiContractService { let api = self.workspace_api().map_err(|_| { workspace_config::pre_error(Error::Store("Workspace unavailable".into())) })?; - let worker = workspace_config::identity(api, &context, &workspace_id)?; + let worker = workspace_config::identity(api, &context, &workspace_id).await?; workspace_config::read(api, &worker, request).await } async fn current_worker_workspace_config_commit( @@ -11686,7 +11711,7 @@ impl server_api::ServerApi for ServerApiContractService { let api = self.workspace_api().map_err(|_| { workspace_config::pre_error(Error::Store("Workspace unavailable".into())) })?; - let worker = workspace_config::identity(api, &context, &workspace_id)?; + let worker = workspace_config::identity(api, &context, &workspace_id).await?; workspace_config::commit(api, &worker, request).await } async fn workspace_config_grant_create( @@ -11999,6 +12024,16 @@ impl server_api::ServerApi for ServerApiContractService { } } +fn runtime_execution_error(error: Error) -> runtime_api::RuntimeApiError { + let code = match &error { + Error::DbCapacityExceeded => "db_capacity_exceeded", + Error::DbExecutorUnavailable => "db_executor_unavailable", + Error::DbOperationOutcomeUnknown => "db_operation_outcome_unknown", + _ => "backend_execution_failed", + }; + runtime_api::RuntimeApiError::new(api_error_status(&error).as_u16(), code, error.to_string()) +} + async fn scoped_worker_session_attachment( State(api): State, AxumPath((workspace_id, runtime_id, worker_id, session_id, attachment_id)): AxumPath<( @@ -12018,10 +12053,25 @@ async fn scoped_worker_session_attachment( .into_response(); } let worker = RuntimeWorkerRef::new(runtime_id, worker_id); - match api - .runtime - .worker_session_attachment(&workspace_id, &worker, &session_id, &attachment_id) - { + let executor = match api.host_executors.host() { + Ok(value) => value, + Err(error) => return ApiError::from(error).into_response(), + }; + let fetched = executor + .run(move || { + Ok::<_, Error>(api.runtime.worker_session_attachment( + &workspace_id, + &worker, + &session_id, + &attachment_id, + )) + }) + .await; + let fetched = match fetched { + Ok(value) => value, + Err(error) => return ApiError::from(error).into_response(), + }; + match fetched { Ok(attachment) => { let Ok(content_type) = HeaderValue::from_str(&attachment.media_type) else { return ( @@ -12941,10 +12991,15 @@ async fn scoped_get_repository_access_projection( Extension(actor): Extension, ) -> ApiResult> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - Ok(Json(active_repository_access_projection( - &api, - &path.workspace_id, - )?)) + api.store + .db_executor() + .run(move || { + Ok(Json(active_repository_access_projection( + &api, + &path.workspace_id, + )?)) + }) + .await } async fn scoped_list_repository_ssh_credentials( @@ -12953,13 +13008,18 @@ async fn scoped_list_repository_ssh_credentials( Extension(actor): Extension, ) -> ApiResult>> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - api.repository_secrets - .ensure_workspace_default_credential(&path.workspace_id)?; - let projection = active_repository_access_projection(&api, &path.workspace_id)?; - Ok(Json( - api.repository_secrets - .list_credentials(&path.workspace_id, &projection)?, - )) + api.store + .db_executor() + .run(move || { + api.repository_secrets + .ensure_workspace_default_credential(&path.workspace_id)?; + let projection = active_repository_access_projection(&api, &path.workspace_id)?; + Ok(Json( + api.repository_secrets + .list_credentials(&path.workspace_id, &projection)?, + )) + }) + .await } async fn scoped_get_repository_ssh_credential( @@ -12968,14 +13028,19 @@ async fn scoped_get_repository_ssh_credential( Extension(actor): Extension, ) -> ApiResult> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - let projection = active_repository_access_projection(&api, &path.workspace_id)?; - let credential = api - .repository_secrets - .list_credentials(&path.workspace_id, &projection)? - .into_iter() - .find(|credential| credential.credential_id == path.credential_id) - .ok_or_else(|| Error::InvalidRecordId(path.credential_id.clone()))?; - Ok(Json(credential)) + api.store + .db_executor() + .run(move || { + let projection = active_repository_access_projection(&api, &path.workspace_id)?; + let credential = api + .repository_secrets + .list_credentials(&path.workspace_id, &projection)? + .into_iter() + .find(|credential| credential.credential_id == path.credential_id) + .ok_or_else(|| Error::InvalidRecordId(path.credential_id.clone()))?; + Ok(Json(credential)) + }) + .await } async fn scoped_get_repository_ssh_public_key( @@ -12984,11 +13049,16 @@ async fn scoped_get_repository_ssh_public_key( Extension(actor): Extension, ) -> ApiResult> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - let public_key = api - .repository_secrets - .credential_public_key(&path.workspace_id, &path.credential_id)? - .ok_or_else(|| Error::InvalidRecordId(path.credential_id.clone()))?; - Ok(Json(public_key)) + api.store + .db_executor() + .run(move || { + let public_key = api + .repository_secrets + .credential_public_key(&path.workspace_id, &path.credential_id)? + .ok_or_else(|| Error::InvalidRecordId(path.credential_id.clone()))?; + Ok(Json(public_key)) + }) + .await } async fn scoped_create_repository_ssh_credential( @@ -12998,18 +13068,25 @@ async fn scoped_create_repository_ssh_credential( Json(request): Json, ) -> ApiResult<(StatusCode, Json)> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - if request.credential_id - == crate::repository_access::WORKSPACE_DEFAULT_REPOSITORY_SSH_CREDENTIAL_ID - { - return Err(Error::InvalidInput( - "Workspace default SSH credential id is reserved".to_string(), - ) - .into()); - } - let credential = - api.repository_secrets - .create_credential(&path.workspace_id, request, &actor.account_id)?; - Ok((StatusCode::CREATED, Json(credential))) + api.store + .db_executor() + .run(move || { + if request.credential_id + == crate::repository_access::WORKSPACE_DEFAULT_REPOSITORY_SSH_CREDENTIAL_ID + { + return Err(Error::InvalidInput( + "Workspace default SSH credential id is reserved".to_string(), + ) + .into()); + } + let credential = api.repository_secrets.create_credential( + &path.workspace_id, + request, + &actor.account_id, + )?; + Ok((StatusCode::CREATED, Json(credential))) + }) + .await } async fn scoped_generate_repository_ssh_credential( @@ -13019,20 +13096,25 @@ async fn scoped_generate_repository_ssh_credential( Json(request): Json, ) -> ApiResult<(StatusCode, Json)> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - if request.credential_id - == crate::repository_access::WORKSPACE_DEFAULT_REPOSITORY_SSH_CREDENTIAL_ID - { - return Err(Error::InvalidInput( - "Workspace default SSH credential id is reserved".to_string(), - ) - .into()); - } - let credential = api.repository_secrets.generate_credential( - &path.workspace_id, - request, - &actor.account_id, - )?; - Ok((StatusCode::CREATED, Json(credential))) + api.store + .db_executor() + .run(move || { + if request.credential_id + == crate::repository_access::WORKSPACE_DEFAULT_REPOSITORY_SSH_CREDENTIAL_ID + { + return Err(Error::InvalidInput( + "Workspace default SSH credential id is reserved".to_string(), + ) + .into()); + } + let credential = api.repository_secrets.generate_credential( + &path.workspace_id, + request, + &actor.account_id, + )?; + Ok((StatusCode::CREATED, Json(credential))) + }) + .await } async fn scoped_rotate_repository_ssh_credential( @@ -13042,12 +13124,17 @@ async fn scoped_rotate_repository_ssh_credential( Json(request): Json, ) -> ApiResult> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - Ok(Json(api.repository_secrets.rotate_credential( - &path.workspace_id, - &path.credential_id, - request, - &actor.account_id, - )?)) + api.store + .db_executor() + .run(move || { + Ok(Json(api.repository_secrets.rotate_credential( + &path.workspace_id, + &path.credential_id, + request, + &actor.account_id, + )?)) + }) + .await } async fn scoped_delete_repository_ssh_credential( @@ -13057,15 +13144,20 @@ async fn scoped_delete_repository_ssh_credential( Json(request): Json, ) -> ApiResult { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - let projection = active_repository_access_projection(&api, &path.workspace_id)?; - api.repository_secrets.delete_credential( - &path.workspace_id, - &path.credential_id, - request, - &actor.account_id, - &projection, - )?; - Ok(StatusCode::NO_CONTENT) + api.store + .db_executor() + .run(move || { + let projection = active_repository_access_projection(&api, &path.workspace_id)?; + api.repository_secrets.delete_credential( + &path.workspace_id, + &path.credential_id, + request, + &actor.account_id, + &projection, + )?; + Ok(StatusCode::NO_CONTENT) + }) + .await } async fn scoped_list_repository_ssh_host_trusts( @@ -13074,11 +13166,16 @@ async fn scoped_list_repository_ssh_host_trusts( Extension(actor): Extension, ) -> ApiResult>> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - let projection = active_repository_access_projection(&api, &path.workspace_id)?; - Ok(Json( - api.repository_secrets - .list_host_trusts(&path.workspace_id, &projection)?, - )) + api.store + .db_executor() + .run(move || { + let projection = active_repository_access_projection(&api, &path.workspace_id)?; + Ok(Json( + api.repository_secrets + .list_host_trusts(&path.workspace_id, &projection)?, + )) + }) + .await } async fn scoped_get_repository_ssh_host_trust( @@ -13087,14 +13184,19 @@ async fn scoped_get_repository_ssh_host_trust( Extension(actor): Extension, ) -> ApiResult> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - let projection = active_repository_access_projection(&api, &path.workspace_id)?; - let host_trust = api - .repository_secrets - .list_host_trusts(&path.workspace_id, &projection)? - .into_iter() - .find(|host_trust| host_trust.host_trust_id == path.host_trust_id) - .ok_or_else(|| Error::InvalidRecordId(path.host_trust_id.clone()))?; - Ok(Json(host_trust)) + api.store + .db_executor() + .run(move || { + let projection = active_repository_access_projection(&api, &path.workspace_id)?; + let host_trust = api + .repository_secrets + .list_host_trusts(&path.workspace_id, &projection)? + .into_iter() + .find(|host_trust| host_trust.host_trust_id == path.host_trust_id) + .ok_or_else(|| Error::InvalidRecordId(path.host_trust_id.clone()))?; + Ok(Json(host_trust)) + }) + .await } async fn scoped_put_repository_ssh_host_trust( @@ -13104,15 +13206,22 @@ async fn scoped_put_repository_ssh_host_trust( Json(request): Json, ) -> ApiResult<(StatusCode, Json)> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - let status = if request.expected_fingerprint.is_some() { - StatusCode::OK - } else { - StatusCode::CREATED - }; - let host_trust = - api.repository_secrets - .put_host_trust(&path.workspace_id, request, &actor.account_id)?; - Ok((status, Json(host_trust))) + api.store + .db_executor() + .run(move || { + let status = if request.expected_fingerprint.is_some() { + StatusCode::OK + } else { + StatusCode::CREATED + }; + let host_trust = api.repository_secrets.put_host_trust( + &path.workspace_id, + request, + &actor.account_id, + )?; + Ok((status, Json(host_trust))) + }) + .await } async fn scoped_delete_repository_ssh_host_trust( @@ -13122,15 +13231,20 @@ async fn scoped_delete_repository_ssh_host_trust( Json(request): Json, ) -> ApiResult { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; - let projection = active_repository_access_projection(&api, &path.workspace_id)?; - api.repository_secrets.delete_host_trust( - &path.workspace_id, - &path.host_trust_id, - request, - &actor.account_id, - &projection, - )?; - Ok(StatusCode::NO_CONTENT) + api.store + .db_executor() + .run(move || { + let projection = active_repository_access_projection(&api, &path.workspace_id)?; + api.repository_secrets.delete_host_trust( + &path.workspace_id, + &path.host_trust_id, + request, + &actor.account_id, + &projection, + )?; + Ok(StatusCode::NO_CONTENT) + }) + .await } fn config_content_type_to_api( @@ -13450,8 +13564,8 @@ async fn scoped_commit_workspace_config_tree( AxumPath(path): AxumPath, Json(request): Json, ) -> ApiResult<(StatusCode, Json)> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { let request = config_commit_request_from_api(request)?; validate_workspace_scope(&api, &path.workspace_id)?; @@ -23165,56 +23279,61 @@ async fn scoped_create_repository( Json(request): Json, ) -> ApiResult<(StatusCode, Json)> { require_workspace_owner(&api, &path.workspace_id, &actor, "ManageRepositories").await?; + api.store + .db_executor() + .run(move || { + server_api::validate_repository_key(&request.repository_key) + .map_err(|error| Error::InvalidInput(format!("invalid Repository key: {error}")))?; + let repository_key = request.repository_key; + let default_ref = request + .default_ref + .as_deref() + .map(|value| normalize_repository_text(value, "default_ref", 512)) + .transpose()?; + let source = parse_repository_source(&request.source).map_err(|error| { + Error::InvalidInput(format!("invalid repository source: {error}")) + })?; + let now = Utc::now().to_rfc3339(); + let record = RepositoryRecord { + workspace_id: path.workspace_id.clone(), + repository_id: Uuid::now_v7().to_string(), + repository_key: repository_key.clone(), + kind: "git".to_string(), + provider: Some("git".to_string()), + source_fingerprint: repository_source_fingerprint(&source), + source, + default_ref, + observed_status: server_api::RepositoryObservedStatus::Unverified, + observed_at: None, + created_at: now.clone(), + updated_at: now, + }; - server_api::validate_repository_key(&request.repository_key) - .map_err(|error| Error::InvalidInput(format!("invalid Repository key: {error}")))?; - let repository_key = request.repository_key; - let default_ref = request - .default_ref - .as_deref() - .map(|value| normalize_repository_text(value, "default_ref", 512)) - .transpose()?; - let source = parse_repository_source(&request.source) - .map_err(|error| Error::InvalidInput(format!("invalid repository source: {error}")))?; - let now = Utc::now().to_rfc3339(); - let record = RepositoryRecord { - workspace_id: path.workspace_id.clone(), - repository_id: Uuid::now_v7().to_string(), - repository_key: repository_key.clone(), - kind: "git".to_string(), - provider: Some("git".to_string()), - source_fingerprint: repository_source_fingerprint(&source), - source, - default_ref, - observed_status: server_api::RepositoryObservedStatus::Unverified, - observed_at: None, - created_at: now.clone(), - updated_at: now, - }; - - let (status, replayed) = match api.config_store.insert_repository(&record)? { - RepositoryInsertOutcome::Created => (StatusCode::CREATED, false), - RepositoryInsertOutcome::Existing(existing) - if repository_create_intent_matches(&existing, &record) => - { - (StatusCode::OK, true) - } - RepositoryInsertOutcome::Existing(_) => { - return Err(Error::RepositoryConflict(format!( + let (status, replayed) = match api.config_store.insert_repository(&record)? { + RepositoryInsertOutcome::Created => (StatusCode::CREATED, false), + RepositoryInsertOutcome::Existing(existing) + if repository_create_intent_matches(&existing, &record) => + { + (StatusCode::OK, true) + } + RepositoryInsertOutcome::Existing(_) => { + return Err(Error::RepositoryConflict(format!( "Repository key {repository_key} already exists with different registration intent" )) .into()); - } - }; + } + }; - Ok(( - status, - Json(CreateWorkspaceRepositoryResponse { - workspace_id: path.workspace_id, - repository_key, - replayed, - }), - )) + Ok(( + status, + Json(CreateWorkspaceRepositoryResponse { + workspace_id: path.workspace_id, + repository_key, + replayed, + }), + )) + }) + .await } fn normalize_repository_text(value: &str, field: &str, max_len: usize) -> Result { @@ -23256,7 +23375,10 @@ async fn scoped_probe_repository_ssh_connection( ) -> ApiResult> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; require_active_workspace_runtime_binding(&api, &request.runtime_id).await?; - probe_repository_ssh_connection(&api, &path, &request.runtime_id).map(Json) + api.host_executors + .host()? + .run(move || probe_repository_ssh_connection(&api, &path, &request.runtime_id).map(Json)) + .await } async fn scoped_confirm_repository_ssh_host_trust( @@ -23267,35 +23389,40 @@ async fn scoped_confirm_repository_ssh_host_trust( ) -> ApiResult> { require_manage_repository_secrets(&api, &path.workspace_id, &actor).await?; require_active_workspace_runtime_binding(&api, &request.runtime_id).await?; - let probe = probe_repository_ssh_connection(&api, &path, &request.runtime_id)?; - let candidate = probe - .candidates - .iter() - .find(|candidate| candidate.host_key == request.host_key) - .ok_or_else(|| { - settings_bad_request( - "repository_ssh_host_key_not_observed", - "Selected SSH host key was not presented by the target Runtime", - ) - })?; - if request.expected_host_key_fingerprint != probe.expected_host_key_fingerprint { - return Err(ApiError::from(Error::WorkspaceConfigConflict( - "SSH host trust fingerprint changed after the connection test".to_string(), - ))); - } - let host_trust = api.repository_secrets.put_host_trust( - &path.workspace_id, - PutRepositorySshHostTrustRequest { - operation_id: request.operation_id, - host_trust_id: probe.host_trust_id, - hostname: probe.hostname, - port: probe.port, - host_key: candidate.host_key.clone(), - expected_fingerprint: probe.expected_host_key_fingerprint, - }, - &actor.account_id, - )?; - Ok(Json(host_trust)) + api.host_executors + .host()? + .run(move || { + let probe = probe_repository_ssh_connection(&api, &path, &request.runtime_id)?; + let candidate = probe + .candidates + .iter() + .find(|candidate| candidate.host_key == request.host_key) + .ok_or_else(|| { + settings_bad_request( + "repository_ssh_host_key_not_observed", + "Selected SSH host key was not presented by the target Runtime", + ) + })?; + if request.expected_host_key_fingerprint != probe.expected_host_key_fingerprint { + return Err(ApiError::from(Error::WorkspaceConfigConflict( + "SSH host trust fingerprint changed after the connection test".to_string(), + ))); + } + let host_trust = api.repository_secrets.put_host_trust( + &path.workspace_id, + PutRepositorySshHostTrustRequest { + operation_id: request.operation_id, + host_trust_id: probe.host_trust_id, + hostname: probe.hostname, + port: probe.port, + host_key: candidate.host_key.clone(), + expected_fingerprint: probe.expected_host_key_fingerprint, + }, + &actor.account_id, + )?; + Ok(Json(host_trust)) + }) + .await } fn repository_ssh_connection_trust_state( @@ -25948,8 +26075,8 @@ async fn scoped_list_working_directories( State(api): State, AxumPath(path): AxumPath, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { validate_workspace_scope(&api, &path.workspace_id)?; let (items, diagnostics) = working_directory_summaries(&api)?; @@ -28085,75 +28212,84 @@ async fn upload_runtime_worker_file_contract( path: ScopedAttachmentUploadPath, body: &[u8], ) -> ApiResult { - validate_workspace_scope(&api, &path.workspace_id)?; - let body_sha256 = attachment_body_sha256(body); - let grant = { - let mut grants = api - .attachment_upload_grants - .lock() - .map_err(|_| Error::Store("attachment upload grant lock is poisoned".into()))?; - let grant = grants - .get_mut(&path.upload_id) - .ok_or_else(|| Error::InvalidInput("attachment upload grant is unknown".into()))?; - if let Some(file) = grant - .claim( - &path.workspace_id, - &path.runtime_id, - &path.worker_id, - &actor.account_id, - attachment_upload_now_ms()?, - &body_sha256, - ) - .map_err(ApiError::from)? - { - return Ok(WorkerFileUploadResponse { file }); - } - grant.clone() - }; + let body = body.to_vec(); + api.host_executors + .host()? + .run(move || { + validate_workspace_scope(&api, &path.workspace_id)?; + let body_sha256 = attachment_body_sha256(&body); + let grant = { + let mut grants = api + .attachment_upload_grants + .lock() + .map_err(|_| Error::Store("attachment upload grant lock is poisoned".into()))?; + let grant = grants.get_mut(&path.upload_id).ok_or_else(|| { + Error::InvalidInput("attachment upload grant is unknown".into()) + })?; + if let Some(file) = grant + .claim( + &path.workspace_id, + &path.runtime_id, + &path.worker_id, + &actor.account_id, + attachment_upload_now_ms()?, + &body_sha256, + ) + .map_err(ApiError::from)? + { + return Ok(WorkerFileUploadResponse { file }); + } + grant.clone() + }; - let worker = resolve_workspace_worker_reference(&api, &path.runtime_id, &path.worker_id)?; - let upload_context = worker_runtime::UploadedFileUploadContext { - upload_id: path.upload_id.clone(), - principal_id: grant.account_id.clone(), - workspace_id: grant.workspace_id.clone(), - runtime_id: grant.runtime_id.clone(), - worker_id: grant.worker_id.clone(), - }; - let uploaded = api.runtime.upload_worker_file( - &worker, - &grant.file_name, - &grant.media_type, - body, - Some(&upload_context), - ); - let mut grants = api - .attachment_upload_grants - .lock() - .map_err(|_| Error::Store("attachment upload grant lock is poisoned".into()))?; - let current = grants - .get_mut(&path.upload_id) - .ok_or_else(|| Error::RepositoryConflict("attachment upload grant disappeared".into()))?; - if matches!(current.state, AttachmentUploadGrantState::Cancelled) { - let uploaded_file = uploaded.ok(); - grants.remove(&path.upload_id); - drop(grants); - if let Some(file) = uploaded_file { - api.runtime - .delete_worker_uploaded_file(&worker, &file.artifact_id) - .map_err(|error| error.into_error())?; - } - return Err(Error::RepositoryConflict("attachment upload was cancelled".into()).into()); - } - match uploaded { - Ok(file) => { - current.state = AttachmentUploadGrantState::Completed(file.clone()); - Ok(WorkerFileUploadResponse { file }) - } - Err(error) => { - current.state = AttachmentUploadGrantState::Pending; - Err(error.into_error().into()) - } - } + let worker = + resolve_workspace_worker_reference(&api, &path.runtime_id, &path.worker_id)?; + let upload_context = worker_runtime::UploadedFileUploadContext { + upload_id: path.upload_id.clone(), + principal_id: grant.account_id.clone(), + workspace_id: grant.workspace_id.clone(), + runtime_id: grant.runtime_id.clone(), + worker_id: grant.worker_id.clone(), + }; + let uploaded = api.runtime.upload_worker_file( + &worker, + &grant.file_name, + &grant.media_type, + &body, + Some(&upload_context), + ); + let mut grants = api + .attachment_upload_grants + .lock() + .map_err(|_| Error::Store("attachment upload grant lock is poisoned".into()))?; + let current = grants.get_mut(&path.upload_id).ok_or_else(|| { + Error::RepositoryConflict("attachment upload grant disappeared".into()) + })?; + if matches!(current.state, AttachmentUploadGrantState::Cancelled) { + let uploaded_file = uploaded.ok(); + grants.remove(&path.upload_id); + drop(grants); + if let Some(file) = uploaded_file { + api.runtime + .delete_worker_uploaded_file(&worker, &file.artifact_id) + .map_err(|error| error.into_error())?; + } + return Err( + Error::RepositoryConflict("attachment upload was cancelled".into()).into(), + ); + } + match uploaded { + Ok(file) => { + current.state = AttachmentUploadGrantState::Completed(file.clone()); + Ok(WorkerFileUploadResponse { file }) + } + Err(error) => { + current.state = AttachmentUploadGrantState::Pending; + Err(error.into_error().into()) + } + } + }) + .await } async fn scoped_cancel_attachment_upload( @@ -29112,8 +29248,8 @@ async fn get_objective( async fn list_repositories( State(api): State, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { let RepositoryListProjection { items, diagnostics } = api.live_repository_reader()?.list(); @@ -29131,8 +29267,8 @@ async fn repository_detail( State(api): State, AxumPath(repository_key): AxumPath, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { server_api::validate_repository_key(&repository_key).map_err(|error| { ApiError::from(Error::InvalidInput(format!( @@ -29157,8 +29293,8 @@ async fn repository_log( AxumPath(repository_key): AxumPath, Query(query): Query, ) -> ApiResult> { - api.store - .db_executor() + api.host_executors + .host()? .run(move || { server_api::validate_repository_key(&repository_key).map_err(|error| { ApiError::from(Error::InvalidInput(format!( @@ -37568,6 +37704,7 @@ mod tests { Arc>>, reject_repository_access: Arc>, lifecycle_callback: Option>, + stop_callback: Option>, next_restore_outcome: Arc>>, restore_attempts: Arc>, } @@ -37874,6 +38011,9 @@ mod tests { worker_id: &str, _request: WorkerLifecycleRequest, ) -> WorkerLifecycleResult { + if let Some(callback) = &self.stop_callback { + callback(); + } WorkerLifecycleResult { state: InternalWorkerOperationState::Accepted, worker: RuntimeWorkerRef::new(Self::RUNTIME_ID, worker_id), @@ -38547,6 +38687,112 @@ mod tests { assert_eq!(error.error, "Payload Too Large"); } + #[tokio::test(flavor = "current_thread")] + async fn accepted_stop_closes_sessions_even_when_its_waiter_is_cancelled() { + let directory = tempfile::tempdir().unwrap(); + let api = test_api(directory.path()).await; + api.worker_projection.shutdown_async().await; + api.store.db_executor().drain().await.unwrap(); + register_test_runtime(&api, WorkdirlessFixtureRuntime::RUNTIME_ID).await; + let stopped = Arc::new(tokio::sync::Notify::new()); + let notification = stopped.clone(); + let runtime = WorkdirlessFixtureRuntime { + stop_callback: Some(Arc::new(move || notification.notify_one())), + ..Default::default() + }; + api.runtime.register_or_replace(runtime.clone()); + let identity = RuntimeWorkerRef::new( + WorkdirlessFixtureRuntime::RUNTIME_ID, + Uuid::now_v7().to_string(), + ); + api.store + .upsert_worker_registry(&WorkerRegistryRecord { + workspace_id: TEST_WORKSPACE_ID.into(), + worker: identity.clone(), + display_name: "stop owner fixture".into(), + profile: None, + retention_state: "normal".into(), + transcript_ref: None, + session_ref: None, + summary_ref: None, + diagnostics_ref: None, + created_at: "1".into(), + updated_at: "1".into(), + }) + .unwrap(); + runtime + .workers + .lock() + .unwrap() + .push(worker_summary_from_registry( + &api.store + .get_worker_registry(TEST_WORKSPACE_ID, &identity) + .unwrap() + .unwrap(), + )); + let session: WorkdirSessionHandle = Arc::new(workdir::LocalWorkdirSession::new( + manifest::Scope::writable(directory.path()).unwrap(), + directory.path().to_path_buf(), + )); + api.workdir_sessions + .lock() + .unwrap() + .insert_attachment(identity.clone(), session); + // An actual session fence prevents cleanup until after cancellation; + // the Runtime receipt is the synchronization event, not an elapsed delay. + let session_guard = current_worker_session_lock(&api, &identity) + .lock_owned() + .await; + let worker = + WorkspaceWorker::resolve(&api, &identity.runtime_id, &identity.worker_id).unwrap(); + let waiter = tokio::spawn(async move { + worker + .stop( + &WorkerOperationContext::Backend, + WorkerLifecycleRequest { + reason: None, + ticket_assignment: None, + }, + ) + .await + }); + tokio::time::timeout(std::time::Duration::from_secs(2), stopped.notified()) + .await + .unwrap(); + assert_eq!(tokio::spawn(async { "control" }).await.unwrap(), "control"); + api.store + .db_executor() + .run(|| Ok::<_, Error>(())) + .await + .unwrap(); + waiter.abort(); + assert!(waiter.await.unwrap_err().is_cancelled()); + assert!( + api.workdir_sessions + .lock() + .unwrap() + .attachments + .contains_key(&identity) + ); + drop(session_guard); + tokio::time::timeout( + std::time::Duration::from_secs(2), + api.host_executors.host().unwrap().drain(), + ) + .await + .unwrap() + .unwrap(); + api.store.db_executor().drain().await.unwrap(); + assert!( + !api.workdir_sessions + .lock() + .unwrap() + .attachments + .contains_key(&identity), + "accepted Runtime stop must own session cleanup after the waiter disappears" + ); + } + #[tokio::test] async fn command_session_survives_attachment_refresh_until_worker_revocation() { let directory = tempfile::tempdir().unwrap(); diff --git a/crates/workspace-server/src/server/worker_operations.rs b/crates/workspace-server/src/server/worker_operations.rs index 447ae7e29..cce576556 100644 --- a/crates/workspace-server/src/server/worker_operations.rs +++ b/crates/workspace-server/src/server/worker_operations.rs @@ -383,46 +383,38 @@ impl WorkspaceWorker { ) -> ApiResult { let guard = self.authorize_operation(context, "stop").await?; let worker = self.clone(); - let (result, guard) = self - .api - .host_executors - .host()? - .run(move || -> ApiResult<_> { - let result = worker - .api - .runtime - .stop_worker(&worker.identity, request) - .map_err(|e| e.into_error())?; - Ok((result, guard)) - }) - .await?; - if result.state != InternalWorkerOperationState::Accepted { - return Ok(result); - } - parse_runtime_worker_id_for_registry(&self.identity.worker_id)?; - let session_lock = current_worker_session_lock(&self.api, &self.identity); - let session_guard = session_lock.lock_owned().await; - close_current_worker_session_locked(&self.api, &self.identity).await?; - let worker = self.clone(); - self.api - .host_executors - .host()? - .run(move || -> ApiResult<_> { - let (_control, _session) = (guard, session_guard); - if let Some(record) = worker - .api - .store - .get_worker_registry(worker.api.workspace_id(), &worker.identity)? - { - sync_linked_workdir_after_worker_stop( - &worker.api, - &worker.identity.runtime_id, - &record, - )?; - } - Ok(result) - }) - .await + // Once admitted, Runtime stop, session close and linked Workdir + // publication have one owner. Never hand an accepted stop's guards + // back to its cancellable waiter or acquire this Host lane again. + run_host_workflow(self.api.host_executors.host()?, async move { + let _control = guard; + let result = worker + .api + .runtime + .stop_worker(&worker.identity, request) + .map_err(|error| error.into_error())?; + if result.state != InternalWorkerOperationState::Accepted { + return Ok(result); + } + parse_runtime_worker_id_for_registry(&worker.identity.worker_id)?; + let _session = current_worker_session_lock(&worker.api, &worker.identity) + .lock_owned() + .await; + close_current_worker_session_locked(&worker.api, &worker.identity).await?; + if let Some(record) = worker + .api + .store + .get_worker_registry(worker.api.workspace_id(), &worker.identity)? + { + sync_linked_workdir_after_worker_stop( + &worker.api, + &worker.identity.runtime_id, + &record, + )?; + } + Ok(result) + }) + .await } pub(super) async fn cancel( diff --git a/crates/workspace-server/src/server_db_execution_tests.rs b/crates/workspace-server/src/server_db_execution_tests.rs index d52ffacd6..e02698316 100644 --- a/crates/workspace-server/src/server_db_execution_tests.rs +++ b/crates/workspace-server/src/server_db_execution_tests.rs @@ -20,10 +20,16 @@ const SUBJECTS: &str = "/api/w/db-execution/subjektiv/subjects"; const OBSERVE_LIMIT: StdDuration = StdDuration::from_secs(2); const WATCHDOG: StdDuration = StdDuration::from_secs(5); -async fn observed(future: impl Future) -> T { - tokio::time::timeout(OBSERVE_LIMIT, future) - .await - .expect("request did not reach the expected DB/lifecycle state") +#[track_caller] +fn observed(future: impl Future) -> impl Future { + let caller = std::panic::Location::caller(); + async move { + tokio::time::timeout(OBSERVE_LIMIT, future) + .await + .unwrap_or_else(|_| { + panic!("request did not reach the expected DB/lifecycle state at {caller}") + }) + } } struct Fixture { @@ -954,7 +960,7 @@ impl HeldRuntimeReply { Self::with_held_path(Some(path.to_owned())) } fn with_held_path(path: Option) -> Self { - use std::io::{Read, Write}; + use std::io::Write; let listener = std::net::TcpListener::bind("127.0.0.1:0").unwrap(); let address = listener.local_addr().unwrap(); let (entered, received) = oneshot::channel(); @@ -967,16 +973,9 @@ impl HeldRuntimeReply { return; }; stream.set_read_timeout(Some(WATCHDOG)).unwrap(); - let mut buf = [0; 4096]; - let Ok(n) = stream.read(&mut buf) else { return }; - if n == 0 { + let Ok(Some(request)) = read_runtime_fixture_request(&mut stream) else { return; - } - let request = String::from_utf8_lossy(&buf[..n]) - .lines() - .next() - .unwrap() - .to_owned(); + }; if path.as_ref().is_some_and(|path| !request.contains(path)) { // Ready's best-effort notification lookup precedes its detail // source observation. Deny it promptly; hold only the named stage. @@ -1012,6 +1011,76 @@ impl Drop for HeldRuntimeReply { } } +// Read one complete request before selecting a held path or denying a prior +// notification. TCP reads are not HTTP frame boundaries; replying while a POST +// body remains unread can also reset the connection instead of delivering 503. +fn read_runtime_fixture_request(reader: impl std::io::Read) -> std::io::Result> { + use std::io::{BufRead, Read}; + let mut reader = std::io::BufReader::new(reader); + let mut line = String::new(); + if reader.read_line(&mut line)? == 0 { + return Ok(None); + } + let request = line.trim_end().to_owned(); + let mut header_bytes = line.len(); + let mut body_len = 0; + loop { + line.clear(); + if reader.read_line(&mut line)? == 0 { + return Err(std::io::ErrorKind::UnexpectedEof.into()); + } + header_bytes += line.len(); + if header_bytes > 16384 { + return Err(std::io::ErrorKind::InvalidData.into()); + } + if line == "\r\n" { + break; + } + if let Some((name, value)) = line.split_once(':') { + if name.eq_ignore_ascii_case("content-length") { + body_len = value + .trim() + .parse::() + .map_err(|_| std::io::ErrorKind::InvalidData)?; + if body_len > 1024 * 1024 { + return Err(std::io::ErrorKind::InvalidData.into()); + } + } + if name.eq_ignore_ascii_case("transfer-encoding") { + return Err(std::io::ErrorKind::InvalidData.into()); + } + } + } + let mut body = vec![0; body_len]; + reader.read_exact(&mut body)?; + Ok(Some(request)) +} + +#[test] +fn held_runtime_fixture_consumes_fragmented_headers_and_body_before_selecting_the_path() { + struct Fragmented(std::io::Cursor>); + impl std::io::Read for Fragmented { + fn read(&mut self, buffer: &mut [u8]) -> std::io::Result { + let length = buffer.len().min(2); + std::io::Read::read(&mut self.0, &mut buffer[..length]) + } + } + let request = b"POST /v1/repository-refs/observe HTTP/1.1\r\nHost: localhost\r\nContent-Length: 5\r\n\r\nnotes"; + let mut reader = Fragmented(std::io::Cursor::new(request.to_vec())); + assert_eq!( + read_runtime_fixture_request(&mut reader) + .unwrap() + .as_deref(), + Some("POST /v1/repository-refs/observe HTTP/1.1") + ); + assert_eq!(reader.0.position(), request.len() as u64); + let truncated = &request[..request.len() - 1]; + assert_eq!( + read_runtime_fixture_request(truncated).unwrap_err().kind(), + std::io::ErrorKind::UnexpectedEof + ); +} + struct CountJobLifetime(Arc); struct CountJobLease(Arc); impl DbOperationLifetime for CountJobLifetime { @@ -1538,6 +1607,31 @@ async fn cancelled_runtime_key_revoke_commits_audit_and_removes_broker_registrat protocol::subscription::EventSubscriptionSelector::RuntimeWorkers, ) .unwrap(); + // Broker registration starts an independent cold signing DB job. Observe + // its actual handshake before occupying SQL for revoke: otherwise this + // fixture races that producer against HTTP auth in its 1+1 DB budget. + // Keep the handshake pending so downstream shutdown is still exercised. + let (mut upstream, _) = observed(listener.accept()).await.unwrap(); + let handshake = observed(async { + use tokio::io::AsyncReadExt; + let mut bytes = Vec::new(); + while !bytes.ends_with(b"\r\n\r\n") { + let mut chunk = [0; 1024]; + let count = upstream.read(&mut chunk).await.unwrap(); + assert_ne!(count, 0, "broker closed before sending its handshake"); + bytes.extend_from_slice(&chunk[..count]); + assert!(bytes.len() <= 8192, "unexpectedly large handshake"); + } + String::from_utf8(bytes).unwrap() + }) + .await; + assert!(handshake.starts_with("GET /v1/protocol/ws ")); + assert!( + handshake + .to_ascii_lowercase() + .contains("authorization: bearer ") + ); + observed(fixture.executor.drain()).await.unwrap(); let (mut busy, waiting) = BusyObservation::install_store(fixture.store.clone()); let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); let server = fixture.server.clone(); @@ -1546,9 +1640,17 @@ async fn cancelled_runtime_key_revoke_commits_audit_and_removes_broker_registrat "/api/w/db-execution/runtimes/revoke-runtime/trust-key", serde_json::json!({"expected_binding_id": binding.binding_id}), ); - let revoke = + let mut revoke = tokio::spawn(async move { dispatch_workspace_request(State(server), request).await }); - observed(waiting).await.unwrap(); // Actual authenticated revoke reached BEGIN IMMEDIATE. + tokio::select! { + reached = observed(waiting) => reached.unwrap(), // Actual authenticated BEGIN IMMEDIATE. + response = &mut revoke => { + let response = response.unwrap(); + let status = response.status(); + let body = axum::body::to_bytes(response.into_body(), 65536).await.unwrap(); + panic!("revoke returned before its SQLite wait: {status}: {}", String::from_utf8_lossy(&body)); + } + } runtime_progress_while_locked(&lock).await; revoke.abort(); assert!(revoke.await.unwrap_err().is_cancelled()); diff --git a/crates/workspace-server/src/server_drive_grants.rs b/crates/workspace-server/src/server_drive_grants.rs index 4d1f82fe5..8a82c51db 100644 --- a/crates/workspace-server/src/server_drive_grants.rs +++ b/crates/workspace-server/src/server_drive_grants.rs @@ -64,8 +64,9 @@ pub(super) async fn authorized_drive( // incomplete, or inconsistent with its transport identity. let headers = current_worker_contract_headers(context) .map_err(|_| fail(DriveApiErrorCode::Denied))?; - let worker = current_worker_identity(api, workspace_id, &headers) - .map_err(|_| fail(DriveApiErrorCode::Denied))?; + let worker = authenticate_worker_mutation_source_owned(api, workspace_id, &headers) + .await + .map_err(pre_error)?; if context.worker_source.as_ref().is_some_and(|source| { source.runtime_id != worker.runtime_id || source.worker_id != worker.worker_id }) { @@ -139,9 +140,14 @@ pub(super) async fn create_grant( } } let worker = RuntimeWorkerRef::new(&request.runtime_id, &request.worker_id); + let operation_api = api.clone(); let summary = api - .runtime - .worker(&worker) + .host_executors + .host() + .map_err(pre_error)? + .run(move || Ok::<_, Error>(operation_api.runtime.worker(&worker))) + .await + .map_err(pre_error)? .map_err(|_| fail(DriveApiErrorCode::NotFound))?; if summary.workspace.workspace_id.as_deref() != Some(workspace_id) { return Err(fail(DriveApiErrorCode::Denied)); diff --git a/crates/workspace-server/src/server_residual_execution_tests.rs b/crates/workspace-server/src/server_residual_execution_tests.rs index abbe46393..6cf3759fa 100644 --- a/crates/workspace-server/src/server_residual_execution_tests.rs +++ b/crates/workspace-server/src/server_residual_execution_tests.rs @@ -561,7 +561,14 @@ async fn ticket_source_ref_reads_and_ready_publication_leave_sql_and_tokio_free( }, )); assert!(futures::poll!(&mut response).is_pending()); - let request = observed(reply.entered.take().unwrap()).await.unwrap(); + // Ready reaches the source POST only after mutation and best-effort + // notification. Bound this multi-stage setup with the fixture watchdog, + // not the shorter budget used below to prove timer/independent control + // progress while the actual provider request is held. + let request = tokio::select! { + request = tokio::time::timeout(WATCHDOG, reply.entered.take().unwrap()) => request.expect("source observation setup did not reach the provider").unwrap(), + result = &mut response => panic!("{operation} returned before the held source observation: {result:?}"), + }; assert!( request.contains("/v1/repository-refs/observe"), "{operation}: {request}" @@ -902,3 +909,473 @@ async fn cancelled_feature_shutdown_keeps_sql_close_owned_without_blocking_contr "accepted shutdown must fence later use" ); } + +#[tokio::test(flavor = "current_thread")] +async fn session_workdir_and_upload_callers_keep_provider_waits_off_sql_and_tokio() { + use server_api::ServerApi; + for operation in ["session", "history", "attachment", "workdirs", "upload"] { + let fixture = fixture().await; + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "caller-runtime", + &format!("http://{}", reply.address), + false, + ); + let worker_id = Uuid::now_v7().to_string(); + fixture + .store + .upsert_worker_registry(®istry_record(RuntimeWorkerRef::new( + "caller-runtime", + &worker_id, + ))) + .unwrap(); + let actor = RequestActor { + user_id: "fixture-user".into(), + account_id: format!("account-{WORKSPACE}"), + handle: "fixture-user".into(), + display_name: "Fixture user".into(), + auth_method: ActorAuthMethod::BrowserSession, + }; + fixture.api.attachment_upload_grants.lock().unwrap().insert( + "caller-upload".into(), + AttachmentUploadGrant { + workspace_id: WORKSPACE.into(), + runtime_id: "caller-runtime".into(), + worker_id: worker_id.clone(), + account_id: actor.account_id.clone(), + file_name: "notes.txt".into(), + media_type: "text/plain".into(), + expires_at_ms: u64::MAX, + state: AttachmentUploadGrantState::Pending, + }, + ); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let api = fixture.api.clone(); + let task = tokio::spawn(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + async move { + let service = ServerApiContractService::Workspace(api.clone()); + match operation { + "session" => { + let _ = service + .worker_session(WORKSPACE.into(), "caller-runtime".into(), worker_id) + .await; + } + "history" => { + let _ = service + .worker_session_history( + WORKSPACE.into(), + "caller-runtime".into(), + worker_id, + server_api::WorkspaceWorkerSessionHistoryQuery { + cursor: None, + limit: Some(1), + }, + ) + .await; + } + "attachment" => { + let _ = scoped_worker_session_attachment( + State(api), + AxumPath(( + WORKSPACE.into(), + "caller-runtime".into(), + worker_id, + "session".into(), + "attachment".into(), + )), + Extension(actor), + ) + .await; + } + "workdirs" => { + let _ = scoped_list_working_directories( + State(api), + AxumPath(ScopedWorkspacePath { + workspace_id: WORKSPACE.into(), + }), + ) + .await; + } + "upload" => { + let _ = upload_runtime_worker_file_contract( + api, + actor, + ScopedAttachmentUploadPath { + workspace_id: WORKSPACE.into(), + runtime_id: "caller-runtime".into(), + worker_id, + upload_id: "caller-upload".into(), + }, + b"notes", + ) + .await; + } + _ => unreachable!(), + } + }, + )); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!(request.contains("/v1/"), "{operation}: {request}"); + assert!( + !reply.upstream_released.load(Ordering::SeqCst), + "{operation}: watchdog release is not executor progress" + ); + let store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; + let mut queued = Box::pin(fixture.host_executor.run(|| Ok::<_, Error>(()))); + assert!(futures::poll!(&mut queued).is_pending()); + let error = scoped_list_working_directories( + State(fixture.api.clone()), + AxumPath(ScopedWorkspacePath { + workspace_id: WORKSPACE.into(), + }), + ) + .await + .unwrap_err(); + response_json(error.into_response(), StatusCode::SERVICE_UNAVAILABLE).await; + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + assert_eq!( + leases.load(Ordering::SeqCst), + 1, + "{operation}: lost provider owner" + ); + reply.finish(); + observed(queued).await.unwrap(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!( + leases.load(Ordering::SeqCst), + 0, + "{operation}: retained owner after completion" + ); + if operation == "upload" { + assert!( + matches!( + fixture + .api + .attachment_upload_grants + .lock() + .unwrap() + .get("caller-upload") + .unwrap() + .state, + AttachmentUploadGrantState::Pending + ), + "failed Runtime lookup must finish the accepted grant claim even with a cancelled waiter" + ); + } + } +} + +#[tokio::test(flavor = "current_thread")] +async fn repository_create_keeps_cancelled_sql_commit_owned_and_replays_one_intent() { + let fixture = fixture().await; + let (mut busy, waiting) = BusyObservation::install_store(fixture.api.config_store.clone()); + let mut lock = WriteLock::acquire(fixture.api.config.database_path.clone()); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let request = || { + Fixture::request( + "POST", + &format!("/api/w/{WORKSPACE}/repositories"), + serde_json::json!({ "repository_key": "owned-repository", "source": "https://example.invalid/repo.git", "default_ref": "main" }), + ) + }; + let mut response = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + fixture.inner_router().oneshot(request()), + )); + tokio::select! { + result = &mut response => panic!("repository must wait for its write transaction: {:?}", result.unwrap().status()), + reached = observed(waiting) => reached.unwrap(), + } + runtime_progress_while_locked(&lock).await; + assert_eq!( + observed(fixture.host_executor.run(|| Ok::<_, Error>("control"))) + .await + .unwrap(), + "control" + ); + drop(response); + assert_eq!(leases.load(Ordering::SeqCst), 1); + lock.release(); + busy.resume(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + let rows = fixture.store.list_repositories(WORKSPACE).unwrap(); + assert_eq!( + rows.iter() + .filter(|record| record.repository_key == "owned-repository") + .count(), + 1 + ); + let replay = observed(fixture.inner_router().oneshot(request())) + .await + .unwrap(); + let body = response_json(replay, StatusCode::OK).await; + assert_eq!(body["replayed"], true); + assert_eq!( + fixture.store.list_repositories(WORKSPACE).unwrap().len(), + rows.len() + ); +} + +#[tokio::test(flavor = "current_thread")] +async fn drive_and_config_preflight_keep_live_authority_owned_and_report_overload_as_availability() +{ + async fn preflight( + api: &WorkspaceApi, + context: &server_api::ServerRequestContext, + operation: &str, + ) -> u16 { + match operation { + "config" => { + workspace_config::identity(api, context, WORKSPACE) + .await + .err() + .unwrap() + .status + } + "drive" => api_macros::HttpError::status_code( + &drive_grants::authorized_drive(api, context, WORKSPACE, false) + .await + .err() + .unwrap(), + ), + "grant" => api_macros::HttpError::status_code( + &drive_grants::create_grant( + api, + context, + WORKSPACE, + server_api::DriveGrantCreateRequest { + runtime_id: "preflight-runtime".into(), + worker_id: "target-worker".into(), + access: server_api::DriveAccess::ReadOnly, + }, + ) + .await + .err() + .unwrap(), + ), + _ => unreachable!(), + } + } + for operation in ["config", "drive", "grant"] { + let fixture = fixture().await; + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "preflight-runtime", + &format!("http://{}", reply.address), + false, + ); + let context = server_api::ServerRequestContext { + actor: if operation == "grant" { + Some(RequestActor { + user_id: "fixture-user".into(), + account_id: format!("account-{WORKSPACE}"), + handle: "fixture-user".into(), + display_name: "Fixture user".into(), + auth_method: ActorAuthMethod::BrowserSession, + }) + } else { + None + }, + worker_source: None, + runtime_source: if operation == "grant" { + None + } else { + Some(server_api::ServerRuntimeSource { + authentication_binding_id: "fixture-binding".into(), + runtime_id: "preflight-runtime".into(), + worker_id: Some("source-worker".into()), + }) + }, + origin: None, + transport_headers: Vec::new(), + }; + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let api = fixture.api.clone(); + let request_context = context.clone(); + let task = tokio::spawn(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + async move { preflight(&api, &request_context, operation).await }, + )); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!(request.contains("/v1/workers/"), "{operation}: {request}"); + let store = fixture.store.clone(); + observed( + fixture + .executor + .run(move || store.get_workspace_memory_settings(WORKSPACE)), + ) + .await + .unwrap(); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + assert!(!reply.upstream_released.load(Ordering::SeqCst)); + task.abort(); + assert!(task.await.unwrap_err().is_cancelled()); + assert_eq!(leases.load(Ordering::SeqCst), 1); + fixture.host_executor.dispatch(|| {}).unwrap(); + assert_eq!( + observed(preflight(&fixture.api, &context, operation)).await, + 503, + "{operation}: overload must not be authority denial" + ); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + } +} + +#[tokio::test(flavor = "current_thread")] +async fn ui_config_commit_publishes_after_commit_without_holding_the_sql_lane() { + let fixture = fixture().await; + let current = fixture + .api + .config_store + .load_workspace_config(WORKSPACE) + .unwrap() + .unwrap(); + let mut reply = HeldRuntimeReply::new(); + install_remote_fixture( + &fixture, + "prompt-runtime", + &format!("http://{}", reply.address), + false, + ); + let leases = Arc::new(std::sync::atomic::AtomicUsize::new(0)); + let mut commit = Box::pin(crate::db_executor::with_operation_lifetime( + Arc::new(CountJobLifetime(leases.clone())), + scoped_commit_workspace_config_tree( + State(fixture.api.clone()), + AxumPath(ScopedWorkspacePath { + workspace_id: WORKSPACE.into(), + }), + Json(server_api::ConfigCommitRequest { + base_digest: current.snapshot.digest.clone(), + entrypoints: current + .contract + .entrypoints + .iter() + .map(ToString::to_string) + .collect(), + changes: vec![server_api::ConfigTreeChange::Create { + path: "notes/owned-publication.txt".into(), + content_type: server_api::ConfigContentType::Text, + content: "committed before prompt publication".into(), + }], + }), + ), + )); + assert!(futures::poll!(&mut commit).is_pending()); + let request = observed(reply.entered.take().unwrap()).await.unwrap(); + assert!( + request.starts_with("POST "), + "prompt publication: {request}" + ); + let config_store = fixture.api.config_store.clone(); + let state = observed( + fixture + .executor + .run(move || config_store.load_workspace_config(WORKSPACE)), + ) + .await + .unwrap() + .unwrap(); + assert_ne!( + state.snapshot.digest, current.snapshot.digest, + "publication must observe the committed tree" + ); + assert!( + state + .snapshot + .entries + .keys() + .any(|path| path.to_string() == "notes/owned-publication.txt") + ); + observed(tokio::time::sleep(StdDuration::from_millis(1))).await; + assert_eq!( + observed(tokio::spawn(async { "control" })).await.unwrap(), + "control" + ); + assert!(!reply.upstream_released.load(Ordering::SeqCst)); + drop(commit); + assert_eq!(leases.load(Ordering::SeqCst), 1); + reply.finish(); + observed(fixture.host_executor.drain()).await.unwrap(); + observed(fixture.executor.drain()).await.unwrap(); + assert_eq!(leases.load(Ordering::SeqCst), 0); + assert_eq!( + fixture + .api + .config_store + .load_workspace_config(WORKSPACE) + .unwrap() + .unwrap() + .snapshot + .digest, + state.snapshot.digest, + "failed best-effort publication must not repeat or roll back the commit" + ); +} + +#[tokio::test(flavor = "current_thread")] +async fn repository_provider_reads_do_not_consume_the_sql_execution_queue() { + let fixture = fixture().await; + let (entered, waiting) = oneshot::channel(); + let (release, permit) = mpsc::channel(); + fixture + .executor + .dispatch(move || { + let _ = entered.send(()); + let _ = permit.recv_timeout(WATCHDOG); + }) + .unwrap(); + observed(waiting).await.unwrap(); + fixture.executor.dispatch(|| {}).unwrap(); + let probe = fixture.executor.run(|| Ok::<_, Error>(())).await; + assert!(matches!(probe, Err(Error::DbCapacityExceeded))); + // Empty catalog / absent key are deliberate boundary inputs: provider + // reads keep their ordinary response even when SQL job admission is full. + let list = observed(list_repositories(State(fixture.api.clone()))) + .await + .unwrap(); + assert!(list.0.items.is_empty()); + let detail = observed(repository_detail( + State(fixture.api.clone()), + AxumPath("missing".into()), + )) + .await + .unwrap_err(); + response_json(detail.into_response(), StatusCode::NOT_FOUND).await; + let log = observed(repository_log( + State(fixture.api.clone()), + AxumPath("missing".into()), + Query(LogQuery { limit: Some(1) }), + )) + .await + .unwrap_err(); + response_json(log.into_response(), StatusCode::NOT_FOUND).await; + let _ = release.send(()); + observed(fixture.executor.drain()).await.unwrap(); + observed(fixture.host_executor.drain()).await.unwrap(); +} diff --git a/crates/workspace-server/src/server_workspace_config.rs b/crates/workspace-server/src/server_workspace_config.rs index f1372b1e4..dfaa104af 100644 --- a/crates/workspace-server/src/server_workspace_config.rs +++ b/crates/workspace-server/src/server_workspace_config.rs @@ -62,7 +62,7 @@ fn unknown() -> Failure { ..fail(500, "outcome_unknown") } } -pub(super) fn identity( +pub(super) async fn identity( api: &WorkspaceApi, context: &server_api::ServerRequestContext, workspace_id: &str, @@ -70,7 +70,9 @@ pub(super) fn identity( validate_workspace_scope(api, workspace_id).map_err(pre_error)?; let headers = current_worker_contract_headers(context).map_err(|_| fail(403, "access_denied"))?; - current_worker_identity(api, workspace_id, &headers).map_err(pre_error) + authenticate_worker_mutation_source_owned(api, workspace_id, &headers) + .await + .map_err(pre_error) } async fn lock( api: &WorkspaceApi, diff --git a/docs/report/2026-10-12-t748-caller-inventory.md b/docs/report/2026-10-12-t748-caller-inventory.md new file mode 100644 index 000000000..3fe21efba --- /dev/null +++ b/docs/report/2026-10-12-t748-caller-inventory.md @@ -0,0 +1,287 @@ +# T-748: async caller ownership inventory and scoped repair evidence + +## Current status and evidence boundary + +MR `01a12d3f-e90c-7b02-9ed9-cf3bd2e822a1` seq13 requested changes for published +`ebce7ab7b1230654b8eb5340afca8de835f24f12`. The independent review confirmed the +revoke cancellation/publication fix, but found three further direct async callers. +Earlier full suites exercised successful outcomes without proving every composed +caller boundary. Repeatedly treating a helper's apparent purpose as its lane +classification missed Runtime lookup in Ticket projection and post-await reads. +A successful suite is not a substitute for the entry-to-publication inventory. + +The seq13 and adjacent fixes were committed as `5873f250`; embedded protocol +and Feature shutdown fixes as `4d981194`. The remaining confirmed caller-placement +and Worker stop repairs are included in the follow-up below. The read-only audit +children were released after result-observation gaps; that release was not treated +as completion. No audit-to-implementation approval gate is imposed. +No target integration, Ticket conclusion, live update or attachment cleanup is +performed. T743 domain APIs are not copied or implemented here; their owner still +needs normal composition and exact combined-source review. + +The tables and line numbers below preserve the **pre-repair audit snapshot**. +“Unfixed”, “pending”, and “remaining” in those historical sections describe that +snapshot, not the current implementation. The final section supersedes their +placement/stop status and explicitly distinguishes tested boundaries from limits. + +Audited snapshot `server.rs` SHA-256: +`dae05a2649f5d4f884e8118ba4ce6baa902de6e61727c20e97c0afa3661ef3a6`. +Audited snapshot `worker_operations.rs` SHA-256: +`e3ad2327370440fdc40fb2c76f9604b984667e3472570d76145f4d0987d343fd`. +These file hashes identify the inspected implementation, not a provider review ref. + +## Current confirmed ownership changes + +| Entry / composed family | Authority and synchronous dependencies | Current owner / completion boundary | Evidence and limits | +|---|---|---|---| +| `ticket_implementation_cancel` → `scoped_cancel_ticket_implementation` | Ticket authority/assignment reads; Runtime cancel; current-session close; intact assignment cancellation transaction; detail | One accepted finite Host workflow owns cancellation through transaction and response. No Runtime wait on DB, no nested Host admission | Pre-authenticated generated POST route; real separate SQLite writer holds actual cancellation transaction; timer/control progress; cancelled waiter retains actual lease, then one operation/cleared assignment after drain. Saturated Host returns 503 without mutation. Existing live Coder cancellation policy passes | +| `merge_request_selector_repair` → `scoped_repair_merge_request_selector` | Browser auth/explicit confirmation; MR store; current assignment; provider ref; intact repair/audit; public projection | Auth remains an owned DB read; confirmed post-auth mixed stage is one finite Host job | Browser auth is drained before real SQL/shared-mutex contention; post-auth stage stays Pending without blocking Tokio and retains cancelled Host lease. Imported NULL-selector/no-assignment fixture rejects without repair/thread effects. Host overload preserves confirmation priority. Successful provider repair is not claimed by this narrow fixture | +| restore routes → `restore_runtime_worker_with_context` → `project_worker_restore_response_owned` | Existing authorized Host restore returns historical result; then whole stored Worker/links/Workdir/capability projection | Whole independent DB stage. A result with no Worker remains a pure response and needs no DB admission | Real SQL writer/shared-mutex hold; timer and independent Host control progress; bounded overload; cancellation retains actual DB lease; historical state preserved and no registry observation write. This tests post-restore policy, not a complete restore endpoint/E2E | +| Ticket detail/show/query and alias `get_ticket`; edit/event detail and assignment preparation | `TicketAuthority` projection includes open-MR `RuntimeTicketSourceCommitResolver` → synchronous provider HTTP | Mixed stages use finite Host, not sole DB. Subsequent pure assignment mutation stages keep their existing owner | Detail/query held loopback `/v1/repository-refs/observe`; unrelated DB/timer/control progress; cancel retains actual Host budget; overload stays 503. Provider-placement fixture is explicitly unsigned, not proof of signed request authority | +| Ticket tracking/case decision authority | Retained assignment/CAS/body/event checks plus `find_workspace_orchestrator` → live `RuntimeRegistry::worker` | Whole existing decision helper on finite Host; transaction and notification order unchanged | Held loopback live Orchestrator lookup for both tracking/case; independent SQL/timer/control; cancelled actual Host lease retained; failed live authority leaves Ticket unchanged. Existing non-assignee/browser bypass denial passes | +| Browser Ready | Existing mutation/notifications, followed by detail (including source ref observation) | Mutation and detail use one accepted Host job and explicit `execute_ticket_rest_operation_on_host`, never a second Host admission | Fake promptly denies notification GET, holds the subsequent source-ref POST; exactly one Ready event/readiness is durable before held detail; SQL/timer/control progress and cancelled Host ownership persist. Rejected fixture setup/misidentified notification GET are excluded | +| Worker control input/cancel/stop/restore source preflight | Synchronous live source Runtime lookup precedes `resolve_async` and operation admission | `authenticate_worker_mutation_source_owned` admits finite Host; already-owned synchronous callers retain the original helper | Actual control handler's held source GET proves DB/timer/control progress and cancelled finite Host budget; saturation returns availability, not source denial. The fake's lookup fails before an actual Worker cancellation; no product process is used | +| Worker observation capture | Source lookup, initial SQL grant, control lock, live grant re-fence and target Runtime lookup, awaited cold protocol connection | Source on Host; initial grant on DB; actual re-fence/live lookup on Host owns the acquired control guard; cold protocol signing retains existing DB stage | Source/permission/cold-signing contracts remain covered separately. Full capture success/held-protocol composition is not claimed by the new source lookup fixture | + +## Regressions and causal checks + +New `server_residual_execution_tests.rs` contains eight focused tests, including +three read/Ready and two decision-authority table cases. Exact old methods or the +old direct stored-projection placement were restored diagnostically in turn for +cancel, repair, restore, detail, tracking, and control. All six fail at their expected +first-poll/shared-mutex/independent-SQL boundary. Five-second fixture watchdog +release and its induced SQLite BUSY/panic are diagnostic bailout, not progress. +The product source is restored in `finally` after each case. + +Artifacts (local, not tracked product code): +- `target/t748-residual-old-{cancel,repair,restore,detail,tracking,control}-red.log` +- `target/t748-residual-final-restored-green.log`: **26 DB-boundary tests pass** +- `target/t748-residual-decision-policy.log`: non-assignee/browser bypass policy passes +- `target/t748-residual-root-check.log`: root compile closure passes +- `target/t748-residual-provisional-full-suite.log`: original default-parallel Server + **979 passed / 0 failed / 1 existing ignored**, binary11; **178.38 seconds**. + Only the three deliberate executor unknown-outcome panic diagnostics appeared. + No retained test/repro processes were observed after completion. +- `target/t748-residual-provisional-workdir-{default,http}.log`: default153 and + http-client156 tests pass. This does not complete the pending transitive audit. +- fmt/diff and state-contract names **558 / 0 unapproved / 0 stale** pass. + +All Cargo validation uses `target/t748-candidate-build`; no parallelism reduction, +production budget increase, skipped tests or mutation retry was introduced. +Initial test-only private enum/state/DTO mistakes failed compilation; missing +repository seeding and Ready notification GET confusion were diagnosed and fixed. +Those attempts are not semantic proof. The full-suite result above is provisional: +it does **not** conclude the caller inventory or approval. + +## Remaining inventory (not complete) + +`target/t748-async-entry-candidates.tsv` records 661 lexical async-entry candidates +from Server, Worker operations, auth, projection and subscription sources. Visible +lane markers/await counts are discovery hints, not an AST/type-checked call graph +or proof that all transitive operations are safe. Forwarders, actual handlers, +service adapters and callback/background callers must be matched by name and +traced through their real dependencies before marking a family inspected. + +| Inventory work | Current status / next evidence | +|---|---| +| Ticket/MR/Objective/Memory/Subjektiv/Backend Job: canonical/alias/browser/Runtime entries, assignment/routing/decision/projection/publication and callback edges | First committed child result inspected. Major handler owner table returned, but follow-up final result was not observable. Parent released the child and is directly checking remaining adapters/forwarders/launch edges. Macro trait dispatch and representative Objective/Memory/Ticket forwarding plus legacy Memory/cleanup owners inspected; complete per-operation mapping remains open | +| Auth/Runtime/Worker/control/observation/resources/Workdir/repository/Drive/settings/config/catalog/uploads/sessions/subscriptions and lifecycle background edges | First committed child result inspected; parent verified the residual structures below against actual source. The follow-up result was not observable after two Idle notifications; parent released the read-only child and directly traced the remaining four edge groups below. This is not delegated completion evidence | +| Existing pure DB, mixed Host, mandatory Subject callback and accepted publication boundaries | Earlier documented evidence remains relevant; unchanged code does not exempt newly found composed callers from tracing | +| `WorkspaceWorker::stop` first Host result → waiter-owned session close → second Host stage | Parent verified the split in actual source. Structural ownership gap, not yet a cancellation reproducer; tracked below rather than silently marked safe | + +## Additional residuals found before republication (unfixed) + +The first two read-only audit results were inspected through their committed +sessions. The parent then read the concrete handler/helper source listed below. +These are **static execution-placement/continuation findings**, not new successful +endpoint, cancellation or overload tests. None has been repaired or published in +this snapshot. The provisional suite above therefore cannot establish safety for +these families. Lower recovery/transport edges are still being traced by the +parent after releasing the read-only children; this is not independent review. + +`S` below means `crates/workspace-server/src/server.rs` at the frozen hash; +`O` means `crates/workspace-server/src/server/worker_operations.rs`. + +| Entry / family | Entry → authorization → synchronous work → completion | Verified residual and required boundary | +|---|---|---| +| `repository_access_projection`, `repository_ssh_credential_{list,create,generate,get,delete,public_key,rotate}`, `repository_ssh_host_trust_{list,put,get,delete}` | ServerApi entries `S:7879–8107` directly await handlers `S:12934–13129`; owner authentication is awaited first, then active config projection / secret / trust / CAS / audit helpers execute synchronously | Auth ownership does not carry past await. Parent read all handlers and actual `repository_access.rs:391–436` connection reads. Whole existing synchronous operation needs an owner; pure DB and any external provider work must be classified by dependency, not by the SSH label | +| `repository_create` → `scoped_create_repository` | Owner authentication await → validation → `config_store.insert_repository` → intent/replay classification → response, `S:23157–23214` | Insert is outside an accepted job. Preserve the existing intact insert/intent/commit helper and successful replay vs conflicting intent semantics | +| `repository_ssh_connection_probe`, `repository_ssh_host_trust_confirm` | Owner and active binding awaits → catalog / `probe_repository_ssh_connection` → Runtime observation → trust read/CAS/write → response, `S:23247–23358` | Post-auth mixed work has no finite Host owner. Confirm must keep observed host-key selection and fingerprint comparison before the existing trust mutation | +| `worker_session`, `worker_session_history`, `scoped_worker_session_attachment` | Workspace checks → synchronous Runtime Session API → response/body, `S:8696–8769,12011–12069`; registry facade `hosts.rs:2129–2195` | No finite Host admission at these request boundaries. Binding/signing and remote transport dependencies must stay off Tokio and out of the sole DB lane; preserve Session diagnostics and response limits | +| `runtime_worker_attachment_upload` → `upload_runtime_worker_file_contract` | Grant claim → registry resolution → Runtime upload → grant completion or cancellation artifact deletion, `S:28081–28152` | Whole body-upload operation is still direct synchronous mixed work, distinct from the already-owned grant/cancel/delete routes. Admission must precede grant claim; accepted upload must own completion and cancellation cleanup | +| Drive Worker authorization / `drive_grant_create` | `server_drive_grants.rs:54–74,126–166`: source/header checks → `current_worker_identity` or live Runtime worker → later DB grant precheck/mutation | Pre-DB source/live Runtime lookup has no Host owner. `S:18244–18292` confirms `current_worker_identity` directly reaches synchronous `runtime.worker`. Retain no-actor-fallback policy; owner unavailability must not be misreported as authority denial | +| `current_worker_workspace_config_{get,attach,observe,read,commit}` | `S:11625–11699` directly invokes `workspace_config::identity`; `server_workspace_config.rs:65–74` reaches the same synchronous Runtime source helper before later owned DB stages | All five logical-config entries share the unowned identity preflight. Later accepted persistence/publication does not cover this earlier stage | +| `workspace_workdir_list` → `scoped_list_working_directories` | DB job `S:25943–25959` → summaries `32974–32999` → all Runtime workdir observations `33515–33524` → list/detail Runtime calls and SQL reconciliation `33421–33467` | Mixed Runtime/SQL helper is incorrectly on the sole DB worker. Preserve live reconciliation and diagnostics, but move the whole mixed stage to finite Host rather than pretending the list is stored-only | +| `workspace_config_tree_commit` → `scoped_commit_workspace_config_tree` | DB job `S:13444–13460` → prepare → repository-access projection → config commit → Runtime prompt publication, `13498–13517`; remote publication `hosts.rs:5090–5099` | SQL commit is followed by synchronous network publication inside sole DB job. Keep commit-before-best-effort-publication and accepted ownership; do not leave the continuation on the waiter. Logical config's separate owned Host dispatch is not this UI path | +| `repository_{list,detail,log}` and aliases | DB jobs `S:29108–29183` → live repository reader → summary/log provider; `repositories.rs:276–375` invokes Git subprocess for materialized local repositories | Provider work is not pure DB. Mixed catalog/provider work needs finite Host, without changing existing diagnostics for unmaterialized sources | +| `worker_control_stop`, `runtime_worker_stop`, `runtime_worker_stop_alias` → `WorkspaceWorker::stop` | `O:379–425`: grant/lock/re-fence → Host #1 Runtime stop returns result/control guard → waiter session lock/close → Host #2 linked Workdir observation/SQL | Structural ownership gap confirmed by parent: accepted Runtime stop has no owner for session close and subsequent Workdir synchronization if its waiter vanishes. `S:18609–18619` awaits real session close; the second Host admission also occurs after side effects. Whole accepted stop must own this continuation without nested Host acquisition. Cancellation reproduction remains pending | + +### Parent takeover of the four remaining Runtime edge groups + +The Runtime child's first result remains useful, but its follow-up final result +was not available through authorized observation. The parent did not infer +completion from Idle and did not loop Submit or inspect raw session storage. +See `docs/report/2026-10-12-worker-observation-followup-gap.md` for the bounded +observations and handling. The parent read the following actual source directly: + +| Edge | Source and actual stages | Classification / unresolved contract | +|---|---|---| +| Embedded protocol connect and method pump | `S:31845–31902` directly calls synchronous `attach_worker_protocol_scoped` before spawning its async pump; the pump directly calls `send_connected_protocol_method_scoped`. `worker-runtime/src/runtime.rs:3486–3598` takes synchronous operation/state locks; Shutdown performs `begin_stop_operation` and `commit_stopped_worker` (`4359–4445`), with durable record/snapshot writes in fs-store builds (`5256–5269`) | **Additional confirmed placement residual**: Backend async connect/pump is not an accepted finite Host owner. The existing endpoint identity/settled-operation checks and commit/publication helpers must remain intact; no Runtime-store refactor is needed | +| Workdir creation reservation → Runtime result → finalization | `S:26316–26609`: DB preparation/reservation, Host runtime selection/observation/create, then waiter-admitted failure or final registry/receipt DB jobs. Same-id reentry observes the reserved Runtime Workdir before attempting create (`26473–26517`) | **Confirmed continuation split**, with durable same-operation recovery intent, not automatic mutation retry. Existing recovery does not make the original create job own finalization after its waiter disappears. Final classification/regression must cover accepted Runtime outcome, final receipt, cancellation and overload without creating a second provider operation | +| External grant fence → attachment/provider cleanup | `S:25595–25709`: DB authority/read or revoke/fence/audit, then waiter-owned cleanup/provider stop; terminal GET/revoke explicitly retries cleanup. Cleanup itself commits pending, awaits release, then admits completion/audit DB (`25719–25799`). Expiry task retains its async owner and retries (`24790–24854`) | **Confirmed continuation split plus explicit recovery**, not a claim that revocation failed or is unsafe after its DB fence. Grants remain denied by their committed fence. Need causal cancellation/lease evidence for cleanup and preserved retry/expiry policy before choosing the smallest ownership fix | +| Command provider acceptance → registration/audit | `S:19507–19631`: Host identity, DB attachment fence, async session/provider, then synchronous command-handle registration. External `operate` admits audit-requested DB, sends provider request with explicit cancel guard, receives result, then admits audit-completed DB (`888–1024`); External command-handle registration follows another await (`1372–1428`). Local command start retains Runtime resources in its actual task and owns terminal-close admission (`workdir/src/local.rs:1831–1897`) | **Partially protected / continuation question**. SQL jobs are owned and local actual command resources are retained; explicit external cancel is not SQL retry. Provider acknowledgement-to-registration can cross a cancellable audit await. Do not assert an orphan process or whole command success without a focused protocol/registration reproducer; do not rewrite external cancellation semantics speculatively | +| Catalog and single-workspace shutdown | `S:12222–12279`: catalog waits for HTTP serve, drains Worker projection and owns Feature shutdown DB (`2651–2664`); single-workspace `serve` aborts its hook but calls synchronous Feature shutdown directly. `feature-storage/src/lib.rs:220–241,521–523` fences and waits on connection close. Executor `drain` (`db_executor.rs:290–315`) covers only jobs accepted before its first poll, not later submissions | **Additional confirmed direct shutdown placement residual** in single-workspace serve. Separately, catalog does not demonstrate hook/expiry/Host/cleanup producer quiescence before storage shutdown. Whole producer/drain ordering remains a lifecycle question; no live shutdown was invoked to test it | + +Domain generated adapters, remaining Tool/WIP forwarding, Backend Job / legacy +Memory launch lower edges remain under follow-up inspection. Existing domain SQL +helpers are not being reimplemented or globally refactored; ownership tracing is +separate from re-auditing every SQL semantic branch. None of the additional rows +above has new executable regression evidence yet. + + +T743's additional assignment page/diagnostics/answer APIs are absent from this +source. Ticket seq8–9 identifies their separate owner's normal composition with +the published T748 facade. This inventory is **not** an attestation of that future +combined source; no duplicate API or combined approval is introduced here. + +## Republication conditions + +Review-ready requires a consolidated caller-name list with each entry's auth, +SQL/provider/Runtime stage, post-await projection, post-commit publication and +owner identified; confirmed residuals repaired with focused causal regressions; +remaining uncertainty explicit; and final root/changed-crate/affected-feature, +format/diff/name checks on that final source. Only then normal same-selector push, +provider exact-HEAD verification and fresh trusted independent review of this MR. +No new Reviewer Runtime or separate Coder is created. Read-only audit children do +not grant review verdict or integration authority. + +## Scoped caller repair and validation history + +The same Ticket branch is retained for advancing the existing MR. This section +records validation history, not approval or integration. The final-validation +section below supersedes the intervening failed runs without deleting their evidence. +The existing domain helpers, +authorization fences, receipt/replay rules, and transaction/publication order +remain intact. No automatic mutation retry, production limit increase, nested +Host admission, or test parallelism reduction was introduced. + +| Repaired family | Current owner / executable evidence | +|---|---| +| Repository access/SSH credential and host-trust CRUD, Repository registration | Whole post-owner-auth synchronous operation on DB. Registration regression observes the **config store's actual connection** waiting behind a separate SQLite writer, then cancels the waiter; after drain exactly one Repository exists and the same registration intent returns HTTP200/replayed without another row | +| SSH probe/host-trust confirm | Whole post-owner/binding-auth mixed probe/observed-key selection/fingerprint comparison/trust mutation on Host. Existing narrow SSH/trust policy tests pass in the full Server suite; no successful real SSH connection or host-key exchange is claimed | +| Repository list/detail/log and aliases | Live Git/catalog helpers on Host, not DB. With DB admission saturated, empty catalog and absent-key detail/log retain their ordinary success/404 responses. This regression proves lane selection, not held Git subprocess behavior | +| Worker Session/history/attachment | Typed Session service and attachment handler admit Host before synchronous Runtime observation. Loopback held Runtime lookup proves independent SQL/timer/control progress, cancelled actual lease retention, bounded overload and release after actual drain | +| Body upload | Host admission precedes the upload grant claim and owns resolution/upload/completion/cancellation cleanup. The same loopback table holds the Runtime lookup after claim; cancelled waiter leaves actual owner, and provider refusal completes the claim back to Pending. Successful artifact upload/delete and established remote sessions are not exercised by this refusal fixture | +| Drive Worker authorization / grant create; all five logical-config identity entries | Live source/target preflight uses Host; later DB authorization and mutations retain their previous transaction owner. Three held-lookup table cases prove independent SQL/timer/control, cancelled lease retention, and typed HTTP503 availability (not false authority denial) when Host admission is full. Existing no-actor-fallback and source-fence policies remain in the suite | +| Workdir list | Whole live Runtime reconciliation/SQL helper on Host. Held Runtime list lookup in the Session/Workdir/upload table proves sole DB remains free and cancellation retains the actual Host budget | +| UI config commit | Whole existing evaluate/project/commit/best-effort prompt publication helper on Host. Loopback held prompt POST confirms the changed tree is already durable while independent DB/timer/control proceeds. Cancelling the waiter retains publication ownership; refused publication neither repeats nor rolls back the commit | +| Worker stop canonical/alias/control entries | One accepted Host workflow owns Runtime stop, session fence/close, stored read and linked Workdir synchronization. An in-process Runtime emits its stop receipt while a real session fence is held; after waiter abort and fence release the registered session is removed. Restoring the exact old stop method makes this test fail at the missing cleanup assertion; restoring the fix passes. No real Worker/product process is spawned or stopped. Linked Workdir provider success is not claimed by this session-only fixture | +| Embedded protocol connect/method; single-workspace Feature shutdown | `4d981194` owns these operations on the existing finite Host/DB lanes. Focused protocol and actual Feature connection shutdown regressions remain green in the final suite | + +Final logs in the Workdir (build artifacts, not committed product content): +- `target/t748-stop-old-placement-red.log`: old stop fails **missing cleanup**, not timeout. +- `target/t748-stop-restored-green.log`: restored accepted-stop ownership passes. +- `target/t748-caller-host-green.log`: Session/history/attachment/Workdir/upload table passes. +- `target/t748-repository-create-green.log`: actual SQLite commit/cancel/replay passes. +- `target/t748-drive-config-preflight-green.log`: all three live authority cases pass. +- `target/t748-ui-prompt-owner-green.log`: commit-before-publication passes. +- `target/t748-repository-provider-green.log`: all three Repository read lanes pass. +- `target/t748-caller-final-root-check.log`: workspace-root `cargo check` passes. +- `target/t748-caller-final-server-suite.log`: original default-parallel Server + **988 passed, 0 failed, 1 existing ignored**, plus **11 binary tests**, 185.38s. +- `target/t748-caller-final-workdir-default.log`: **153** tests pass across targets. +- `target/t748-caller-final-workdir-http.log`: **156** tests pass across targets. +- fmt/diff checks pass; `target/t748-caller-final-names.log`: **558 / 0 unapproved / 0 stale**. + +Test setup errors are excluded from semantic evidence: the first stop fixture +omitted the Runtime's live Worker summary; the Repository busy observer initially +watched the general store rather than the config store's distinct connection. +Both were corrected before the successful runs. Missing package/Python command +and compile-time DTO/Duration/Drive-error mistakes also prove no product behavior. + +### Explicit limits, not an expanding audit gate + +The four lower-edge rows above separate confirmed direct placement repairs from +Workdir create / external grant cleanup / provider acknowledgement continuation +questions and catalog producer quiescence. The latter are not redesigned here, +per the user's instruction not to expand speculative Workdir/grant or unrelated +Feature work indefinitely. Their existing durable recovery, explicit cleanup +retry and provider cancellation policy are preserved, not reclassified as +end-to-end cancellation proof. This report is not a mechanically complete +transitive call graph or an assertion that every future/composed caller is safe. +T743's separate new API composition still requires its owner's exact combined +source validation/review. None of these limits constitutes Ticket conclusion, +MR approval, target integration or permission to update the running environment. + +### Post-suite focused validation blocker + +After the successful full suite above, the current candidate was checked with +`cargo test -p yoi-workspace-server --lib db_execution_tests` using the same +candidate target directory and ordinary parallel execution. Result: **31 passed / +1 failed**, in 8.06s. Failure: +`cancelled_runtime_key_revoke_commits_audit_and_removes_broker_registration` +reported the shared `observed` helper's two-second `Elapsed(())` timeout, without +identifying which of its several lifecycle waits timed out. Source inspection +confirms projection shutdown is already present in this fixture; it is not +legitimate to attribute the failure to omitted projection shutdown, treat it as +load-only, increase limits, reduce parallelism, or rerun until green. + +Evidence is preserved in `target/t748-caller-final-boundaries.log`. The prior +full-suite success is real but insufficient to declare final validation complete. +The new caller and stop cases all passed in this focused run. Next diagnosis must +identify the exact wait and inspect actual HTTP/job/broker ownership events before +choosing a fixture or product repair. No normal push, fresh review, approval, +integration or Ticket conclusion was performed for this candidate. + +## Continued diagnosis and final validation + +Returning the failed test as a stop condition was a workflow mistake, not an +external outage, lost authority, or additional audit approval gate. Work continued +on the same branch. The failed run above is retained as evidence. + +The `observed` helper now reports its caller location. A diagnostic 32-case run +passed; that result alone was **not** treated as a diagnosis or used to erase the +failure. Source tracing identified an uncontrolled cold broker signing producer +in the revoke fixture. The test now receives the actual signed WebSocket +handshake and drains its completed DB work **before** holding the revoke writer. +The handshake remains pending, so broker shutdown is still tested. An early HTTP +response is selected against actual SQLite entry, reporting status/body rather +than waiting for an event that can no longer occur. The original failed run did +not identify its exact wait; attributing it conclusively to signing would exceed +the captured evidence. + +Subsequent full runs exposed the Ready table's distinct failure **before** source +POST arrival (caller location, not timer/control failure). Artifacts: +`target/t748-post-fixture-server-suite.log` and +`target/t748-final-framing-server-suite.log`. The HTTP fake previously assumed +one TCP read contained the request line and answered without consuming a body. +It now parses the complete header/body; a deterministic two-byte-fragment reader +proves complete frame consumption and truncated-body rejection. This is a real +fixture flaw but does not by itself establish the historical timeout cause. + +Ready setup includes mutation and best-effort notification before the source +POST. The held-provider arrival now uses the existing five-second fixture +watchdog as its setup bound, selected against handler completion. The subsequent +**timer and independent SQL/control observations retain their two-second budget**; +actual held-request cancellation/lease/overload/cleanup assertions are unchanged. +Production admission limits, normal test parallelism, and provider timeout are +unchanged. This setup-bound change is explicit, not a hidden limit increase or +claim that Ready completes within two seconds under a whole-suite workload. +Restoring the exact published old Ready method still fails this revised test +(`target/t748-ready-old-boundary-red.log`, Ready returns only after synchronous +provider wait); restored code passes, so the offload distinction is not weakened. + +Final code validation after these fixture changes: +- `target/t748-boundary-setup-fixed-green.log`: **33 DB-boundary cases pass**. +- `target/t748-final-setup-root-check.log`: root compile closure passes. +- `target/t748-final-setup-server-suite.log`: original default-parallel Server + **989 passed / 0 failed / 1 existing ignored**, **11 binary tests**, 189.66s. +- `target/t748-final-setup-workdir-default.log`: **153 pass** across targets. +- `target/t748-final-setup-workdir-http.log`: **156 pass** across targets. +- `cargo fmt --all -- --check`, `git diff --check HEAD`, and state-contract names + **558 / 0 unapproved / 0 stale** pass. + +No failed semantic run was merely repeated until green: the intervening source +changes and the limits of each diagnosis are recorded above. These results support +publication for fresh exact-source independent review, not reviewer approval, +MR integration, Ticket conclusion, or updates to the live dogfooding environment. diff --git a/docs/report/2026-10-12-t748-suite-diagnosis.md b/docs/report/2026-10-12-t748-suite-diagnosis.md index af3f9a9c1..ed594bde6 100644 --- a/docs/report/2026-10-12-t748-suite-diagnosis.md +++ b/docs/report/2026-10-12-t748-suite-diagnosis.md @@ -430,8 +430,12 @@ panic diagnostics appeared, and no retained test/repro processes were observed. Workdir default153/http-client156, fmt/diff and naming558/0/0 passed. All Cargo checks/tests use the isolated candidate cache. Logs: `target/t748-revoke-{root-check,full-suite,workdir-default,workdir-http,fmt,names}.log`. -The following documentation-only publication changes no tested code and needs a -fresh exact-source independent verdict before an approved handoff. +Docs-only source `ebce7ab7` changed no tested code. MR seq13 independently confirmed +the revoke ownership fix and repeated the full checks, but requested changes for +Ticket cancellation, selector repair and post-restore projection's async callers. +The current uncommitted follow-up and the requested comprehensive caller audit +are tracked in [the caller inventory](2026-10-12-t748-caller-inventory.md). +Successful suites are not evidence that uninspected caller boundaries are safe. No latency benchmark, production incident measurement, target integration, Ticket conclusion, or running-environment update is claimed. T743 has now diff --git a/docs/report/2026-10-12-worker-observation-followup-gap.md b/docs/report/2026-10-12-worker-observation-followup-gap.md new file mode 100644 index 000000000..fe5ef448a --- /dev/null +++ b/docs/report/2026-10-12-worker-observation-followup-gap.md @@ -0,0 +1,49 @@ +# Internal SubWorker follow-up observation gap + +## Observed barrier + +During T748's read-only caller inventory, the parent could read the first Runtime +inventory assistant result by stable entry ref +`E01a12e76-3893-70a0-bb73-28ee0c52f87b`. It then sent a follow-up to the same +Idle direct child, `t748-runtime-caller-inventory`, to close four remaining +execution/ownership boundaries. + +After a turn-ended/Idle notification, `WorkerList` confirmed Idle, but bounded +`SearchSessionEntries` for assistant entries after follow-up user ref +`E01a12e76-d696-73d3-80d7-b8cde9935b37` returned no entries. Tool observations +were visible. The overview showed only the initial and first follow-up user +anchors, not a final result. A single additional Submit asking for the existing +findings (not re-investigation) returned success. After its turn-ended/Idle +notification, the same bounded assistant query still returned no entries, and the +overview still ended at the first follow-up user anchor. + +This is an observation-surface symptom, **not proof** that an assistant result +was generated, lost, or that the underlying investigation completed. No raw +session storage or hidden worker state was read to bypass the projection. + +The same symptom also occurred for `t748-domain-caller-inventory`: its first +result `E01a12e73-7ae4-7f02-9a7e-203140971635` was readable, but after the +follow-up turn-ended/Idle notification the overview ended at the follow-up's +starting commentary `E01a12e75-9c19-7bd0-9284-299116a05761` and intervening tool +records. No final follow-up result was visible. The parent released that +read-only delegation without another Submit and took over the remaining generated +adapter/forwarder and launch lower-edge checks. Neither `WorkerStop` tool outcome +was treated as evidence that the investigation completed. + +## Handling + +The parent did not mark the delegated work or Ticket complete, publish source, +or replace independent review with the notification. It stopped only the two +parent-owned read-only children to release delegation and took over the open +Runtime and domain boundaries. No live product process, parent Worker, target +branch, or attachment was stopped or cleaned. + +## Improvement proposal + +Expose enough committed turn metadata through the authorized observation surface +to distinguish a finalized assistant result, a turn ending without a result, and +an observation capture not yet including an accepted Submit. An explicit +completion marker tied to the visible Submit entry would prevent an Idle +notification from being confused with inspectable evidence. Preserve append-only +session entries and the current authorization projection; do not expose raw +prompts/reasoning or require polling/retry loops as a workaround.