diff --git a/src/auth/tests.rs b/src/auth/tests.rs index 4fc5aecc..80fa3e9d 100644 --- a/src/auth/tests.rs +++ b/src/auth/tests.rs @@ -351,25 +351,18 @@ async fn gate_send( } #[tokio::test] -async fn gate_agent_token_can_call_agent_transport_register() { +async fn gate_token_class_route_matrix_preserves_authority_boundaries() { let config = gate_test_config(Some("secret")); let (_tmp, db) = gate_test_db(); let user = gate_seed_user(&db, "alice"); let agent_token = gate_mint_agent_token(&db, &user, "alice-laptop"); + let user_token = gate_mint_user_token(&db, &user); let service = Service::new(gate_router(config, db)); - // /api/shell/agent/register is an allowed transport path. + let (status, body) = gate_send(&service, "/api/shell/agent/register", Some(&agent_token)).await; - assert_eq!(status, salvo::http::StatusCode::OK, "body: {:?}", body); + assert_eq!(status, salvo::http::StatusCode::OK, "body: {body:?}"); assert_eq!(body["ok"], true); -} -#[tokio::test] -async fn gate_agent_token_cannot_call_non_transport_paths() { - let config = gate_test_config(Some("secret")); - let (_tmp, db) = gate_test_db(); - let user = gate_seed_user(&db, "alice"); - let agent_token = gate_mint_agent_token(&db, &user, "alice-laptop"); - let service = Service::new(gate_router(config, db)); for path in [ "/api/runtime/status", "/api/tools/list", @@ -382,49 +375,39 @@ async fn gate_agent_token_cannot_call_non_transport_paths() { assert_eq!( status, salvo::http::StatusCode::FORBIDDEN, - "agent token should be forbidden on {}: {:?}", - path, - body + "agent token should be forbidden on {path}: {body:?}" ); + if path == "/api/runtime/status" { + assert!(body["error"] + .as_str() + .unwrap_or("") + .contains("agent tokens are only allowed")); + } } - // Verify the error message is descriptive for at least one path. - let (_, body) = gate_send(&service, "/api/runtime/status", Some(&agent_token)).await; - assert!( - body["error"] - .as_str() - .unwrap_or("") - .contains("agent tokens are only allowed"), - "body: {:?}", - body - ); -} -#[tokio::test] -async fn gate_user_token_can_call_normal_apis() { - let config = gate_test_config(Some("secret")); - let (_tmp, db) = gate_test_db(); - let user = gate_seed_user(&db, "alice"); - let user_token = gate_mint_user_token(&db, &user); - let service = Service::new(gate_router(config, db)); - // User tokens must still reach normal runtime/project APIs. for path in [ "/api/runtime/status", "/api/tools/list", "/api/projects/list", ] { let (status, body) = gate_send(&service, path, Some(&user_token)).await; + assert_eq!(status, salvo::http::StatusCode::OK, "{path}: {body:?}"); + } + + for path in [ + "/api/runtime/status", + "/api/tools/list", + "/api/projects/list", + "/api/shell/agent/register", + "/api/agent-tokens/list", + ] { + let (status, body) = gate_send(&service, path, Some("secret")).await; assert_eq!( status, salvo::http::StatusCode::OK, - "{} body: {:?}", - path, - body + "bootstrap should reach {path}: {body:?}" ); } - // And must NOT reach agent transport endpoints (enforced per-handler in - // Phase 3, but here the central gate lets them through; the per-handler - // agent transport check rejects them). For this gate test we only - // assert the central gate does not block user tokens on normal APIs. } #[test] @@ -515,63 +498,25 @@ async fn gate_disabled_user_account_credential_is_rejected() { } #[tokio::test] -async fn query_token_is_rejected_on_runtime_status() { +async fn gate_query_token_is_websocket_only_and_bearer_header_stays_general() { let _env = crate::auth::AuthEnvGuard::auth_required(); let config = gate_test_config(Some("secret")); let (_tmp, db) = gate_test_db(); let service = Service::new(gate_router(config, db)); - let mut resp = TestClient::post("http://localhost/api/runtime/status?token=secret") - .send(&service) - .await; - assert_eq!(gate_status(&resp), salvo::http::StatusCode::UNAUTHORIZED); - let body = resp.take_json::().await.unwrap(); + + let (status, body) = gate_send(&service, "/api/runtime/status?token=secret", None).await; + assert_eq!(status, salvo::http::StatusCode::UNAUTHORIZED); assert_eq!(body["error"], "Unauthorized"); -} -#[tokio::test] -async fn query_token_still_works_for_agent_websocket_path() { - let config = gate_test_config(Some("secret")); - let (_tmp, db) = gate_test_db(); - let service = Service::new(gate_router(config, db)); let (status, body) = gate_send(&service, "/api/agents/ws?token=secret", None).await; - assert_eq!(status, salvo::http::StatusCode::OK, "body: {:?}", body); + assert_eq!(status, salvo::http::StatusCode::OK, "body: {body:?}"); assert_eq!(body["ok"], true); -} -#[tokio::test] -async fn authorization_header_still_works_on_runtime_status() { - let config = gate_test_config(Some("secret")); - let (_tmp, db) = gate_test_db(); - let service = Service::new(gate_router(config, db)); let (status, body) = gate_send(&service, "/api/runtime/status", Some("secret")).await; - assert_eq!(status, salvo::http::StatusCode::OK, "body: {:?}", body); + assert_eq!(status, salvo::http::StatusCode::OK, "body: {body:?}"); assert_eq!(body["ok"], true); } -#[tokio::test] -async fn gate_bootstrap_can_call_all_apis() { - let config = gate_test_config(Some("secret")); - let (_tmp, db) = gate_test_db(); - let service = Service::new(gate_router(config, db)); - // Bootstrap reaches normal APIs and agent transport paths alike. - for path in [ - "/api/runtime/status", - "/api/tools/list", - "/api/projects/list", - "/api/shell/agent/register", - "/api/agent-tokens/list", - ] { - let (status, body) = gate_send(&service, path, Some("secret")).await; - assert_eq!( - status, - salvo::http::StatusCode::OK, - "{} body: {:?}", - path, - body - ); - } -} - #[tokio::test] async fn gate_forbidden_response_is_json_not_html() { let config = gate_test_config(Some("secret")); @@ -933,85 +878,97 @@ async fn oauth2_verifier_rejects_invalid_subject_combinations() { } #[tokio::test] -async fn oauth2_verifier_rejects_unknown_access_token() { - let config = gate_test_config_oauth2(Some("secret")); - let (_tmp, db) = gate_test_db(); - - let verifier = OAuth2Verifier; - let result = verifier - .verify(&config, Some(&db), "wc_oat_nonexistenttoken") - .await; - assert!(result.is_err(), "unknown access token should return Err"); -} - -#[tokio::test] -async fn oauth2_verifier_rejects_expired_access_token() { - let config = gate_test_config_oauth2(Some("secret")); - let (_tmp, db) = gate_test_db(); - let user = gate_seed_user(&db, "alice"); - let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); - - // Create an expired access token. - let now = chrono::Utc::now().timestamp(); - let plaintext = generate_oauth_access_token(); - let token_hash = hash_token(&plaintext); - let record = crate::models::OAuthAccessTokenRecord { - id: uuid::Uuid::new_v4().to_string(), - token_hash, - client_id: client.client_id.clone(), - subject_kind: "managed_user".to_string(), - subject_id: user.id.clone(), - user_id: Some(user.id.clone()), - scopes: "runtime:read".to_string(), - resource: None, - shared_key_hash: None, - created_at: now - 7200, - expires_at: now - 1, // already expired - revoked_at: None, - last_used_at: None, - }; - db.insert_oauth_access_token(&record).unwrap(); - - let verifier = OAuth2Verifier; - let result = verifier.verify(&config, Some(&db), &plaintext).await; - assert!(result.is_err(), "expired access token should return Err"); -} - -#[tokio::test] -async fn oauth2_verifier_rejects_revoked_access_token() { - let config = gate_test_config_oauth2(Some("secret")); - let (_tmp, db) = gate_test_db(); - let user = gate_seed_user(&db, "alice"); - let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); - let (at, plaintext) = gate_seed_oauth_access_token(&db, &client, &user, "runtime:read"); - - // Revoke the token. - let now = chrono::Utc::now().timestamp(); - db.revoke_oauth_access_token(&at.id, now).unwrap(); +async fn oauth2_verifier_rejects_invalid_access_token_state_matrix() { + enum InvalidAccessToken { + Unknown, + Expired, + Revoked, + RevokedClient, + DisabledUser, + } - let verifier = OAuth2Verifier; - let result = verifier.verify(&config, Some(&db), &plaintext).await; - assert!(result.is_err(), "revoked access token should return Err"); + for (label, case) in [ + ("unknown", InvalidAccessToken::Unknown), + ("expired", InvalidAccessToken::Expired), + ("revoked", InvalidAccessToken::Revoked), + ("revoked client", InvalidAccessToken::RevokedClient), + ("disabled user", InvalidAccessToken::DisabledUser), + ] { + let config = gate_test_config_oauth2(Some("secret")); + let (_tmp, db) = gate_test_db(); + let plaintext = match case { + InvalidAccessToken::Unknown => "wc_oat_nonexistenttoken".to_string(), + InvalidAccessToken::Expired => { + let user = gate_seed_user(&db, "alice"); + let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); + let now = chrono::Utc::now().timestamp(); + let plaintext = generate_oauth_access_token(); + db.insert_oauth_access_token(&crate::models::OAuthAccessTokenRecord { + id: uuid::Uuid::new_v4().to_string(), + token_hash: hash_token(&plaintext), + client_id: client.client_id, + subject_kind: "managed_user".to_string(), + subject_id: user.id.clone(), + user_id: Some(user.id), + scopes: "runtime:read".to_string(), + resource: None, + shared_key_hash: None, + created_at: now - 7200, + expires_at: now - 1, + revoked_at: None, + last_used_at: None, + }) + .unwrap(); + plaintext + } + InvalidAccessToken::Revoked => { + let user = gate_seed_user(&db, "alice"); + let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); + let (token, plaintext) = + gate_seed_oauth_access_token(&db, &client, &user, "runtime:read"); + db.revoke_oauth_access_token(&token.id, chrono::Utc::now().timestamp()) + .unwrap(); + plaintext + } + InvalidAccessToken::RevokedClient => { + let user = gate_seed_user(&db, "alice"); + let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); + let (_token, plaintext) = + gate_seed_oauth_access_token(&db, &client, &user, "runtime:read"); + db.revoke_oauth_client(&client.id, chrono::Utc::now().timestamp()) + .unwrap(); + plaintext + } + InvalidAccessToken::DisabledUser => { + let user = gate_seed_user(&db, "alice"); + let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); + let (_token, plaintext) = + gate_seed_oauth_access_token(&db, &client, &user, "runtime:read"); + db.set_user_disabled(&user.id, true, chrono::Utc::now().timestamp()) + .unwrap(); + plaintext + } + }; + let result = OAuth2Verifier.verify(&config, Some(&db), &plaintext).await; + assert!(result.is_err(), "{label} access token should return Err"); + } } #[tokio::test] -async fn oauth2_verifier_rejects_refresh_token() { +async fn oauth2_verifier_ignores_non_access_oauth_credential_kinds() { let config = gate_test_config_oauth2(Some("secret")); let (_tmp, db) = gate_test_db(); let user = gate_seed_user(&db, "alice"); let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); - - // Create a refresh token (wc_ort_*). let now = chrono::Utc::now().timestamp(); - let plaintext = generate_oauth_refresh_token(); - let token_hash = hash_token(&plaintext); - let record = crate::models::OAuthRefreshTokenRecord { + let refresh = generate_oauth_refresh_token(); + db.insert_oauth_refresh_token(&crate::models::OAuthRefreshTokenRecord { id: uuid::Uuid::new_v4().to_string(), - token_hash, + token_hash: hash_token(&refresh), client_id: client.client_id.clone(), subject_kind: "managed_user".to_string(), subject_id: user.id.clone(), - user_id: Some(user.id.clone()), + user_id: Some(user.id), scopes: "runtime:read".to_string(), resource: None, shared_key_hash: None, @@ -1020,46 +977,23 @@ async fn oauth2_verifier_rejects_refresh_token() { revoked_at: None, last_used_at: None, rotated_from_id: None, - }; - db.insert_oauth_refresh_token(&record).unwrap(); - - let verifier = OAuth2Verifier; - let result = verifier - .verify(&config, Some(&db), &plaintext) - .await - .unwrap(); - assert!( - result.is_none(), - "refresh token (wc_ort_*) should return None" - ); -} - -#[tokio::test] -async fn oauth2_verifier_rejects_authorization_code() { - let config = gate_test_config_oauth2(Some("secret")); - let verifier = OAuth2Verifier; - let result = verifier - .verify(&config, None, "wc_oac_sometoken") - .await - .unwrap(); - assert!( - result.is_none(), - "authorization code (wc_oac_*) should return None" - ); -} + }) + .unwrap(); -#[tokio::test] -async fn oauth2_verifier_rejects_client_secret() { - let config = gate_test_config_oauth2(Some("secret")); - let verifier = OAuth2Verifier; - let result = verifier - .verify(&config, None, "wc_csec_sometoken") - .await - .unwrap(); - assert!( - result.is_none(), - "client secret (wc_csec_*) should return None" - ); + for (label, token) in [ + ("refresh token", refresh.as_str()), + ("authorization code", "wc_oac_sometoken"), + ("client secret", "wc_csec_sometoken"), + ] { + let result = OAuth2Verifier + .verify(&config, Some(&db), token) + .await + .unwrap(); + assert!( + result.is_none(), + "{label} must not authenticate as an access token" + ); + } } #[tokio::test] @@ -1117,43 +1051,6 @@ async fn oauth2_verifier_does_not_update_last_used_on_failure() { // doesn't panic or succeed. } -#[tokio::test] -async fn oauth2_verifier_rejects_token_for_revoked_client() { - let config = gate_test_config_oauth2(Some("secret")); - let (_tmp, db) = gate_test_db(); - let user = gate_seed_user(&db, "alice"); - let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); - let (_at, plaintext) = gate_seed_oauth_access_token(&db, &client, &user, "runtime:read"); - - // Revoke the client. - let now = chrono::Utc::now().timestamp(); - db.revoke_oauth_client(&client.id, now).unwrap(); - - let verifier = OAuth2Verifier; - let result = verifier.verify(&config, Some(&db), &plaintext).await; - assert!( - result.is_err(), - "token for revoked client should return Err" - ); -} - -#[tokio::test] -async fn oauth2_verifier_rejects_token_for_disabled_user() { - let config = gate_test_config_oauth2(Some("secret")); - let (_tmp, db) = gate_test_db(); - let user = gate_seed_user(&db, "alice"); - let (client, _secret) = gate_seed_oauth_client(&db, &user, "Test App"); - let (_at, plaintext) = gate_seed_oauth_access_token(&db, &client, &user, "runtime:read"); - - // Disable the user. - let now = chrono::Utc::now().timestamp(); - db.set_user_disabled(&user.id, true, now).unwrap(); - - let verifier = OAuth2Verifier; - let result = verifier.verify(&config, Some(&db), &plaintext).await; - assert!(result.is_err(), "token for disabled user should return Err"); -} - #[tokio::test] async fn oauth2_verifier_accepts_current_project_share_and_preserves_project_identity() { let mut config = (*gate_test_config_oauth2(Some("secret"))).clone(); diff --git a/src/connector_runtime/execution_tests.rs b/src/connector_runtime/execution_tests.rs index 8c2f82b9..713639f3 100644 --- a/src/connector_runtime/execution_tests.rs +++ b/src/connector_runtime/execution_tests.rs @@ -293,13 +293,16 @@ fn project_summary(id: &str, path: &Path) -> ShellAgentProjectSummary { } async fn next_request(registry: &ShellClientRegistry) -> ShellAgentShellRequest { - for _ in 0..2_000 { + let deadline = Instant::now() + Duration::from_secs(10); + loop { if let Some(request) = poll(registry).await { return request; } - tokio::time::sleep(Duration::from_millis(1)).await; + if Instant::now() >= deadline { + panic!("Connector agent dispatch readiness failed: no request dispatched within 10 seconds"); + } + tokio::time::sleep(Duration::from_millis(5)).await; } - panic!("agent request was not dispatched"); } async fn poll(registry: &ShellClientRegistry) -> Option { @@ -313,6 +316,123 @@ async fn poll(registry: &ShellClientRegistry) -> Option .unwrap() } +fn latest_execution(fixture: &Fixture) -> crate::db::ConnectorExecution { + fixture + .connector + .db + .latest_connector_execution( + &fixture.task_id, + &fixture.connector.context.project_id, + tests::PROJECT_SUBJECT_ID, + None, + ) + .unwrap() + .expect("connector execution should exist") +} + +fn execution_by_id(fixture: &Fixture, execution_id: &str) -> crate::db::ConnectorExecution { + fixture + .connector + .db + .connector_execution(execution_id) + .unwrap() +} + +async fn wait_for_execution( + fixture: &Fixture, + execution_id: Option<&str>, + timeout: Duration, + description: &str, + predicate: impl Fn(&crate::db::ConnectorExecution) -> bool, +) -> crate::db::ConnectorExecution { + let deadline = Instant::now() + timeout; + loop { + let current = match execution_id { + Some(execution_id) => execution_by_id(fixture, execution_id), + None => latest_execution(fixture), + }; + if predicate(¤t) { + return current; + } + if Instant::now() >= deadline { + panic!( + "{description} did not become observable within {timeout:?}; last state={} status_failure={:?} executor_reference={:?}", + current.state, current.status_failure_code, current.executor_reference + ); + } + tokio::time::sleep(Duration::from_millis(5)).await; + } +} + +async fn wait_for_monitor_count( + fixture: &Fixture, + expected: usize, + timeout: Duration, + description: &str, +) { + let deadline = Instant::now() + timeout; + loop { + let current = fixture.connector.executions.active_monitor_count(); + if current == expected { + return; + } + if Instant::now() >= deadline { + panic!( + "{description} did not reach active monitor count {expected} within {timeout:?}; last count={current}" + ); + } + tokio::time::sleep(Duration::from_millis(5)).await; + } +} + +async fn wait_for_workspace_slot_state( + fixture: &Fixture, + expected: &str, + timeout: Duration, + description: &str, +) { + let deadline = Instant::now() + timeout; + loop { + let resources = workspace::WorkspaceManager::resource_status( + Path::new(&fixture.connector.context.runs_root), + fixture._temp.path().join("cargo-target").as_path(), + ); + if resources.slot_state == expected { + return; + } + if Instant::now() >= deadline { + panic!( + "{description} did not reach workspace slot state {expected} within {timeout:?}; last state={}", + resources.slot_state + ); + } + tokio::time::sleep(Duration::from_millis(5)).await; + } +} + +fn executor_status_observation<'a>( + executor_status: &'a str, + stdout_cursor: usize, + stderr_cursor: usize, + started_at: Option, + now: i64, +) -> ConnectorExecutionObservation<'a> { + ConnectorExecutionObservation { + executor_status, + stdout_cursor, + stderr_cursor, + exit_code: None, + started_at, + finished_at: None, + check_completed: None, + failed_check: None, + assertion_evidence: None, + validated_workspace_sha256: None, + executor_failure_code: None, + now, + } +} + fn created(reservation: ConnectorExecutionReservation) -> crate::db::ConnectorExecution { match reservation { ConnectorExecutionReservation::Created(execution) => execution, @@ -912,113 +1032,102 @@ async fn node_lockfile_change_makes_successful_checks_run_stale() { } #[tokio::test] -async fn validation_step_spawn_failure_is_executor_failure_without_assertion_evidence() { - let fixture = fixture(1_000).await; - let registry = fixture.registry.clone(); - let responder = tokio::spawn(async move { - let request = next_request(®istry).await; - assert_eq!(request.kind, "start_validation_job"); - let job_id = request.job_id.unwrap(); - update_validation_job( - ®istry, - &job_id, - "running", - Some("format completed\n"), - None, - check_progress(1, Some("check"), None), - ) - .await; - let mut failed = validation_job_update(&job_id, "failed", check_progress(1, None, None)); - failed.error = Some("validation_step_spawn_failed".to_string()); - let updated = registry.update_job(failed).await.unwrap(); - assert_eq!(updated.status, "failed"); - }); - - let outcome = fixture - .call( - "checks_run", - checks(&fixture, "spawn-failure", &["format", "check"]), - ) - .await; - responder.await.unwrap(); - assert!(outcome.ok, "{}", outcome.body); - let execution = &outcome.body["data"]["execution"]; - assert_eq!(execution["execution_status"], "failed"); - assert_eq!(execution["failure_source"], "executor"); - assert_eq!(execution["failure_code"], "validation_step_spawn_failed"); - assert_ne!(execution["assertion_status"], "failed"); - assert!(execution["assertion_evidence"].is_null()); - assert_eq!(execution["checks"][1]["status"], "not_run"); - - let durable = fixture - .connector - .db - .connector_execution(execution["execution_id"].as_str().unwrap()) - .unwrap(); - assert!(durable.failed_check.is_none()); - assert!(durable.assertion_evidence.is_none()); - assert!(durable.validated_workspace_sha256.is_none()); -} +async fn validation_executor_failures_preserve_codes_without_assertion_evidence() { + let cases = [ + ("spawn-failure", "validation_step_spawn_failed", true, false), + ( + "tool-unavailable", + "validation_tool_unavailable", + false, + false, + ), + ( + "wait-failure", + VALIDATION_STEP_WAIT_FAILED_CODE, + false, + true, + ), + ]; -#[tokio::test] -async fn validation_tool_unavailable_is_executor_failure_not_assertion_failure() { - let fixture = fixture(1_000).await; - let registry = fixture.registry.clone(); - let responder = tokio::spawn(async move { - let request = next_request(®istry).await; - let mut failed = validation_job_update( - request.job_id.as_deref().unwrap(), - "failed", - check_progress(0, None, None), + for (operation_id, failure_code, format_completed, assert_wait_failed_step_none) in cases { + let fixture = fixture(1_000).await; + let registry = fixture.registry.clone(); + let responder_failure_code = failure_code.to_string(); + let responder = tokio::spawn(async move { + let request = next_request(®istry).await; + assert_eq!(request.kind, "start_validation_job"); + let job_id = request.job_id.unwrap(); + if format_completed { + update_validation_job( + ®istry, + &job_id, + "running", + Some("format completed\n"), + None, + check_progress(1, Some("check"), None), + ) + .await; + } + let completed = if format_completed { 1 } else { 0 }; + let mut failed = + validation_job_update(&job_id, "failed", check_progress(completed, None, None)); + failed.error = Some(responder_failure_code); + let updated = registry.update_job(failed).await.unwrap(); + assert_eq!(updated.status, "failed"); + if assert_wait_failed_step_none { + assert!(updated + .validation_progress + .is_some_and(|progress| progress.failed_step.is_none())); + } + }); + + let plan: &[&str] = if format_completed { + &["format", "check"] + } else { + &["check"] + }; + let outcome = fixture + .call("checks_run", checks(&fixture, operation_id, plan)) + .await; + responder.await.unwrap(); + assert!(outcome.ok, "{failure_code}: {}", outcome.body); + let execution = &outcome.body["data"]["execution"]; + assert_eq!(execution["execution_status"], "failed", "{failure_code}"); + assert_eq!(execution["failure_source"], "executor", "{failure_code}"); + assert_eq!(execution["failure_code"], failure_code, "{failure_code}"); + assert_ne!(execution["assertion_status"], "failed", "{failure_code}"); + assert!(execution["assertion_evidence"].is_null(), "{failure_code}"); + let projected_checks = execution["checks"].as_array().unwrap(); + assert!( + projected_checks + .iter() + .all(|check| check["status"] != "failed"), + "{failure_code}: {projected_checks:?}" ); - failed.error = Some("validation_tool_unavailable".to_string()); - registry.update_job(failed).await.unwrap(); - }); - let outcome = fixture - .call( - "checks_run", - checks(&fixture, "tool-unavailable", &["check"]), - ) - .await; - responder.await.unwrap(); - let execution = &outcome.body["data"]["execution"]; - assert_eq!(execution["execution_status"], "failed"); - assert_eq!(execution["failure_source"], "executor"); - assert_eq!(execution["failure_code"], "validation_tool_unavailable"); - assert!(execution["assertion_evidence"].is_null()); - assert!(execution["checks"][0]["status"] != "failed"); -} - -#[tokio::test] -async fn validation_step_wait_failure_is_executor_failure_without_assertion_evidence() { - let fixture = fixture(1_000).await; - let registry = fixture.registry.clone(); - let responder = tokio::spawn(async move { - let request = next_request(®istry).await; - let mut failed = validation_job_update( - request.job_id.as_deref().unwrap(), - "failed", - check_progress(0, None, None), + let durable = fixture + .connector + .db + .connector_execution(execution["execution_id"].as_str().unwrap()) + .unwrap(); + assert!(durable.failed_check.is_none(), "{failure_code}"); + assert!(durable.assertion_evidence.is_none(), "{failure_code}"); + assert!( + durable.validated_workspace_sha256.is_none(), + "{failure_code}" ); - failed.error = Some(VALIDATION_STEP_WAIT_FAILED_CODE.to_string()); - let updated = registry.update_job(failed).await.unwrap(); - assert_eq!(updated.status, "failed"); - assert!(updated - .validation_progress - .is_some_and(|progress| progress.failed_step.is_none())); - }); - let outcome = fixture - .call("checks_run", checks(&fixture, "wait-failure", &["check"])) - .await; - responder.await.unwrap(); - assert!(outcome.ok, "{}", outcome.body); - let execution = &outcome.body["data"]["execution"]; - assert_eq!(execution["execution_status"], "failed"); - assert_eq!(execution["failure_source"], "executor"); - assert_eq!(execution["failure_code"], VALIDATION_STEP_WAIT_FAILED_CODE); - assert_ne!(execution["assertion_status"], "failed"); - assert!(execution["assertion_evidence"].is_null()); - assert_ne!(execution["checks"][0]["status"], "failed"); + + if format_completed { + assert_eq!( + execution["checks"], + json!([ + {"check": "format", "status": "passed"}, + {"check": "check", "status": "not_run"} + ]) + ); + assert_eq!(durable.check_plan, vec!["format", "check"]); + assert_eq!(durable.check_completed, 1); + } + } } #[tokio::test] @@ -2488,21 +2597,7 @@ async fn starting_cancel_late_attach_binds_job_and_dispatches_compensating_stop( ); assert_eq!(fixture.connector.executions.active_monitor_count(), 0); tokio::time::sleep(Duration::from_millis(120)).await; - assert_eq!( - fixture - .connector - .db - .latest_connector_execution( - &fixture.task_id, - &fixture.connector.context.project_id, - tests::PROJECT_SUBJECT_ID, - None, - ) - .unwrap() - .unwrap() - .state, - "cancel_requested" - ); + assert_eq!(latest_execution(&fixture).state, "cancel_requested"); let stop_registry = fixture.registry.clone(); let expected_job_id = job_id.clone(); @@ -2522,11 +2617,7 @@ async fn starting_cancel_late_attach_binds_job_and_dispatches_compensating_stop( let execution_id = completed.body["data"]["execution"]["execution_id"] .as_str() .unwrap(); - let durable = fixture - .connector - .db - .connector_execution(execution_id) - .unwrap(); + let durable = execution_by_id(&fixture, execution_id); assert_eq!(durable.executor_reference.as_deref(), Some(job_id.as_str())); assert_eq!(durable.state, "cancelled"); assert_eq!( @@ -2542,17 +2633,13 @@ async fn starting_cancel_late_attach_binds_job_and_dispatches_compensating_stop( job.status.as_str(), "queued" | "agent_queued" | "running" | "stop_requested" ))); - for _ in 0..100 { - let resources = workspace::WorkspaceManager::resource_status( - Path::new(&fixture.connector.context.runs_root), - fixture._temp.path().join("cargo-target").as_path(), - ); - if resources.slot_state == "idle" { - return; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - panic!("cancelled workspace slot was not released"); + wait_for_workspace_slot_state( + &fixture, + "idle", + Duration::from_secs(10), + "late-attach cancellation", + ) + .await; } #[tokio::test] @@ -2597,30 +2684,29 @@ async fn retry_and_cancel_share_one_execution_monitor() { let cancelled_id = cancelled.body["data"]["execution"]["execution_id"] .as_str() .unwrap(); - assert!(fixture - .connector - .db - .connector_execution(cancelled_id) - .unwrap() + assert!(execution_by_id(&fixture, cancelled_id) .validated_workspace_sha256 .is_none()); assert_eq!(fixture.connector.executions.monitor_start_count(), 1); - for _ in 0..100 { - if fixture.connector.executions.active_monitor_count() == 0 { - break; - } - tokio::time::sleep(Duration::from_millis(2)).await; - } - assert_eq!(fixture.connector.executions.active_monitor_count(), 0); + wait_for_monitor_count( + &fixture, + 0, + Duration::from_secs(10), + "cancelled execution monitor shutdown", + ) + .await; } #[tokio::test] async fn transient_check_status_recovers_within_grace() { - // Grace must comfortably exceed scheduler starvation under full-suite - // parallelism, or the monitor finishes the execution as unknown before it - // can observe the recovery update. Recovery itself happens at the next - // successful poll (~fast_poll), so the wide grace does not slow the test. - let fixture = fixture_configured(20, |service| service.with_monitor_timing(2_000, 5)).await; + // Keep readiness comfortably inside the test-only grace so recovery can be + // injected before the monitor is allowed to finalize the execution unknown. + let monitor_grace_ms = 3_000; + let readiness = Duration::from_secs(2); + let fixture = fixture_configured(20, move |service| { + service.with_monitor_timing(monitor_grace_ms, 5) + }) + .await; let arguments = checks(&fixture, "transient-status-1", &["check"]); let connector = fixture.connector.clone(); let owner = fixture.owner.clone(); @@ -2637,31 +2723,16 @@ async fn transient_check_status_recovers_within_grace() { )) .await .unwrap(); - // The degraded observation is asynchronous: wait for the monitor to record - // the failure instead of racing it with a fixed sleep. Grace only starts - // counting at the first observed failure, so waiting here is safe. - let mut observed = None; - for _ in 0..400 { - let current = fixture - .connector - .db - .latest_connector_execution( - &fixture.task_id, - &fixture.connector.context.project_id, - tests::PROJECT_SUBJECT_ID, - None, - ) - .unwrap() - .unwrap(); - if current.status_failure_code.is_some() { - observed = Some(current); - break; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - let degraded = observed.expect("monitor never recorded the unrecognized executor status"); + + let degraded = wait_for_execution( + &fixture, + None, + readiness, + "monitor degraded observation", + |current| current.status_failure_code.as_deref() == Some("executor_status_unrecognized"), + ) + .await; assert!(degraded.is_active()); - assert_ne!(degraded.state, "running"); assert_eq!( degraded.status_failure_code.as_deref(), Some("executor_status_unrecognized") @@ -2678,24 +2749,16 @@ async fn transient_check_status_recovers_within_grace() { check_progress(0, Some("check"), None), ) .await; - for _ in 0..400 { - let recovered = fixture - .connector - .db - .connector_execution(°raded.execution_id) - .unwrap(); - if recovered.status_failure_code.is_none() && recovered.state == "running" { - break; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - let recovered = fixture - .connector - .db - .connector_execution(°raded.execution_id) - .unwrap(); - assert_eq!(recovered.state, "running"); + let recovered = wait_for_execution( + &fixture, + Some(°raded.execution_id), + readiness, + "monitor recovery observation", + |current| current.state == "running" && current.status_failure_code.is_none(), + ) + .await; assert_eq!(recovered.status_failure_code, None); + update_validation_job( &fixture.registry, &job_id, @@ -2706,26 +2769,28 @@ async fn transient_check_status_recovers_within_grace() { ) .await; let _quick_yield = check_call.await.unwrap(); - for _ in 0..400 { - let completed = fixture - .connector - .db - .connector_execution(°raded.execution_id) - .unwrap(); - if completed.state == "succeeded" { - return; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - panic!("recovered execution did not reach succeeded"); + let completed = wait_for_execution( + &fixture, + Some(°raded.execution_id), + Duration::from_secs(10), + "recovered execution success", + |current| current.state == "succeeded", + ) + .await; + assert_eq!(completed.status_failure_code, None); } #[tokio::test] async fn check_transport_failure_becomes_unknown_only_after_grace() { - // Grace must comfortably exceed scheduler starvation under full-suite - // parallelism so the test can observe the degraded active state before the - // monitor legitimately finishes the execution as unknown. - let fixture = fixture_configured(5, |service| service.with_monitor_timing(2_000, 5)).await; + // The degraded observation must arrive inside the grace; terminal `unknown` + // is allowed only after that grace has elapsed from transport loss. + let monitor_grace = Duration::from_millis(3_000); + let degraded_readiness = Duration::from_secs(2); + let unknown_readiness = Duration::from_secs(6); + let fixture = fixture_configured(5, move |service| { + service.with_monitor_timing(monitor_grace.as_millis() as u64, 5) + }) + .await; let arguments = checks(&fixture, "transport-grace-1", &["test"]); let connector = fixture.connector.clone(); let owner = fixture.owner.clone(); @@ -2750,40 +2815,36 @@ async fn check_transport_failure_becomes_unknown_only_after_grace() { .registry .reconcile_disconnect("hosted", "instance") .await; - // The degraded observation is asynchronous. Poll for the recorded failure - // instead of assuming a fixed sleep lands inside the grace window. - let mut observed = None; - for _ in 0..400 { - let current = fixture - .connector - .db - .connector_execution(execution_id) - .unwrap(); - if current.status_failure_code.as_deref() == Some("executor_status_unavailable") { - observed = Some(current); - break; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - let degraded = observed.expect("monitor never recorded the executor transport failure"); + + let degraded = wait_for_execution( + &fixture, + Some(execution_id), + degraded_readiness, + "executor transport degradation", + |current| current.status_failure_code.as_deref() == Some("executor_status_unavailable"), + ) + .await; + let degraded_observed_at = Instant::now(); assert!(degraded.is_active()); + assert_ne!(degraded.state, "unknown"); assert_eq!( degraded.status_failure_code.as_deref(), Some("executor_status_unavailable") ); - for _ in 0..600 { - let current = fixture - .connector - .db - .connector_execution(execution_id) - .unwrap(); - if current.state == "unknown" { - assert_eq!(current.executor_reference.as_deref(), Some(job_id.as_str())); - return; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - panic!("execution did not become unknown after the configured grace"); + + let unknown = wait_for_execution( + &fixture, + Some(execution_id), + unknown_readiness, + "executor transport grace expiry", + |current| current.state == "unknown", + ) + .await; + assert_eq!(unknown.executor_reference.as_deref(), Some(job_id.as_str())); + assert!( + degraded_observed_at.elapsed() + Duration::from_millis(100) >= monitor_grace, + "execution became unknown before the configured {monitor_grace:?} transport grace" + ); } #[tokio::test] @@ -2899,11 +2960,20 @@ async fn running_check_allows_review_wait_cancel_and_releases_slot() { check.body["data"]["execution"]["execution_id"], cancelled.body["data"]["execution"]["execution_id"] ); - let resources = workspace::WorkspaceManager::resource_status( - Path::new(&fixture.connector.context.runs_root), - fixture._temp.path().join("cargo-target").as_path(), - ); - assert_eq!(resources.slot_state, "idle"); + wait_for_monitor_count( + &fixture, + 0, + Duration::from_secs(10), + "running validation cancellation monitor shutdown", + ) + .await; + wait_for_workspace_slot_state( + &fixture, + "idle", + Duration::from_secs(10), + "running validation cancellation", + ) + .await; } #[tokio::test] @@ -2968,11 +3038,7 @@ async fn queued_cancel_never_dispatches_and_restart_is_fail_closed() { .reconcile_connector_executions(&second.connector.context.project_id, 11) .unwrap(); assert_eq!(recovery.1, 1); - let interrupted = second - .connector - .db - .connector_execution(&execution.execution_id) - .unwrap(); + let interrupted = execution_by_id(&second, &execution.execution_id); assert_eq!(interrupted.state, "interrupted"); assert!(interrupted.validated_workspace_sha256.is_none()); assert_eq!(task(&second).task_status, "needs_attention"); @@ -3055,11 +3121,7 @@ async fn cancellation_transport_unknown_preserves_executor_reference_and_blocks_ cancelled.body["data"]["execution"]["execution_status"], "unknown" ); - let durable = fixture - .connector - .db - .connector_execution(&execution.execution_id) - .unwrap(); + let durable = execution_by_id(&fixture, &execution.execution_id); assert_eq!(durable.executor_reference.as_deref(), Some(job_id)); let finish = finish(&fixture, "must stay blocked").await; assert_eq!(finish.body["error"]["code"], "execution_not_terminal"); @@ -3218,20 +3280,7 @@ async fn connector_execution_recovering_status_remains_active_without_duplicate_ let queued_recovery = db .observe_connector_execution( &execution.execution_id, - ConnectorExecutionObservation { - executor_status: "recovering", - stdout_cursor: 1, - stderr_cursor: 1, - exit_code: None, - started_at: None, - finished_at: None, - check_completed: None, - failed_check: None, - assertion_evidence: None, - validated_workspace_sha256: None, - executor_failure_code: None, - now: 4, - }, + executor_status_observation("recovering", 1, 1, None, 4), ) .unwrap(); assert_eq!(queued_recovery.state, "queued"); @@ -3242,46 +3291,18 @@ async fn connector_execution_recovering_status_remains_active_without_duplicate_ let running = db .observe_connector_execution( &execution.execution_id, - ConnectorExecutionObservation { - executor_status: "running", - stdout_cursor: 2, - stderr_cursor: 1, - exit_code: None, - started_at: Some(5), - finished_at: None, - check_completed: None, - failed_check: None, - assertion_evidence: None, - validated_workspace_sha256: None, - executor_failure_code: None, - now: 5, - }, + executor_status_observation("running", 2, 1, Some(5), 5), ) .unwrap(); assert_eq!(running.state, "running"); let running_recovery = db .observe_connector_execution( &execution.execution_id, - ConnectorExecutionObservation { - executor_status: "recovering", - stdout_cursor: 2, - stderr_cursor: 1, - exit_code: None, - started_at: Some(5), - finished_at: None, - check_completed: None, - failed_check: None, - assertion_evidence: None, - validated_workspace_sha256: None, - executor_failure_code: None, - now: 6, - }, + executor_status_observation("recovering", 2, 1, Some(5), 6), ) .unwrap(); assert_eq!(running_recovery.state, "running"); assert!(running_recovery.is_active()); - assert_ne!(running_recovery.state, "succeeded"); - assert_ne!(running_recovery.state, "failed"); match db .reserve_connector_execution( @@ -3329,23 +3350,13 @@ async fn unrecognized_executor_status_is_degraded_instead_of_running() { let observed = db .observe_connector_execution( &execution.execution_id, - ConnectorExecutionObservation { - executor_status: "future-agent-state", - stdout_cursor: 1, - stderr_cursor: 1, - exit_code: None, - started_at: None, - finished_at: None, - check_completed: None, - failed_check: None, - assertion_evidence: None, - validated_workspace_sha256: None, - executor_failure_code: None, - now: 4, - }, + executor_status_observation("future-agent-state", 1, 1, None, 4), ) .unwrap(); assert_eq!(observed.state, "queued"); + assert!(observed.is_active()); + assert!(observed.failure_code.is_none()); + assert!(observed.terminal_reason.is_none()); assert_eq!( observed.status_failure_code.as_deref(), Some("executor_status_unrecognized") @@ -4490,26 +4501,14 @@ async fn provenance_mismatch_fails_honestly_with_evidence() { // Deterministic invariant failure: terminal quickly (no grace burn), // honestly categorized, with the evidence and the remedy in the message. - let mut terminal = None; - for _ in 0..400 { - let execution = fixture - .connector - .db - .latest_connector_execution( - &fixture.task_id, - &fixture.connector.context.project_id, - tests::PROJECT_SUBJECT_ID, - None, - ) - .unwrap() - .unwrap(); - if execution.is_terminal() { - terminal = Some(execution); - break; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - let execution = terminal.expect("execution should reach a terminal state quickly"); + let execution = wait_for_execution( + &fixture, + None, + Duration::from_secs(10), + "workspace provenance mismatch terminal state", + |execution| execution.is_terminal(), + ) + .await; assert_eq!(execution.state, "failed"); assert_eq!(execution.failure_source.as_deref(), Some("workspace")); assert_eq!( @@ -4602,26 +4601,14 @@ async fn provenance_mismatch_from_tracked_changes_keeps_gitignore_out_of_the_rem let outcome = check_call.await.unwrap(); assert!(outcome.ok, "{}", outcome.body); - let mut terminal = None; - for _ in 0..400 { - let execution = fixture - .connector - .db - .latest_connector_execution( - &fixture.task_id, - &fixture.connector.context.project_id, - tests::PROJECT_SUBJECT_ID, - None, - ) - .unwrap() - .unwrap(); - if execution.is_terminal() { - terminal = Some(execution); - break; - } - tokio::time::sleep(Duration::from_millis(5)).await; - } - let execution = terminal.expect("execution should reach a terminal state quickly"); + let execution = wait_for_execution( + &fixture, + None, + Duration::from_secs(10), + "tracked workspace provenance mismatch terminal state", + |execution| execution.is_terminal(), + ) + .await; assert_eq!(execution.state, "failed"); assert_eq!(execution.failure_source.as_deref(), Some("workspace")); assert_eq!( diff --git a/src/mcp_tests/http_transport.rs b/src/mcp_tests/http_transport.rs index c00339da..ebcbf825 100644 --- a/src/mcp_tests/http_transport.rs +++ b/src/mcp_tests/http_transport.rs @@ -11,27 +11,36 @@ fn with_mcp_recording_session(mut arguments: Value, session_id: &str) -> Value { arguments } -async fn stateless_2026_tool_call( +async fn legacy_mcp_jsonrpc(service: &Service, token: &str, body: Value) -> (StatusCode, Value) { + let mut response = TestClient::post("http://localhost/mcp") + .bearer_auth(token) + .json(&body) + .send(service) + .await; + let status = effective_status(&response); + let body = response.take_json::().await.unwrap(); + (status, body) +} + +async fn stateless_2026_jsonrpc( service: &Service, token: &str, - id: i64, - name: &str, - arguments: Value, + protocol_header: Option<&str>, + method_header: Option<&str>, + name_header: Option<&str>, legacy_session_id: Option<&str>, + body: Value, ) -> (StatusCode, Value) { - let params = mcp_2026_params(json!({ - "name": name, - "arguments": arguments, - })); - let mut request = TestClient::post("http://localhost/mcp") - .bearer_auth(token) - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "tools/call", true) - .add_header(MCP_NAME_HEADER, name, true); + let mut request = TestClient::post("http://localhost/mcp").bearer_auth(token); + if let Some(protocol_header) = protocol_header { + request = request.add_header(MCP_PROTOCOL_VERSION_HEADER, protocol_header, true); + } + if let Some(method_header) = method_header { + request = request.add_header(MCP_METHOD_HEADER, method_header, true); + } + if let Some(name_header) = name_header { + request = request.add_header(MCP_NAME_HEADER, name_header, true); + } if let Some(legacy_session_id) = legacy_session_id { request = request.add_header( crate::client_window::MCP_SESSION_HEADER, @@ -39,15 +48,7 @@ async fn stateless_2026_tool_call( true, ); } - let mut response = request - .json(&json!({ - "jsonrpc": "2.0", - "id": id, - "method": "tools/call", - "params": params, - })) - .send(service) - .await; + let mut response = request.json(&body).send(service).await; assert!( response .headers() @@ -60,6 +61,31 @@ async fn stateless_2026_tool_call( (status, body) } +async fn stateless_2026_tool_call( + service: &Service, + token: &str, + id: i64, + name: &str, + arguments: Value, + legacy_session_id: Option<&str>, +) -> (StatusCode, Value) { + stateless_2026_jsonrpc( + service, + token, + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("tools/call"), + Some(name), + legacy_session_id, + json!({ + "jsonrpc": "2.0", + "id": id, + "method": "tools/call", + "params": mcp_2026_params(json!({"name": name, "arguments": arguments})), + }), + ) + .await +} + async fn stateless_observation_shell_clients() -> Arc { let shell_clients = Arc::new(crate::shell_client::ShellClientRegistry::default()); shell_clients @@ -1226,197 +1252,139 @@ async fn http_mcp_2026_observe_session_messages_preserves_stateless_delta_contra } #[tokio::test] -async fn http_mcp_2026_validates_headers_and_ignores_legacy_session_id() { +async fn http_mcp_2026_protocol_error_matrix_and_legacy_session_compatibility() { let config = test_config(Some("secret")); let (_tmp, db) = test_db(); let runtime = Arc::new(test_runtime_with_surface(ModelSurface::FullOperatorRuntime)); let service = Service::new(build_test_router(config, db, runtime)); let params = mcp_2026_params(json!({})); - let mut missing_headers = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .json(&json!({ - "jsonrpc": "2.0", - "id": 200, - "method": "tools/list", - "params": params.clone() - })) - .send(&service) - .await; - assert_eq!(effective_status(&missing_headers), StatusCode::BAD_REQUEST); - let missing_body: Value = missing_headers.take_json().await.unwrap(); - assert_eq!(missing_body["error"]["code"], MCP_HEADER_MISMATCH); - - let mut ok = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "tools/list", true) - .add_header( - crate::client_window::MCP_SESSION_HEADER, - "legacy-session-must-be-ignored", - true, - ) - .json(&json!({ - "jsonrpc": "2.0", - "id": 201, - "method": "tools/list", - "params": params.clone() - })) - .send(&service) - .await; - assert_eq!(effective_status(&ok), StatusCode::OK); - assert!(ok - .headers - .get(crate::client_window::MCP_SESSION_HEADER) - .is_none()); - let ok_body: Value = ok.take_json().await.unwrap(); - assert_eq!(ok_body["result"]["resultType"], "complete"); - assert_eq!(ok_body["result"]["cacheScope"], "private"); - - let mut method_mismatch = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "ping", true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 202, - "method": "tools/list", - "params": params.clone() - })) - .send(&service) - .await; - assert_eq!(effective_status(&method_mismatch), StatusCode::BAD_REQUEST); - let mismatch_body: Value = method_mismatch.take_json().await.unwrap(); - assert_eq!(mismatch_body["error"]["code"], MCP_HEADER_MISMATCH); - - let mut missing_capabilities = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "tools/list", true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 2021, - "method": "tools/list", - "params": { - "_meta": { - "io.modelcontextprotocol/protocolVersion": MCP_STATELESS_PROTOCOL_VERSION - } - } - })) - .send(&service) - .await; - assert_eq!( - effective_status(&missing_capabilities), - StatusCode::BAD_REQUEST - ); - let missing_capabilities_body: Value = missing_capabilities.take_json().await.unwrap(); - assert_eq!(missing_capabilities_body["error"]["code"], -32602); - - let mut version_mismatch = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header(MCP_PROTOCOL_VERSION_HEADER, "2099-01-01", true) - .add_header(MCP_METHOD_HEADER, "tools/list", true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 2022, - "method": "tools/list", - "params": params.clone() - })) - .send(&service) - .await; - assert_eq!(effective_status(&version_mismatch), StatusCode::BAD_REQUEST); - let version_mismatch_body: Value = version_mismatch.take_json().await.unwrap(); - assert_eq!(version_mismatch_body["error"]["code"], MCP_HEADER_MISMATCH); - - let mut malformed_client_info = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "tools/list", true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 2023, - "method": "tools/list", - "params": { - "_meta": { + let cases = vec![ + ( + "missing headers", + None, + None, + json!({"jsonrpc": "2.0", "id": 200, "method": "tools/list", "params": params.clone()}), + StatusCode::BAD_REQUEST, + json!(MCP_HEADER_MISMATCH), + 200, + ), + ( + "method header mismatch", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("ping"), + json!({"jsonrpc": "2.0", "id": 202, "method": "tools/list", "params": params.clone()}), + StatusCode::BAD_REQUEST, + json!(MCP_HEADER_MISMATCH), + 202, + ), + ( + "missing client capabilities", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("tools/list"), + json!({ + "jsonrpc": "2.0", + "id": 2021, + "method": "tools/list", + "params": {"_meta": {"io.modelcontextprotocol/protocolVersion": MCP_STATELESS_PROTOCOL_VERSION}} + }), + StatusCode::BAD_REQUEST, + json!(-32602), + 2021, + ), + ( + "protocol header mismatch", + Some("2099-01-01"), + Some("tools/list"), + json!({"jsonrpc": "2.0", "id": 2022, "method": "tools/list", "params": params.clone()}), + StatusCode::BAD_REQUEST, + json!(MCP_HEADER_MISMATCH), + 2022, + ), + ( + "malformed client info", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("tools/list"), + json!({ + "jsonrpc": "2.0", + "id": 2023, + "method": "tools/list", + "params": {"_meta": { "io.modelcontextprotocol/protocolVersion": MCP_STATELESS_PROTOCOL_VERSION, "io.modelcontextprotocol/clientCapabilities": {}, "io.modelcontextprotocol/clientInfo": {"name": "missing-version"} - } - } - })) - .send(&service) - .await; - assert_eq!( - effective_status(&malformed_client_info), - StatusCode::BAD_REQUEST - ); - let malformed_client_info_body: Value = malformed_client_info.take_json().await.unwrap(); - assert_eq!(malformed_client_info_body["error"]["code"], -32602); - - let mut missing_jsonrpc = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "tools/list", true) - .json(&json!({ - "id": 2024, - "method": "tools/list", - "params": params.clone() - })) - .send(&service) - .await; - assert_eq!(effective_status(&missing_jsonrpc), StatusCode::BAD_REQUEST); - let missing_jsonrpc_body: Value = missing_jsonrpc.take_json().await.unwrap(); - assert_eq!(missing_jsonrpc_body["error"]["code"], -32600); - - let unsupported_params = json!({ - "_meta": { - "io.modelcontextprotocol/protocolVersion": "2099-01-01", - "io.modelcontextprotocol/clientCapabilities": {} + }} + }), + StatusCode::BAD_REQUEST, + json!(-32602), + 2023, + ), + ( + "invalid jsonrpc", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("tools/list"), + json!({"id": 2024, "method": "tools/list", "params": params.clone()}), + StatusCode::BAD_REQUEST, + json!(-32600), + 2024, + ), + ( + "unsupported protocol", + Some("2099-01-01"), + Some("tools/list"), + json!({ + "jsonrpc": "2.0", + "id": 203, + "method": "tools/list", + "params": {"_meta": { + "io.modelcontextprotocol/protocolVersion": "2099-01-01", + "io.modelcontextprotocol/clientCapabilities": {} + }} + }), + StatusCode::BAD_REQUEST, + json!(MCP_UNSUPPORTED_PROTOCOL_VERSION), + 203, + ), + ( + "unknown method", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("prompts/list"), + json!({"jsonrpc": "2.0", "id": 206, "method": "prompts/list", "params": params.clone()}), + StatusCode::NOT_FOUND, + json!(-32601), + 206, + ), + ]; + + for (label, protocol, method, request, expected_status, expected_code, expected_id) in cases { + let (status, body) = + stateless_2026_jsonrpc(&service, "secret", protocol, method, None, None, request).await; + assert_eq!(status, expected_status, "{label}: {body}"); + assert_eq!(body["id"], expected_id, "{label}: {body}"); + assert_eq!(body["error"]["code"], expected_code, "{label}: {body}"); + if label == "unsupported protocol" { + assert_eq!(body["error"]["data"]["requested"], "2099-01-01"); + assert_eq!( + body["error"]["data"]["supported"], + json!(MCP_SUPPORTED_PROTOCOL_VERSIONS) + ); } - }); - let mut unsupported = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header(MCP_PROTOCOL_VERSION_HEADER, "2099-01-01", true) - .add_header(MCP_METHOD_HEADER, "tools/list", true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 203, - "method": "tools/list", - "params": unsupported_params - })) - .send(&service) - .await; - assert_eq!(effective_status(&unsupported), StatusCode::BAD_REQUEST); - let unsupported_body: Value = unsupported.take_json().await.unwrap(); - assert_eq!( - unsupported_body["error"]["code"], - MCP_UNSUPPORTED_PROTOCOL_VERSION - ); - assert_eq!(unsupported_body["error"]["data"]["requested"], "2099-01-01"); - assert_eq!( - unsupported_body["error"]["data"]["supported"], - json!(MCP_SUPPORTED_PROTOCOL_VERSIONS) - ); + } + + let (status, body) = stateless_2026_jsonrpc( + &service, + "secret", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("tools/list"), + None, + Some("legacy-session-must-be-ignored"), + json!({"jsonrpc": "2.0", "id": 201, "method": "tools/list", "params": params}), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(body["id"], 201); + assert_eq!(body["result"]["resultType"], "complete"); + assert_eq!(body["result"]["cacheScope"], "private"); } #[tokio::test] @@ -1427,75 +1395,40 @@ async fn http_mcp_2026_tools_call_requires_matching_name_and_accepts_base64_sent let service = Service::new(build_test_router(config, db, runtime)); let params = mcp_2026_params(json!({"name": "list_projects", "arguments": {}})); - let mut missing_name = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, + for (label, name_header, id) in [ + ("missing name", None, 204), + ("mismatched name", Some("runtime_status"), 2041), + ] { + let (status, body) = stateless_2026_jsonrpc( + &service, + "secret", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("tools/call"), + name_header, + None, + json!({"jsonrpc": "2.0", "id": id, "method": "tools/call", "params": params.clone()}), ) - .add_header(MCP_METHOD_HEADER, "tools/call", true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 204, - "method": "tools/call", - "params": params.clone() - })) - .send(&service) .await; - assert_eq!(effective_status(&missing_name), StatusCode::BAD_REQUEST); - let missing_name_body: Value = missing_name.take_json().await.unwrap(); - assert_eq!(missing_name_body["error"]["code"], MCP_HEADER_MISMATCH); + assert_eq!(status, StatusCode::BAD_REQUEST, "{label}: {body}"); + assert_eq!(body["id"], id); + assert_eq!(body["error"]["code"], MCP_HEADER_MISMATCH); + } let encoded = general_purpose::STANDARD.encode("list_projects"); let encoded = format!("=?base64?{encoded}?="); - let mut ok = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "tools/call", true) - .add_header(MCP_NAME_HEADER, &encoded, true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 205, - "method": "tools/call", - "params": params - })) - .send(&service) - .await; - assert_eq!(effective_status(&ok), StatusCode::OK); - let ok_body: Value = ok.take_json().await.unwrap(); - assert_eq!(ok_body["result"]["resultType"], "complete"); -} - -#[tokio::test] -async fn http_mcp_2026_unknown_method_is_404_jsonrpc_method_not_found() { - let config = test_config(Some("secret")); - let (_tmp, db) = test_db(); - let runtime = Arc::new(test_runtime()); - let service = Service::new(build_test_router(config, db, runtime)); - let mut resp = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .add_header( - MCP_PROTOCOL_VERSION_HEADER, - MCP_STATELESS_PROTOCOL_VERSION, - true, - ) - .add_header(MCP_METHOD_HEADER, "prompts/list", true) - .json(&json!({ - "jsonrpc": "2.0", - "id": 206, - "method": "prompts/list", - "params": mcp_2026_params(json!({})) - })) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::NOT_FOUND); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["error"]["code"], -32601); + let (status, body) = stateless_2026_jsonrpc( + &service, + "secret", + Some(MCP_STATELESS_PROTOCOL_VERSION), + Some("tools/call"), + Some(&encoded), + None, + json!({"jsonrpc": "2.0", "id": 205, "method": "tools/call", "params": params}), + ) + .await; + assert_eq!(status, StatusCode::OK, "{body}"); + assert_eq!(body["id"], 205); + assert_eq!(body["result"]["resultType"], "complete"); } #[tokio::test] @@ -1806,111 +1739,96 @@ async fn http_mcp_2026_rejects_legacy_lifecycle_and_cross_origin_transport() { ); } #[tokio::test] -async fn http_mcp_tools_call_list_projects_returns_mcp_content() { - let config = test_config(Some("secret")); - let (_tmp, db) = test_db(); - let runtime = Arc::new(test_runtime()); - let service = Service::new(build_test_router(config, db, runtime)); - let mut resp = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .json(&json!({ - "jsonrpc": "2.0", - "id": 3, - "method": "tools/call", - "params": {"name": "list_projects", "arguments": {}} - })) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::OK); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["id"], 3); - assert_eq!(body["result"]["content"][0]["type"], "text"); - assert!(body["result"]["content"][0]["text"].is_string()); - assert!(body["result"]["structuredContent"].is_object()); - assert!( - body["result"]["structuredContent"]["success"].is_boolean(), - "structuredContent.success must be a bool" - ); - assert!( - body["result"]["isError"].is_boolean(), - "isError must be a bool" - ); - // A business failure (no projects configured) is an MCP tool error, - // not a JSON-RPC protocol error: the envelope is still a result. - assert!(body["result"].get("error").is_none()); - assert!(body.get("error").is_none(), "no top-level JSON-RPC error"); -} - -#[tokio::test] -async fn http_mcp_tools_call_unknown_tool_returns_jsonrpc_error() { +async fn http_mcp_tools_call_uses_result_envelope_for_success_and_business_failure() { let config = test_config(Some("secret")); let (_tmp, db) = test_db(); let runtime = Arc::new(test_runtime()); let service = Service::new(build_test_router(config, db, runtime)); - let mut resp = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .json(&json!({ - "jsonrpc": "2.0", - "id": 4, - "method": "tools/call", - "params": {"name": "no_such_tool", "arguments": {}} - })) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["error"]["code"], -32602); - assert!(body["error"]["message"] - .as_str() - .unwrap() - .contains("no_such_tool")); -} -#[tokio::test] -async fn http_mcp_unknown_method_returns_jsonrpc_error() { - let config = test_config(Some("secret")); - let (_tmp, db) = test_db(); - let runtime = Arc::new(test_runtime()); - let service = Service::new(build_test_router(config, db, runtime)); - let mut resp = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .json(&json!({ - "jsonrpc": "2.0", - "id": 5, - "method": "resources/list", - "params": {} - })) - .send(&service) + for (id, name, arguments, expected_is_error) in [ + (3, "list_projects", json!({}), false), + ( + 31, + "git_status", + json!({"project": "agent:nope:nope"}), + true, + ), + ] { + let (status, body) = legacy_mcp_jsonrpc( + &service, + "secret", + json!({ + "jsonrpc": "2.0", + "id": id, + "method": "tools/call", + "params": {"name": name, "arguments": arguments} + }), + ) .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["error"]["code"], -32601); - assert!(body["error"]["message"] - .as_str() - .unwrap() - .contains("resources/list")); + assert_eq!(status, StatusCode::OK, "{name}: {body}"); + assert_eq!(body["id"], id); + assert_eq!(body["result"]["content"][0]["type"], "text"); + assert!(body["result"]["content"][0]["text"].is_string()); + assert!(body["result"]["structuredContent"].is_object()); + assert!(body["result"]["structuredContent"]["success"].is_boolean()); + assert_eq!(body["result"]["isError"], expected_is_error); + assert_eq!( + body["result"]["structuredContent"]["success"], + !expected_is_error + ); + assert!(body.get("error").is_none(), "{name}: {body}"); + } } #[tokio::test] -async fn http_mcp_invalid_jsonrpc_returns_jsonrpc_error() { +async fn http_mcp_protocol_error_matrix_preserves_ids() { let config = test_config(Some("secret")); let (_tmp, db) = test_db(); let runtime = Arc::new(test_runtime()); let service = Service::new(build_test_router(config, db, runtime)); - let mut resp = TestClient::post("http://localhost/mcp") - .bearer_auth("secret") - .json(&json!({ - "jsonrpc": "1.0", - "id": 6, - "method": "initialize", - "params": {} - })) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["error"]["code"], -32600); - assert_eq!(body["id"], 6); + let cases = [ + ( + "unknown tool", + json!({ + "jsonrpc": "2.0", + "id": 4, + "method": "tools/call", + "params": {"name": "no_such_tool", "arguments": {}} + }), + 4, + -32602, + Some("no_such_tool"), + ), + ( + "unknown method", + json!({"jsonrpc": "2.0", "id": 5, "method": "resources/list", "params": {}}), + 5, + -32601, + Some("resources/list"), + ), + ( + "invalid jsonrpc", + json!({"jsonrpc": "1.0", "id": 6, "method": "initialize", "params": {}}), + 6, + -32600, + None, + ), + ]; + + for (label, request, expected_id, expected_code, message_fragment) in cases { + let (status, body) = legacy_mcp_jsonrpc(&service, "secret", request).await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{label}: {body}"); + assert_eq!(body["id"], expected_id, "{label}: {body}"); + assert_eq!(body["error"]["code"], expected_code, "{label}: {body}"); + if let Some(fragment) = message_fragment { + assert!( + body["error"]["message"] + .as_str() + .is_some_and(|message| message.contains(fragment)), + "{label}: {body}" + ); + } + } } #[tokio::test] diff --git a/src/runtime_http_tests.rs b/src/runtime_http_tests.rs index 3ad3a15c..183e664a 100644 --- a/src/runtime_http_tests.rs +++ b/src/runtime_http_tests.rs @@ -150,8 +150,28 @@ fn seed_oauth_access_token_with_shared_key_hash( plaintext } +fn phase2_oauth_service_with_scopes( + scopes: &[&str], +) -> (tempfile::TempDir, salvo::Service, Vec) { + let config = test_config_oauth2(Some("secret")); + let (tmp, db) = test_db(); + let user = seed_user(&db, "alice"); + let client = seed_oauth_client(&db, &user); + let tokens = scopes + .iter() + .map(|scope| seed_oauth_access_token_with_shared_key_hash(&db, &client, &user, scope, None)) + .collect(); + let project_dir = tmp.path().join("project"); + std::fs::create_dir(&project_dir).unwrap(); + std::fs::write(project_dir.join("README.md"), "hello\n").unwrap(); + let runtime = Arc::new(runtime_with_local_project(&project_dir, "demo")); + let service = Service::new(build_projects_router(config, db, runtime)); + (tmp, service, tokens) +} + fn phase2_oauth_service(scopes: &str) -> (tempfile::TempDir, salvo::Service, String) { - phase2_oauth_service_with_shared_key_hash(scopes, None) + let (tmp, service, mut tokens) = phase2_oauth_service_with_scopes(&[scopes]); + (tmp, service, tokens.pop().unwrap()) } fn phase2_oauth_service_with_shared_key_hash( @@ -570,6 +590,17 @@ fn phase2_service() -> (tempfile::TempDir, salvo::Service) { (_tmp, service) } +async fn http_tool_call(service: &Service, body: Value) -> (StatusCode, Value) { + let mut response = TestClient::post("http://localhost/api/tools/call") + .bearer_auth("secret") + .json(&body) + .send(service) + .await; + let status = effective_status(&response); + let body = response.take_json::().await.unwrap(); + (status, body) +} + #[tokio::test] async fn flattened_tool_manifest_audit_intent_survives_null_params_wrapper() { let (tool, params) = extract_tool_call(&json!({ @@ -1206,60 +1237,77 @@ async fn http_tools_list_supports_bounded_summary_request() { } #[tokio::test] -async fn http_tools_call_run_codex_is_unknown_without_creating_job() { +async fn http_tools_call_rejects_malformed_and_unknown_request_matrix() { let (_tmp, service) = phase2_service(); - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({ - "tool": "run_codex", - "params": { - "project": "demo", - "prompt": "summarize" - } - })) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["status"], 400); - let err = body["error"].as_str().unwrap(); - assert!(err.contains("unknown tool 'run_codex'"), "{err}"); - assert_eq!( - err.matches("run_codex").count(), - 1, - "unknown-tool error must not advertise removed run_codex: {err}" - ); + let cases = vec![ + ( + "unknown tool", + json!({"tool": "definitely_not_a_tool"}), + vec!["definitely_not_a_tool"], + false, + ), + ( + "removed run_codex", + json!({ + "tool": "run_codex", + "params": {"project": "demo", "prompt": "summarize"} + }), + vec!["run_codex"], + true, + ), + ( + "missing required project", + json!({"tool": "run_shell", "params": {"command": "echo"}}), + vec!["run_shell", "project"], + false, + ), + ( + "wrong project type", + json!({"tool": "run_shell", "params": {"project": 123, "command": "echo"}}), + vec!["run_shell"], + false, + ), + ( + "missing outer tool", + json!({"params": {}}), + vec!["tool"], + false, + ), + ]; - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({"tool": "list_jobs", "params": {}})) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::OK); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["success"], true); - assert_eq!(body["output"]["jobs"].as_array().unwrap().len(), 0); + for (label, request, expected_fragments, verify_no_job) in cases { + let (status, body) = http_tool_call(&service, request).await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{label}: {body}"); + let error = body["error"].as_str().unwrap_or(""); + for fragment in expected_fragments { + assert!( + error.contains(fragment), + "{label}: error must contain {fragment:?}: {error}" + ); + } + if verify_no_job { + let (status, jobs) = + http_tool_call(&service, json!({"tool": "list_jobs", "params": {}})).await; + assert_eq!(status, StatusCode::OK, "{jobs}"); + assert_eq!(jobs["success"], true); + assert!(jobs["output"]["jobs"].as_array().unwrap().is_empty()); + } + } } #[tokio::test] async fn http_tools_call_accepts_omitted_null_and_alias_params() { - // `params` may be omitted or null, and `arguments` is accepted as a - // compatibility alias for `params`. + // Wrapper compatibility is HTTP-owned; ToolRuntime owns list_tools parsing. let (_tmp, service) = phase2_service(); - for body in [ + for request in [ json!({"tool": "list_tools"}), json!({"tool": "list_tools", "params": null}), json!({"tool": "list_tools", "arguments": null}), ] { - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&body) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::OK, "body: {body}"); - let out: Value = resp.take_json().await.unwrap(); - assert_eq!(out["success"], true, "body: {body}"); - assert!(out["output"]["tools"].is_array(), "body: {body}"); + let (status, body) = http_tool_call(&service, request.clone()).await; + assert_eq!(status, StatusCode::OK, "request: {request}"); + assert_eq!(body["success"], true, "request: {request}"); + assert!(body["output"]["tools"].is_array(), "request: {request}"); } } @@ -1663,171 +1711,43 @@ async fn session_summary_bounds_event_limit() { #[tokio::test] async fn http_tools_call_params_wins_over_arguments() { - // When both params and arguments are present, params wins. Use a tool - // whose params shape we can distinguish: git_diff_summary takes a - // `project`. The runtime returns a structured error for an unknown - // project, but the project string from `params` is what gets routed, - // so we assert the error names the params project (not the arguments - // one). + // `params` precedence is an HTTP wrapper contract; ToolRuntime owns the tool semantics. let (_tmp, service) = phase2_service(); - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({ + let (status, body) = http_tool_call( + &service, + json!({ "tool": "git_diff_summary", "params": {"project": "agent:params-wins:p"}, "arguments": {"project": "agent:arguments-loses:p"}, - })) - .send(&service) - .await; - // Authenticated + dispatched to ToolRuntime (structured error, not 401). - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); + }), + ) + .await; + assert_eq!(status, StatusCode::BAD_REQUEST); assert_eq!(body["success"], false); - let err = body["error"].as_str().unwrap(); - assert!( - err.contains("params-wins"), - "params must win over arguments; error was: {}", - err - ); - assert!( - !err.contains("arguments-loses"), - "arguments must not be used when params present; error was: {}", - err - ); + let error = body["error"].as_str().unwrap(); + assert!(error.contains("params-wins"), "{error}"); + assert!(!error.contains("arguments-loses"), "{error}"); } #[tokio::test] -async fn http_tools_call_unknown_tool_returns_useful_error() { - let (_tmp, service) = phase2_service(); - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({"tool": "definitely_not_a_tool"})) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - let err = body["error"].as_str().unwrap(); - assert!( - err.contains("definitely_not_a_tool"), - "error must name the tool" - ); - // Must point the caller at discovery and list available tools. - assert!( - err.contains("listRuntimeTools") || err.contains("list_tools"), - "error should hint at discovery: {}", - err - ); - assert!( - err.contains("git_diff_summary"), - "error should list available tools: {}", - err - ); - // Must not leak secrets / config artifacts. - let lower = err.to_lowercase(); - for forbidden in [ - "token", - "authorization", - "agent.toml", - "webcodex.env", - "secret", - ] { - assert!( - !lower.contains(forbidden), - "unknown-tool error must not leak '{}': {}", - forbidden, - err - ); - } -} - -#[tokio::test] -async fn http_tools_call_missing_required_field_names_tool_and_field() { - let (_tmp, service) = phase2_service(); - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({"tool": "run_shell", "params": {"command": "echo"}})) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - let err = body["error"].as_str().unwrap(); - assert!( - err.contains("run_shell"), - "error must name the tool: {}", - err - ); - assert!( - err.contains("project"), - "error must name the missing field: {}", - err - ); -} - -#[tokio::test] -async fn http_tools_call_wrong_field_type_names_tool() { - let (_tmp, service) = phase2_service(); - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({"tool": "run_shell", "params": {"project": 123, "command": "echo"}})) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - let err = body["error"].as_str().unwrap(); - assert!( - err.contains("run_shell"), - "wrong-type error must name the tool: {}", - err - ); -} - -#[tokio::test] -async fn http_tools_call_missing_tool_field_returns_field_error() { - let (_tmp, service) = phase2_service(); - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({"params": {}})) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - let err = body["error"].as_str().unwrap(); - assert!( - err.contains("tool"), - "error must mention the missing 'tool' field: {}", - err - ); -} - -#[tokio::test] -async fn http_tools_call_generic_path_dispatches_git_tools() { - // callRuntimeTool routes these tools to the runtime. With an unknown - // agent project the runtime returns a structured error (not a 401/404), - // proving the generic path deserializes + dispatches each tool. +async fn http_tools_call_generic_path_dispatches_representative_project_tools() { + // One read-side and one write-side tool are sufficient to prove the generic + // extraction -> ToolCall -> ToolRuntime -> HTTP ToolResult path. let (_tmp, service) = phase2_service(); for (tool, params) in [ ("git_diff_summary", json!({"project": "agent:nope:nope"})), ( - "git_log", - json!({"project": "agent:nope:nope", "limit": 5, "skip": 1}), - ), - ( - "show_changes", - json!({"project": "agent:nope:nope", "include_diff": false}), + "write_project_file", + json!({"project": "agent:nope:nope", "path": "x.txt", "content": "a"}), ), ] { - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({"tool": tool, "params": params})) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST, "{tool}"); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["success"], false, "{tool}"); - assert!( - body["error"].as_str().is_some_and(|e| !e.is_empty()), - "{tool} should return a structured runtime error" - ); + let (status, body) = + http_tool_call(&service, json!({"tool": tool, "params": params})).await; + assert_eq!(status, StatusCode::BAD_REQUEST, "{tool}: {body}"); + assert_eq!(body["success"], false, "{tool}: {body}"); + assert!(body["error"] + .as_str() + .is_some_and(|error| !error.is_empty())); } } @@ -1969,155 +1889,128 @@ fn assert_oauth_scope_rejected( } #[tokio::test] -async fn oauth2_tools_call_requires_runtime_read_for_list_tools_or_runtime_status() { - let (_tmp, service, token) = phase2_oauth_service("runtime:read"); - let (status, body, _) = oauth_tools_call(&service, &token, "list_tools", Value::Null).await; - assert_eq!(status, StatusCode::OK, "body: {:?}", body); - - let (_tmp, service, token) = phase2_oauth_service("project:read"); - let (status, body, challenge) = - oauth_tools_call(&service, &token, "runtime_status", Value::Null).await; - assert_oauth_scope_rejected( - status, - &body, - challenge.as_deref(), - Some(crate::auth::SCOPE_RUNTIME_READ), - ); -} - -#[tokio::test] -async fn session_tools_oauth_scope_policy() { - let (_tmp, service, token) = phase2_oauth_service("runtime:read"); - let (status, body, _) = - oauth_tools_call(&service, &token, "start_session", json!({"title": "oauth"})).await; - assert_eq!(status, StatusCode::OK, "body: {:?}", body); - let session_id = body["output"]["session_id"].as_str().unwrap(); - let (status, body, _) = oauth_tools_call( - &service, - &token, - "session_summary", - json!({"session_id": session_id}), - ) - .await; - assert_eq!(status, StatusCode::OK, "body: {:?}", body); - - let (_tmp, service, token) = phase2_oauth_service("project:read"); - let (status, body, challenge) = - oauth_tools_call(&service, &token, "start_session", json!({})).await; - assert_oauth_scope_rejected( - status, - &body, - challenge.as_deref(), - Some(crate::auth::SCOPE_RUNTIME_READ), - ); -} +async fn oauth2_tools_call_scope_matrix() { + let (_tmp, service, tokens) = phase2_oauth_service_with_scopes(&[ + crate::auth::SCOPE_RUNTIME_READ, + crate::auth::SCOPE_PROJECT_READ, + crate::auth::SCOPE_PROJECT_WRITE, + crate::auth::SCOPE_JOB_RUN, + ]); + let runtime_read = &tokens[0]; + let project_read = &tokens[1]; + let project_write = &tokens[2]; + let job_run = &tokens[3]; + + let cases = [ + ( + "list_tools", + Value::Null, + runtime_read, + project_read, + crate::auth::SCOPE_RUNTIME_READ, + ), + ( + "runtime_status", + Value::Null, + runtime_read, + project_read, + crate::auth::SCOPE_RUNTIME_READ, + ), + ( + "read_file", + json!({"project": "demo", "path": "README.md"}), + project_read, + runtime_read, + crate::auth::SCOPE_PROJECT_READ, + ), + ( + "show_changes", + json!({"project": "agent:nope:nope", "session_id": "wc_sess_missing"}), + project_read, + runtime_read, + crate::auth::SCOPE_PROJECT_READ, + ), + ( + "write_project_file", + json!({"project": "demo", "path": "README.md", "content": "new"}), + project_write, + project_read, + crate::auth::SCOPE_PROJECT_WRITE, + ), + ( + "run_shell", + json!({"project": "demo", "command": "echo hi"}), + job_run, + project_read, + crate::auth::SCOPE_JOB_RUN, + ), + ( + "run_job", + json!({"project": "demo", "command": "echo hi"}), + job_run, + project_read, + crate::auth::SCOPE_JOB_RUN, + ), + ]; -#[tokio::test] -async fn oauth2_tools_call_requires_project_read_for_read_file() { - let (_tmp, service, token) = phase2_oauth_service("project:read"); - let (status, body, _) = oauth_tools_call( - &service, - &token, - "read_file", - json!({"project": "demo", "path": "README.md"}), - ) - .await; - assert_ne!(status, StatusCode::FORBIDDEN, "body: {:?}", body); + for (tool, params, allowed_token, denied_token, required_scope) in cases { + let (status, body, _) = + oauth_tools_call(&service, allowed_token, tool, params.clone()).await; + assert_ne!(status, StatusCode::FORBIDDEN, "{tool}: {body}"); + assert_ne!(status, StatusCode::UNAUTHORIZED, "{tool}: {body}"); - let (_tmp, service, token) = phase2_oauth_service("runtime:read"); - let (status, body, challenge) = oauth_tools_call( - &service, - &token, - "read_file", - json!({"project": "demo", "path": "README.md"}), - ) - .await; - assert_oauth_scope_rejected( - status, - &body, - challenge.as_deref(), - Some(crate::auth::SCOPE_PROJECT_READ), - ); + let (status, body, challenge) = + oauth_tools_call(&service, denied_token, tool, params).await; + assert_oauth_scope_rejected(status, &body, challenge.as_deref(), Some(required_scope)); + } } #[tokio::test] -async fn oauth2_tools_call_show_changes_tool_scope_is_project_read() { - let (_tmp, service, token) = phase2_oauth_service("project:read"); - let (status, body, _) = oauth_tools_call( - &service, - &token, - "show_changes", - json!({"project": "agent:nope:nope", "session_id": "wc_sess_missing"}), - ) - .await; - assert_eq!(status, StatusCode::BAD_REQUEST, "body: {:?}", body); - assert_eq!(body["success"], false); - - let (_tmp, service, token) = phase2_oauth_service("runtime:read"); - let (status, body, challenge) = oauth_tools_call( - &service, - &token, - "show_changes", - json!({"project": "agent:nope:nope", "session_id": "wc_sess_missing"}), - ) - .await; - assert_oauth_scope_rejected(status, &body, challenge.as_deref(), Some("project:read")); -} +async fn session_tools_oauth_scope_policy() { + let (_tmp, service, tokens) = phase2_oauth_service_with_scopes(&[ + crate::auth::SCOPE_RUNTIME_READ, + crate::auth::SCOPE_PROJECT_READ, + ]); + let runtime_read = &tokens[0]; + let project_read = &tokens[1]; -#[tokio::test] -async fn oauth2_tools_call_requires_project_write_for_edit_tools() { - let (_tmp, service, token) = phase2_oauth_service("project:write"); let (status, body, _) = oauth_tools_call( &service, - &token, - "write_project_file", - json!({"project": "demo", "path": "README.md", "content": "new"}), - ) - .await; - assert_ne!(status, StatusCode::FORBIDDEN, "body: {:?}", body); - - let (_tmp, service, token) = phase2_oauth_service("project:read"); - let (status, body, challenge) = oauth_tools_call( - &service, - &token, - "write_project_file", - json!({"project": "demo", "path": "README.md", "content": "new"}), + runtime_read, + "start_session", + json!({"title": "oauth"}), ) .await; - assert_oauth_scope_rejected( - status, - &body, - challenge.as_deref(), - Some(crate::auth::SCOPE_PROJECT_WRITE), - ); -} + assert_eq!(status, StatusCode::OK, "body: {body}"); + let session_id = body["output"]["session_id"].as_str().unwrap(); -#[tokio::test] -async fn oauth2_tools_call_requires_job_run_for_run_shell_or_run_job() { - let (_tmp, service, token) = phase2_oauth_service("job:run"); - let (status, body, _) = oauth_tools_call( - &service, - &token, - "run_shell", - json!({"project": "demo", "command": "echo hi"}), - ) - .await; - assert_ne!(status, StatusCode::FORBIDDEN, "body: {:?}", body); + for (tool, params) in [ + ("session_summary", json!({"session_id": session_id})), + ( + "post_session_message", + json!({"session_id": session_id, "kind": "note", "message": "oauth metadata"}), + ), + ] { + let (status, body, _) = oauth_tools_call(&service, runtime_read, tool, params).await; + assert_eq!(status, StatusCode::OK, "{tool}: {body}"); + } - let (_tmp, service, token) = phase2_oauth_service("project:read"); - let (status, body, challenge) = oauth_tools_call( - &service, - &token, - "run_job", - json!({"project": "demo", "command": "echo hi"}), - ) - .await; - assert_oauth_scope_rejected( - status, - &body, - challenge.as_deref(), - Some(crate::auth::SCOPE_JOB_RUN), - ); + for (tool, params) in [ + ("start_session", json!({})), + ( + "post_session_message", + json!({"session_id": "wc_sess_missing", "kind": "note", "message": "denied"}), + ), + ] { + let (status, body, challenge) = + oauth_tools_call(&service, project_read, tool, params).await; + assert_oauth_scope_rejected( + status, + &body, + challenge.as_deref(), + Some(crate::auth::SCOPE_RUNTIME_READ), + ); + } } #[tokio::test] @@ -2204,43 +2097,3 @@ async fn http_tools_list_includes_phase4_edit_tools() { ); } } - -#[tokio::test] -async fn http_tools_call_dispatches_phase4_edit_tools() { - // callRuntimeTool routes write_project_file / apply_text_edits to the - // runtime. With a non-agent project the runtime returns a structured - // error (not a 401/404), proving the generic path dispatches them. - let (_tmp, service) = phase2_service(); - for (tool, params) in [ - ( - "write_project_file", - json!({"project": "agent:nope:nope", "path": "x.txt", "content": "a"}), - ), - ( - "apply_text_edits", - json!({ - "project": "agent:nope:nope", - "changes": [{ - "kind": "edit", - "path": "x.txt", - "expected_sha256": "a".repeat(64), - "edits": [{"kind": "replace_exact", "old_text": "a", "new_text": "b"}] - }] - }), - ), - ] { - let mut resp = TestClient::post("http://localhost/api/tools/call") - .bearer_auth("secret") - .json(&json!({"tool": tool, "params": params})) - .send(&service) - .await; - assert_eq!(effective_status(&resp), StatusCode::BAD_REQUEST); - let body: Value = resp.take_json().await.unwrap(); - assert_eq!(body["success"], false); - assert!( - body["error"].as_str().is_some_and(|e| !e.is_empty()), - "{} should return a structured runtime error", - tool - ); - } -} diff --git a/src/tool_runtime/sessions/tests.rs b/src/tool_runtime/sessions/tests.rs index 6c0c7c74..9cf7c011 100644 --- a/src/tool_runtime/sessions/tests.rs +++ b/src/tool_runtime/sessions/tests.rs @@ -522,102 +522,89 @@ fn write_ledger_atomic_cleans_up_temp_file_when_rename_fails() { } #[test] -fn session_execution_context_persists_and_legacy_ledgers_default_empty() { - let tmp = tempfile::tempdir().unwrap(); - let ledger = tmp.path().join("sessions.json"); - let store = persistent_store(ledger.clone()); - let expected = SessionExecutionContext { - default_cwd: Some("frontend/./src".to_string()), - default_shell: Some(ExecutionShell::Bash), - resource: None, - }; - let session = store - .start_session_with_options( - SessionCreateOptions::new( - Some("agent:oe:private-drop".to_string()), - Some("persistent context".to_string()), - SessionMode::Normal, - SessionGuards::default(), - ) - .with_execution_context(expected), - ) - .unwrap(); - assert_eq!( - session.execution_context, - SessionExecutionContext { - default_cwd: Some("frontend/src".to_string()), - default_shell: Some(ExecutionShell::Bash), - resource: None, - } - ); +fn session_execution_context_persistence_matrix_and_legacy_default() { + let cases = [ + ( + "local-normalized", + SessionExecutionContext { + default_cwd: Some("frontend/./src".to_string()), + default_shell: Some(ExecutionShell::Bash), + resource: None, + }, + SessionExecutionContext { + default_cwd: Some("frontend/src".to_string()), + default_shell: Some(ExecutionShell::Bash), + resource: None, + }, + json!({"default_cwd": "frontend/src", "default_shell": "bash"}), + true, + ), + ( + "remote-resource", + SessionExecutionContext { + default_cwd: Some("/opt/webcodex-edge".to_string()), + default_shell: None, + resource: Some("tmp".to_string()), + }, + SessionExecutionContext { + default_cwd: Some("/opt/webcodex-edge".to_string()), + default_shell: None, + resource: Some("tmp".to_string()), + }, + json!({"default_cwd": "/opt/webcodex-edge", "resource": "tmp"}), + false, + ), + ]; - store.flush_persistence(); - let raw = std::fs::read_to_string(&ledger).unwrap(); - let mut value: Value = serde_json::from_str(&raw).unwrap(); - assert_eq!( - value["sessions"][0]["execution_context"], - json!({"default_cwd": "frontend/src", "default_shell": "bash"}) - ); - let restored = SessionStore::with_persistence(ledger.clone(), 10, 10); - assert_eq!( - restored - .summary(&session.session_id, None) - .unwrap() - .execution_context, - session.execution_context - ); + for (label, input, expected, persisted_context, check_legacy_default) in cases { + let tmp = tempfile::tempdir().unwrap(); + let ledger = tmp.path().join("sessions.json"); + let store = persistent_store(ledger.clone()); + let session = store + .start_session_with_options( + SessionCreateOptions::new( + Some("agent:oe:private-drop".to_string()), + Some(format!("persistent context {label}")), + SessionMode::Normal, + SessionGuards::default(), + ) + .with_execution_context(input), + ) + .unwrap(); + assert_eq!(session.execution_context, expected, "{label}"); - value["sessions"][0] - .as_object_mut() - .unwrap() - .remove("execution_context"); - std::fs::write(&ledger, serde_json::to_vec_pretty(&value).unwrap()).unwrap(); - let legacy = SessionStore::with_persistence(ledger, 10, 10); - assert_eq!( - legacy - .summary(&session.session_id, None) - .unwrap() - .execution_context, - SessionExecutionContext::default() - ); -} + store.flush_persistence(); + let mut value: Value = serde_json::from_slice(&std::fs::read(&ledger).unwrap()).unwrap(); + assert_eq!( + value["sessions"][0]["execution_context"], persisted_context, + "{label}" + ); + let restored = SessionStore::with_persistence(ledger.clone(), 10, 10); + assert_eq!( + restored + .summary(&session.session_id, None) + .unwrap() + .execution_context, + expected, + "{label}" + ); -#[test] -fn session_ssh_resource_and_remote_default_cwd_persist_without_connection_state() { - let tmp = tempfile::tempdir().unwrap(); - let ledger = tmp.path().join("sessions.json"); - let store = persistent_store(ledger.clone()); - let context = SessionExecutionContext { - default_cwd: Some("/opt/webcodex-edge".to_string()), - default_shell: None, - resource: Some("tmp".to_string()), - }; - let session = store - .start_session_with_options( - SessionCreateOptions::new( - Some("agent:oe:private-drop".to_string()), - Some("remote context".to_string()), - SessionMode::Normal, - SessionGuards::default(), - ) - .with_execution_context(context.clone()), - ) - .unwrap(); - assert_eq!(session.execution_context, context); - store.flush_persistence(); - let value: Value = serde_json::from_slice(&std::fs::read(&ledger).unwrap()).unwrap(); - assert_eq!( - value["sessions"][0]["execution_context"], - json!({"default_cwd": "/opt/webcodex-edge", "resource": "tmp"}) - ); - let restored = SessionStore::with_persistence(ledger, 10, 10); - assert_eq!( - restored - .summary(&session.session_id, None) - .unwrap() - .execution_context, - context - ); + if check_legacy_default { + value["sessions"][0] + .as_object_mut() + .unwrap() + .remove("execution_context"); + std::fs::write(&ledger, serde_json::to_vec_pretty(&value).unwrap()).unwrap(); + let legacy = SessionStore::with_persistence(ledger, 10, 10); + assert_eq!( + legacy + .summary(&session.session_id, None) + .unwrap() + .execution_context, + SessionExecutionContext::default() + ); + } + } } #[test] @@ -839,63 +826,6 @@ fn archived_session_rejects_execution_context_update() { ); } -#[test] -fn session_messages_survive_restore() { - let tmp = tempfile::tempdir().unwrap(); - let ledger = tmp.path().join("sessions.json"); - let store = persistent_store(ledger.clone()); - let session = store.start_session(None, Some("discussion".to_string())); - post_message( - &store, - &session.session_id, - SessionMessageKind::Guidance, - "keep OpenAPI operation count stable", - ); - post_message( - &store, - &session.session_id, - SessionMessageKind::Progress, - "ledger snapshot wired", - ); - - let restored = flush_and_restore(&store, ledger); - let messages = restored - .list_messages(&session.session_id, ListSessionMessagesFilter::default()) - .unwrap(); - assert_eq!(messages.len(), 2); - assert_eq!(messages[0].message, "ledger snapshot wired"); - assert_eq!(messages[1].kind, SessionMessageKind::Guidance); - let discussion = restored - .discussion_summary(&session.session_id, Some(10)) - .unwrap(); - assert_eq!(discussion.counts.total, 2); - assert_eq!(discussion.counts.guidance, 1); - assert_eq!(discussion.counts.progress, 1); -} - -#[test] -fn session_events_survive_restore() { - let tmp = tempfile::tempdir().unwrap(); - let ledger = tmp.path().join("sessions.json"); - let store = persistent_store(ledger.clone()); - let session = store.start_session(None, Some("events".to_string())); - let start = store.record_tool_call_started( - Some(&session.session_id), - SessionTransport::Api, - "git_log", - &json!({"project": "agent:oe:private-drop", "limit": 1}), - ); - store.record_tool_call_finished(start, true, &json!({}), None, None); - - let restored = flush_and_restore(&store, ledger); - let summary = restored.summary(&session.session_id, Some(10)).unwrap(); - assert_eq!(summary.events.len(), 2); - assert_eq!(summary.counts.tool_calls, 1); - assert_eq!(summary.counts.succeeded, 1); - assert_eq!(summary.counts.git_like, 1); - assert_eq!(summary.events[1].tool_name, "git_log"); -} - #[test] fn persistent_shell_evidence_survives_restore_without_command_or_output() { let tmp = tempfile::tempdir().unwrap(); @@ -2903,8 +2833,8 @@ fn concurrent_persistence_reloads_current_snapshot_before_write() { ); }); - let mut newer_message_visible = false; - for _ in 0..100 { + let visibility_deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + loop { let messages = store .list_messages(&session.session_id, ListSessionMessagesFilter::default()) .unwrap(); @@ -2912,12 +2842,14 @@ fn concurrent_persistence_reloads_current_snapshot_before_write() { .iter() .any(|message| message.message == "newer mutation") { - newer_message_visible = true; break; } + assert!( + std::time::Instant::now() < visibility_deadline, + "newer persistence mutation did not become visible within 5 seconds" + ); std::thread::sleep(std::time::Duration::from_millis(10)); } - assert!(newer_message_visible); allow_old_write_tx.send(()).unwrap(); delayed_write.join().unwrap(); @@ -2984,30 +2916,6 @@ fn post_message( .unwrap() } -#[test] -fn post_session_message_creates_message() { - let store = SessionStore::default(); - let session = store.start_session(None, None); - let message = store - .post_message(PostSessionMessageInput { - session_id: session.session_id.clone(), - kind: SessionMessageKind::Guidance, - message: "Keep this behind callRuntimeTool.".to_string(), - tags: vec!["openapi".to_string(), "constraint".to_string()], - reply_to: None, - priority: SessionMessagePriority::High, - }) - .unwrap(); - - assert!(message.message_id.starts_with(MESSAGE_ID_PREFIX)); - assert_eq!(message.session_id, session.session_id); - assert_eq!(message.kind, SessionMessageKind::Guidance); - assert_eq!(message.status, SessionMessageStatus::Open); - assert_eq!(message.priority, SessionMessagePriority::High); - assert_eq!(message.message, "Keep this behind callRuntimeTool."); - assert_eq!(message.tags, vec!["openapi", "constraint"]); -} - #[test] fn list_session_messages_filters_and_clamps_limit() { let store = SessionStore::default(); @@ -3057,35 +2965,6 @@ fn list_session_messages_filters_and_clamps_limit() { assert_eq!(open[0].message, "r1"); } -#[test] -fn resolve_session_message_is_idempotent() { - let store = SessionStore::default(); - let session = store.start_session(None, None); - let message = post_message( - &store, - &session.session_id, - SessionMessageKind::Todo, - "fix it", - ); - - let resolved = store - .resolve_message( - &session.session_id, - &message.message_id, - Some("Done".to_string()), - ) - .unwrap(); - assert_eq!(resolved.status, SessionMessageStatus::Resolved); - assert!(resolved.resolved_at.is_some()); - assert_eq!(resolved.resolution.as_deref(), Some("Done")); - - let resolved_again = store - .resolve_message(&session.session_id, &message.message_id, None) - .unwrap(); - assert_eq!(resolved_again.status, SessionMessageStatus::Resolved); - assert_eq!(resolved_again.resolution.as_deref(), Some("Done")); -} - #[test] fn session_message_unknown_errors_are_explicit() { let store = SessionStore::default(); @@ -3455,7 +3334,7 @@ fn stale_binding_is_cleared_when_session_missing() { } #[test] -fn message_post_and_resolve_round_trip_through_store() { +fn session_message_create_list_and_resolve_contract() { let store = SessionStore::default(); let session = store.start_session(None, None); let posted = store @@ -3463,12 +3342,18 @@ fn message_post_and_resolve_round_trip_through_store() { session_id: session.session_id.clone(), kind: SessionMessageKind::Todo, message: "do the thing".to_string(), - tags: vec!["work".to_string()], + tags: vec!["work".to_string(), "constraint".to_string()], reply_to: None, priority: SessionMessagePriority::High, }) .unwrap(); + assert!(posted.message_id.starts_with(MESSAGE_ID_PREFIX)); + assert_eq!(posted.session_id, session.session_id); + assert_eq!(posted.kind, SessionMessageKind::Todo); assert_eq!(posted.status, SessionMessageStatus::Open); + assert_eq!(posted.priority, SessionMessagePriority::High); + assert_eq!(posted.message, "do the thing"); + assert_eq!(posted.tags, vec!["work", "constraint"]); let listed = store .list_messages( @@ -3496,17 +3381,23 @@ fn message_post_and_resolve_round_trip_through_store() { assert_eq!(resolved.resolution.as_deref(), Some("shipped")); let first_resolved_at = resolved.resolved_at.expect("resolved_at set"); - // Resolved messages are not reopened by a second resolve. - let again = store + let idempotent = store + .resolve_message(&session.session_id, &posted.message_id, None) + .unwrap(); + assert_eq!(idempotent.status, SessionMessageStatus::Resolved); + assert_eq!(idempotent.resolved_at, Some(first_resolved_at)); + assert_eq!(idempotent.resolution.as_deref(), Some("shipped")); + + let updated = store .resolve_message( &session.session_id, &posted.message_id, Some("still done".to_string()), ) .unwrap(); - assert_eq!(again.status, SessionMessageStatus::Resolved); - assert_eq!(again.resolved_at, Some(first_resolved_at)); - assert_eq!(again.resolution.as_deref(), Some("still done")); + assert_eq!(updated.status, SessionMessageStatus::Resolved); + assert_eq!(updated.resolved_at, Some(first_resolved_at)); + assert_eq!(updated.resolution.as_deref(), Some("still done")); let open = store .list_messages( @@ -3579,7 +3470,7 @@ fn read_only_guards_block_write_and_shell_classifications() { } #[test] -fn ledger_round_trip_preserves_session_events_and_messages() { +fn ledger_round_trip_preserves_session_state_events_and_messages() { let dir = tempfile::tempdir().unwrap(); let ledger = dir.path().join("sessions.json"); let store = SessionStore::with_persistence(&ledger, 10, 50); @@ -3598,6 +3489,12 @@ fn ledger_round_trip_preserves_session_events_and_messages() { ) .unwrap(); store.record_tool_call_finished(Some(start), true, &json!({}), None, None); + post_message( + &store, + &session.session_id, + SessionMessageKind::Guidance, + "keep OpenAPI operation count stable", + ); post_message( &store, &session.session_id, @@ -3615,12 +3512,24 @@ fn ledger_round_trip_preserves_session_events_and_messages() { assert!(!summary.guards.deny_shell_tools); assert_eq!(summary.lifecycle, SessionLifecycle::Active); assert_eq!(summary.counts.tool_calls, 1); + assert_eq!(summary.counts.succeeded, 1); assert!(summary .events .iter() - .any(|event| event.kind == "tool_call_finished")); - assert_eq!(summary.messages.total, 1); - assert_eq!(summary.messages.progress, 1); + .any(|event| event.kind == "tool_call_finished" && event.tool_name == "read_file")); + + let messages = restored + .list_messages(&session.session_id, ListSessionMessagesFilter::default()) + .unwrap(); + assert_eq!(messages.len(), 2); + assert_eq!(messages[0].message, "checkpoint"); + assert_eq!(messages[1].kind, SessionMessageKind::Guidance); + let discussion = restored + .discussion_summary(&session.session_id, Some(10)) + .unwrap(); + assert_eq!(discussion.counts.total, 2); + assert_eq!(discussion.counts.guidance, 1); + assert_eq!(discussion.counts.progress, 1); // This session was never bound, so the additive ledger field stays empty. let key = test_binding_key("proj"); @@ -3827,20 +3736,6 @@ fn durable_current_binding_restore_enforces_bounded_count() { assert!(restored.summary(&session.session_id, None).is_some()); } -#[test] -fn lifecycle_ledger_round_trip_preserves_active() { - let tmp = tempfile::tempdir().unwrap(); - let ledger = tmp.path().join("sessions.json"); - let store = persistent_store(ledger.clone()); - let session = store.start_session(None, Some("round trip".to_string())); - assert_eq!(session.lifecycle, SessionLifecycle::Active); - - let restored = flush_and_restore(&store, ledger); - let summary = restored.summary(&session.session_id, Some(10)).unwrap(); - assert_eq!(summary.lifecycle, SessionLifecycle::Active); - assert_eq!(summary.session_id, session.session_id); -} - #[test] fn persisted_session_record_serde_defaults_missing_lifecycle() { // Direct serde check: omit lifecycle entirely; deserialize succeeds as Active. diff --git a/src/tool_runtime/tests/apply_text_edits.rs b/src/tool_runtime/tests/apply_text_edits.rs index a978a835..7e94d824 100644 --- a/src/tool_runtime/tests/apply_text_edits.rs +++ b/src/tool_runtime/tests/apply_text_edits.rs @@ -2,9 +2,7 @@ use super::super::*; use super::support::*; -use crate::shell_protocol::{ - ShellAgentPollRequest, ShellAgentResultRequest, ShellClientCapabilities, -}; +use crate::shell_protocol::{ShellAgentResultRequest, ShellClientCapabilities}; use serde_json::Value; #[test] @@ -343,23 +341,7 @@ async fn apply_text_edits_dry_run_does_not_write() { .await }); - let mut req = None; - for _ in 0..20 { - req = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "ate-dry".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - if req.is_some() { - break; - } - tokio::task::yield_now().await; - } - let req = req.expect("apply_text_edits should enqueue an agent file op"); + let req = wait_for_patch_agent_request(&runtime, "ate-dry").await; assert_eq!(req.kind, "file_apply_text_edits"); // The payload carries dry_run and the edits. let payload: Value = serde_json::from_str(req.content.as_deref().unwrap()).unwrap(); @@ -486,23 +468,7 @@ async fn apply_text_edits_session_event_summary() { .await }); - let mut req = None; - for _ in 0..20 { - req = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "ate-sess".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - if req.is_some() { - break; - } - tokio::task::yield_now().await; - } - let req = req.expect("apply_text_edits should enqueue an agent file op"); + let req = wait_for_patch_agent_request(&runtime, "ate-sess").await; assert_eq!(req.kind, "file_apply_text_edits"); runtime .shell_clients diff --git a/src/tool_runtime/tests/coding_task.rs b/src/tool_runtime/tests/coding_task.rs index 81ffae0d..8d9836a5 100644 --- a/src/tool_runtime/tests/coding_task.rs +++ b/src/tool_runtime/tests/coding_task.rs @@ -2849,114 +2849,20 @@ async fn finish_coding_task_summary_only_treats_read_failure_as_historical_non_a #[tokio::test] async fn finish_coding_task_includes_active_jobs_warning_without_logs() { - let tmp = tempfile::tempdir().unwrap(); - init_git_repo(tmp.path()); - commit_file(tmp.path(), "README.md", "hello\n", "add readme"); - let runtime = test_runtime(); - let caps = ShellClientCapabilities { - shell: true, - git: true, - async_shell_jobs: true, - internal_posix_script: true, - ..Default::default() - }; - let project_path = tmp.path().to_string_lossy().to_string(); - let auth = open_auth_context(); - register_agent_projects_for_auth( - &runtime, - "coding-finish-jobs", - &auth, - caps, - vec![registered_project("demo", &project_path)], - ) - .await; - let project = "agent:coding-finish-jobs:demo".to_string(); - let start = runtime - .dispatch_with_auth( - ToolCall::StartCodingTask { - project: project.clone(), - client_id: None, - path: None, - temporary_project_name: None, - title: Some("finish active jobs".to_string()), - mode: SessionMode::Normal, - detail: Default::default(), - deny_write_tools: false, - deny_shell_tools: false, - resume_session_id: None, - bind_current: false, - new_session: false, - execution_context: None, - }, - Some(&auth), - ) - .await; - assert!(start.success, "{:?}", start.error); - let session_id = start.output["session"]["session_id"] - .as_str() - .unwrap() - .to_string(); - - let run = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: project.clone(), - command: "printf secret-job-output".to_string(), - session_id: Some(session_id.clone()), - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - Some(&auth), - ) - .await; - assert!(run.success, "{:?}", run.error); - let queued_job = next_agent_request_for_client(&runtime, "coding-finish-jobs") - .await - .expect("run_job should enqueue a job request"); - assert_eq!(queued_job.kind, "start_job"); - - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let session_id = session_id.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth( - ToolCall::FinishCodingTask { - project, - session_id, - summary_only: false, - include_diff: Some(false), - include_workspace: None, - include_hygiene: Some(false), - include_handoff: Some(false), - include_validation_summary: Some(false), - }, - Some(&auth), - ) - .await - } - }); - let req = next_agent_request_for_client(&runtime, "coding-finish-jobs") - .await - .expect("finish_coding_task should inspect changes through the agent"); - assert_internal_posix_script_contains(&req, "git status --porcelain=v1 -b"); - let show_changes_stdout = "## main\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nabc123\0abc123\0add readme\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n"; - complete_patch_agent_request_for_instance( - &runtime, - "coding-finish-jobs", - "inst-coding-finish-jobs", - &req.request_id, - 0, - show_changes_stdout, - "", + let fixture = finish_summary_fixture("coding-finish-jobs").await; + let job_id = "22222222-3333-4444-5555-666666666661"; + seed_session_projection_job( + &fixture.runtime, + fixture._tmp.path(), + job_id, + &fixture.project, + &fixture.session_id, + "running", + "secret-job-output\n", ) .await; - let result = task.await.unwrap(); + let result = finish_coding_task_jobs_projection(&fixture).await; assert!(result.success, "{:?}", result.error); assert_eq!(result.output["jobs"]["active_count"], 1); assert_eq!(result.output["jobs"]["running_count"], 1); @@ -2966,10 +2872,7 @@ async fn finish_coding_task_includes_active_jobs_warning_without_logs() { assert_eq!(result.output["jobs"]["nonblocking_active_count"], 0); assert_eq!(result.output["task_outcome"]["status"], "fail"); assert_eq!(result.output["task_outcome"]["blocking"], true); - assert_eq!( - result.output["jobs"]["recent"][0]["job_id"], - run.output["job_id"] - ); + assert_eq!(result.output["jobs"]["recent"][0]["job_id"], job_id); assert!(result.output["final_warnings"] .as_array() .unwrap() @@ -2982,133 +2885,20 @@ async fn finish_coding_task_includes_active_jobs_warning_without_logs() { #[tokio::test] async fn finish_coding_task_treats_stop_requested_jobs_as_nonblocking() { - let tmp = tempfile::tempdir().unwrap(); - init_git_repo(tmp.path()); - commit_file(tmp.path(), "README.md", "hello\n", "add readme"); - let runtime = test_runtime(); - let caps = ShellClientCapabilities { - shell: true, - git: true, - async_shell_jobs: true, - internal_posix_script: true, - ..Default::default() - }; - let project_path = tmp.path().to_string_lossy().to_string(); - let auth = open_auth_context(); - register_agent_projects_for_auth( - &runtime, - "coding-finish-stop-pending", - &auth, - caps, - vec![registered_project("demo", &project_path)], - ) - .await; - let project = "agent:coding-finish-stop-pending:demo".to_string(); - let start = runtime - .dispatch_with_auth( - ToolCall::StartCodingTask { - project: project.clone(), - client_id: None, - path: None, - temporary_project_name: None, - title: Some("finish stop pending".to_string()), - mode: SessionMode::Normal, - detail: Default::default(), - deny_write_tools: false, - deny_shell_tools: false, - resume_session_id: None, - bind_current: false, - new_session: false, - execution_context: None, - }, - Some(&auth), - ) - .await; - assert!(start.success, "{:?}", start.error); - let session_id = start.output["session"]["session_id"] - .as_str() - .unwrap() - .to_string(); - - let run = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: project.clone(), - command: "printf stop-pending-secret-output".to_string(), - session_id: Some(session_id.clone()), - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - Some(&auth), - ) - .await; - assert!(run.success, "{:?}", run.error); - let job_id = run.output["job_id"].as_str().unwrap().to_string(); - let start_job = next_agent_request_for_client(&runtime, "coding-finish-stop-pending") - .await - .expect("run_job should enqueue a job request"); - assert_eq!(start_job.kind, "start_job"); - - let stop = runtime - .dispatch_with_auth( - ToolCall::StopJob { - project: project.clone(), - job_id: job_id.clone(), - session_id: Some(session_id.clone()), - confirm: true, - }, - Some(&auth), - ) - .await; - assert!(stop.success, "{:?}", stop.error); - assert_eq!(stop.output["status_after"], "stop_requested"); - let stop_req = next_agent_request_for_client(&runtime, "coding-finish-stop-pending") - .await - .expect("stop_job should enqueue a stop request"); - assert_eq!(stop_req.kind, "stop_job"); - - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let session_id = session_id.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth( - ToolCall::FinishCodingTask { - project, - session_id, - summary_only: false, - include_diff: Some(false), - include_workspace: None, - include_hygiene: Some(false), - include_handoff: Some(false), - include_validation_summary: Some(false), - }, - Some(&auth), - ) - .await - } - }); - let req = next_agent_request_for_client(&runtime, "coding-finish-stop-pending") - .await - .expect("finish_coding_task should inspect changes through the agent"); - assert_internal_posix_script_contains(&req, "git status --porcelain=v1 -b"); - let show_changes_stdout = "## main\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nabc123\0abc123\0add readme\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n"; - complete_patch_agent_request_for_instance( - &runtime, - "coding-finish-stop-pending", - "inst-coding-finish-stop-pending", - &req.request_id, - 0, - show_changes_stdout, - "", + let fixture = finish_summary_fixture("coding-finish-stop-pending").await; + let job_id = "22222222-3333-4444-5555-666666666662"; + seed_session_projection_job( + &fixture.runtime, + fixture._tmp.path(), + job_id, + &fixture.project, + &fixture.session_id, + "stop_requested", + "stop-pending-secret-output\n", ) .await; - let result = task.await.unwrap(); + let result = finish_coding_task_jobs_projection(&fixture).await; assert!(result.success, "{:?}", result.error); assert_eq!(result.output["jobs"]["active_count"], 1); assert_eq!(result.output["jobs"]["running_count"], 0); @@ -3355,6 +3145,44 @@ async fn finish_summary_fixture(client_id: &'static str) -> FinishSummaryFixture } } +async fn finish_coding_task_jobs_projection(fixture: &FinishSummaryFixture) -> ToolResult { + let task = tokio::spawn({ + let runtime = fixture.runtime.clone(); + let project = fixture.project.clone(); + let session_id = fixture.session_id.clone(); + let auth = fixture.auth.clone(); + async move { + runtime + .dispatch_with_auth( + ToolCall::FinishCodingTask { + project, + session_id, + summary_only: false, + include_diff: Some(false), + include_workspace: None, + include_hygiene: Some(false), + include_handoff: Some(false), + include_validation_summary: Some(false), + }, + Some(&auth), + ) + .await + } + }); + let request = wait_for_patch_agent_request(&fixture.runtime, fixture.client_id).await; + assert_internal_posix_script_contains(&request, "git status --porcelain=v1 -b"); + complete_patch_agent_request( + &fixture.runtime, + fixture.client_id, + &request.request_id, + 0, + "## main\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nabc123\0abc123\0add readme\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n", + "", + ) + .await; + task.await.unwrap() +} + async fn finish_coding_task_summary_only_with_agent( runtime: &ToolRuntime, client_id: &str, diff --git a/src/tool_runtime/tests/continuation_feedback.rs b/src/tool_runtime/tests/continuation_feedback.rs index caf859cc..131150f2 100644 --- a/src/tool_runtime/tests/continuation_feedback.rs +++ b/src/tool_runtime/tests/continuation_feedback.rs @@ -1777,30 +1777,6 @@ async fn start_coding_task_continuation_describes_previous_attempt_not_empty_new feedback["attempt"]["instruction"]["excerpt"], "instruction A" ); - assert_eq!(feedback["attempt"]["instruction"]["truncated"], false); - // A's attempt had a successful write tool call and a failed validation run; - // both count as meaningful tool calls. - assert_eq!(feedback["attempt"]["activity"]["meaningful_tool_calls"], 2); - assert_eq!(feedback["attempt"]["activity"]["successful_tool_calls"], 1); - assert_eq!(feedback["attempt"]["activity"]["failed_tool_calls"], 1); - assert_eq!(feedback["attempt"]["validation"]["latest_status"], "failed"); - assert_eq!(feedback["attempt"]["activity"]["unresolved_failures"], 1); - assert_eq!(feedback["attempt"]["validation"]["total_open_failures"], 1); - assert_eq!( - feedback["attempt"]["validation"]["open_failures"][0]["kind"], - "test" - ); - assert_eq!( - feedback["attempt"]["validation"]["open_failures"][0]["name"], - "tests::a" - ); - assert!(feedback["attempt"]["suggested_next_actions"] - .as_array() - .unwrap() - .iter() - .any(|action| action == "fix failing test tests::a")); - assert_eq!(feedback["attempt"]["changes"]["total_changed_paths"], 1); - // Instruction B must be appended exactly once. let summary = runtime.sessions.summary(&session_id, Some(200)).unwrap(); let instructions: Vec<&str> = summary @@ -1858,19 +1834,6 @@ async fn start_coding_task_fresh_session_continuation_is_not_applicable() { let feedback = &first.output["continuation_feedback"]; assert_eq!(feedback["status"], "not_applicable"); assert_eq!(feedback["reason_code"], "fresh_session"); - assert_eq!( - feedback["attempt"]["exploration"], - json!({ - "observed_paths": [], - "total_observed_paths": 0, - "truncated": false, - "read_count": 0, - "search_count": 0, - "navigation_count": 0, - "latest_tool": null, - "complete": true - }) - ); } // ========================================================================= @@ -2172,133 +2135,92 @@ async fn finish_coding_task_continuation_matches_handoff_attempt_without_rerunni let project = register_agent_project_at_path(&runtime, "finish-agent", "demo", dir.path()).await; let auth = auth_context(None, true); - - // Start a real coding session with instruction A, then do real work. - let start = dispatch_start_coding_task_in_window( + let session = runtime.sessions.start_session( + Some(project.clone()), + Some("finish continuation".to_string()), + ); + let session_id = session.session_id.clone(); + add_instruction_for( &runtime, - "finish-agent", - coding_call(&project, "instruction A", None, true), - Some(&auth), - "finish-window", - ) - .await; - assert!(start.success, "{:?}", start.error); - let session_id = start.output["session"]["session_id"] - .as_str() - .unwrap() - .to_string(); - record_write(&runtime, &session_id, &["src/lib.rs"]); - record_validation_event( + &session_id, + "instruction A", + SessionMode::Normal, + &project, + ); + record_write_for(&runtime, &session_id, &project, &["src/lib.rs"]); + record_validation_event_for( &runtime, &session_id, "cargo_test", false, test_output(0, 1, 0, &["tests::a"]), + &project, ); - let events_before = runtime + let handoff = runtime + .session_handoff_summary( + session_id.clone(), + Some(project.clone()), + Some(false), + Some(false), + Some(false), + false, + Some(20), + Some(&auth), + ) + .await; + assert!(handoff.success, "{:?}", handoff.error); + let handoff_feedback = handoff.output["continuation_feedback"].clone(); + assert_eq!(handoff_feedback["status"], "available"); + + let events_before_finish = runtime .sessions .summary(&session_id, Some(200)) .unwrap() .events .len(); - - // finish_coding_task with summary_only=false exposes continuation_feedback, - // reuses the same closeout helper as handoff (same attempt boundary), and - // must not re-run validation. - let finish = dispatch_start_coding_task_in_window( - &runtime, - "finish-agent", - ToolCall::FinishCodingTask { - project: project.clone(), - session_id: session_id.clone(), - summary_only: false, - include_diff: Some(false), - include_workspace: Some(false), - include_hygiene: Some(false), - include_handoff: Some(false), - include_validation_summary: Some(false), - }, - Some(&auth), - "finish-window", - ) - .await; + let finish = runtime + .dispatch_with_auth( + ToolCall::FinishCodingTask { + project: project.clone(), + session_id: session_id.clone(), + summary_only: false, + include_diff: Some(false), + include_workspace: Some(false), + include_hygiene: Some(false), + include_handoff: Some(false), + include_validation_summary: Some(false), + }, + Some(&auth), + ) + .await; assert!(finish.success, "{:?}", finish.error); + let finish_feedback = &finish.output["continuation_feedback"]; + + for pointer in [ + "/status", + "/attempt/boundary", + "/attempt/instruction", + "/attempt/validation/latest_status", + "/attempt/changes/total_changed_paths", + ] { + assert_eq!( + finish_feedback.pointer(pointer), + handoff_feedback.pointer(pointer), + "finish and handoff must project the same established attempt at {pointer}" + ); + } - let feedback = &finish.output["continuation_feedback"]; - assert_eq!(feedback["status"], "available"); - assert_eq!( - feedback["attempt"]["boundary"]["source"], - "task_instruction" - ); - // Same attempt boundary as a handoff would report: A's real work. The - // finish path internally records a `show_changes` tool call against the - // session, so meaningful_tool_calls counts that derivation call in - // addition to A's write + validation; assert the user-visible work is - // present (>= 2) and the proven failure/changed-path counts are exact. - assert!( - feedback["attempt"]["activity"]["meaningful_tool_calls"].as_u64() >= Some(2), - "finish attempt must count A's write + validation work" - ); - assert_eq!(feedback["attempt"]["activity"]["failed_tool_calls"], 1); - assert_eq!(feedback["attempt"]["changes"]["total_changed_paths"], 1); - // The continuation feedback itself never leaks raw command/output. - let feedback_serialized = serde_json::to_string(feedback).unwrap(); - assert!( - !feedback_serialized.contains("command_summary") - && !feedback_serialized.contains("stdout_tail"), - "finish continuation feedback leaked raw command/output" - ); - - // A separate summary_only=true call must not surface raw output fields. - let finish_summary = dispatch_start_coding_task_in_window( - &runtime, - "finish-agent", - ToolCall::FinishCodingTask { - project: project.clone(), - session_id: session_id.clone(), - summary_only: true, - include_diff: Some(false), - include_workspace: Some(false), - include_hygiene: Some(false), - include_handoff: Some(false), - include_validation_summary: Some(false), - }, - Some(&auth), - "finish-window", - ) - .await; - assert!(finish_summary.success, "{:?}", finish_summary.error); - let summary_serialized = serde_json::to_string(&finish_summary.output).unwrap(); - assert!( - !summary_serialized.contains("stdout_tail") && !summary_serialized.contains("stderr_tail"), - "summary_only finish leaked raw output fields" - ); - - // The finish continuation feedback must not re-run validation (no new - // validation events beyond the single cargo_test run recorded under A) and - // must not enqueue an agent/runner request. - let events_after = runtime - .sessions - .summary(&session_id, Some(200)) - .unwrap() - .events - .len(); - let new_validation = runtime - .sessions - .summary(&session_id, Some(200)) - .unwrap() - .events - .iter() - .filter(|e| e.kind == "tool_call_finished" && e.tool_name == "cargo_test") - .count(); + let summary = runtime.sessions.summary(&session_id, Some(200)).unwrap(); + assert!(summary.events.len() >= events_before_finish); assert_eq!( - new_validation, 1, - "finish_coding_task must not re-run validation; exactly one cargo_test run recorded" - ); - assert!( - events_after >= events_before, - "finish_coding_task regressed the ledger" + summary + .events + .iter() + .filter(|event| event.kind == "tool_call_finished" && event.tool_name == "cargo_test") + .count(), + 1, + "finish_coding_task must not re-run validation" ); assert!( next_patch_agent_request(&runtime, "finish-agent") @@ -2399,6 +2321,10 @@ fn record_validation_event( /// the start event and surfaces them on the finished event, the same extraction /// the production `closeout_work_projection` reads. fn record_write(runtime: &ToolRuntime, session_id: &str, paths: &[&str]) { + record_write_for(runtime, session_id, "test-project", paths); +} + +fn record_write_for(runtime: &ToolRuntime, session_id: &str, project: &str, paths: &[&str]) { let changes: Vec = paths .iter() .map(|path| json!({"kind": "edit", "path": path})) @@ -2408,7 +2334,7 @@ fn record_write(runtime: &ToolRuntime, session_id: &str, paths: &[&str]) { SessionTransport::Api, "apply_text_edits", &json!({ - "project": "test-project", + "project": project, "changes": changes, }), ); diff --git a/src/tool_runtime/tests/files.rs b/src/tool_runtime/tests/files.rs index df837009..22ce1278 100644 --- a/src/tool_runtime/tests/files.rs +++ b/src/tool_runtime/tests/files.rs @@ -6,7 +6,7 @@ use super::super::patch::*; use super::super::*; use super::support::*; use crate::shell_protocol::{ - ShellAgentPollRequest, ShellAgentResultRequest, ShellAgentShellRequest, ShellClientCapabilities, + ShellAgentResultRequest, ShellAgentShellRequest, ShellClientCapabilities, }; use serde_json::{json, Value}; #[cfg(unix)] @@ -47,9 +47,7 @@ async fn write_project_file_with_session_id_records_changed_path_without_content .await } }); - let req = next_patch_agent_request(&runtime, "telemetry-write") - .await - .expect("write_project_file should enqueue a native file-op request"); + let req = wait_for_patch_agent_request(&runtime, "telemetry-write").await; assert_eq!(req.kind, "file_write_project_file"); assert!(req.command.is_empty()); assert!(req.stdin.is_none()); @@ -142,9 +140,7 @@ async fn write_project_file_with_session_id_records_changed_path_without_content .await } }); - let req = next_patch_agent_request(&runtime, "telemetry-write") - .await - .expect("finish_coding_task should inspect changes"); + let req = wait_for_patch_agent_request(&runtime, "telemetry-write").await; assert_internal_posix_script_contains(&req, "git status --porcelain=v1 -b"); complete_patch_agent_request( &runtime, @@ -188,9 +184,7 @@ async fn delete_project_files_capable_agent_uses_structured_delete_without_outpu } }); - let req = next_patch_agent_request(&runtime, "cleanup-delete") - .await - .expect("delete_project_files should enqueue a structured file request"); + let req = wait_for_patch_agent_request(&runtime, "cleanup-delete").await; assert_eq!(req.kind, "file_delete_project_files"); assert!(req.command.is_empty()); assert_eq!(req.path.as_deref(), Some(".")); @@ -240,9 +234,7 @@ async fn delete_project_files_old_agent_keeps_legacy_shell_fallback() { } }); - let req = next_patch_agent_request(&runtime, "cleanup-delete-legacy") - .await - .expect("old agent should receive the legacy shell request"); + let req = wait_for_patch_agent_request(&runtime, "cleanup-delete-legacy").await; assert_eq!(req.kind, "run_shell"); assert!(req.command.contains("rm -f --")); complete_patch_agent_request( @@ -403,22 +395,28 @@ async fn delete_project_files_capability_revoked_before_enqueue_falls_back_to_le ); } -/// Spin (yield, never sleep) until the client has at least `expected` pending -/// requests, without polling/dispatching any of them. Used to synchronize -/// replacement/timeout tests deterministically on the queue being populated. +/// Wait until the client has at least `expected` pending requests without +/// polling/dispatching them. The single wall-clock deadline is deliberately +/// independent of scheduler yield counts. async fn wait_for_pending_requests(runtime: &ToolRuntime, client_id: &str, expected: usize) { - for _ in 0..200 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { let view = runtime .shell_clients .get_client_view(client_id) .await .unwrap_or_else(|| panic!("client {client_id} must be registered")); - if view.pending_requests >= expected { + let pending = view.pending_requests; + if pending >= expected { return; } - tokio::task::yield_now().await; + if tokio::time::Instant::now() >= deadline { + panic!( + "pending requests did not reach {expected} within 10 seconds for {client_id}; last pending count={pending}" + ); + } + tokio::time::sleep(std::time::Duration::from_millis(5)).await; } - panic!("pending requests did not reach {expected} for {client_id}"); } #[tokio::test] @@ -859,9 +857,7 @@ async fn artifact_upload_chunk_session_log_arguments_do_not_store_base64() { } }); - let req = next_patch_agent_request(&runtime, "telemetry-artifact-chunk") - .await - .expect("artifact_upload_chunk should enqueue a native file-op request"); + let req = wait_for_patch_agent_request(&runtime, "telemetry-artifact-chunk").await; let payload: serde_json::Value = serde_json::from_str(req.content.as_deref().expect("file-op payload")).unwrap(); assert_eq!(payload["content_base64"], content_base64); @@ -1046,9 +1042,7 @@ async fn read_project_artifact_metadata_allow_missing_does_not_count_as_failed() } }); - let req = next_patch_agent_request(&runtime, "artifact-missing-session") - .await - .expect("read_project_artifact_metadata should enqueue file-op"); + let req = wait_for_patch_agent_request(&runtime, "artifact-missing-session").await; complete_patch_agent_request( &runtime, "artifact-missing-session", @@ -1149,9 +1143,7 @@ async fn validate_patch_never_enqueues_mutating_apply_command() { }); // 1) `git apply --check -` (read-only applicability test). - let check_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("validate_patch should enqueue a check request"); + let check_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&check_req.command, marker); assert_eq!(check_req.command, "git apply --check -"); assert_ne!(check_req.command, "git apply -"); @@ -1159,9 +1151,7 @@ async fn validate_patch_never_enqueues_mutating_apply_command() { complete_patch_agent_request(&runtime, "patcher", &check_req.request_id, 0, "", "").await; // 2) `git apply --stat -` (read-only summary). - let stat_req = next_patch_agent_request(&runtime, "patcher") - .await - .expect("validate_patch should enqueue a stat request"); + let stat_req = wait_for_patch_agent_request(&runtime, "patcher").await; assert_safe_patch_command(&stat_req.command, marker); assert_eq!(stat_req.command, "git apply --stat -"); complete_patch_agent_request(&runtime, "patcher", &stat_req.request_id, 0, "stat", "").await; @@ -1740,9 +1730,7 @@ async fn execute_agent_search( .await } }); - let req = next_patch_agent_request(runtime, client_id) - .await - .expect("search_project_text agent request"); + let req = wait_for_patch_agent_request(runtime, client_id).await; let inspected = req.clone(); complete_agent_request_by_running_locally(runtime, client_id, req).await; let result = task.await.unwrap(); @@ -2705,9 +2693,7 @@ async fn search_agent_command_timeout_returns_search_timeout() { .await } }); - let req = next_patch_agent_request(&runtime, "search-cmd-timeout") - .await - .expect("search request"); + let req = wait_for_patch_agent_request(&runtime, "search-cmd-timeout").await; assert_eq!(req.timeout_secs, 1); // Simulate agent-side command timeout response (lowercase message + error field). runtime @@ -2764,9 +2750,7 @@ async fn search_agent_timeout_with_complete_records_returns_partial_success() { .await } }); - let req = next_patch_agent_request(&runtime, "search-ptimeout") - .await - .expect("search request"); + let req = wait_for_patch_agent_request(&runtime, "search-ptimeout").await; runtime .shell_clients .complete(ShellAgentResultRequest { @@ -2826,9 +2810,7 @@ async fn search_agent_outer_timeout_returns_search_timeout_and_cancels() { .await } }); - let req = next_patch_agent_request(&runtime, "search-outer-timeout") - .await - .expect("search request"); + let req = wait_for_patch_agent_request(&runtime, "search-outer-timeout").await; let request_id = req.request_id.clone(); assert_eq!(req.timeout_secs, 1); // Do not complete the agent request; outer tokio timeout should fire. @@ -2888,9 +2870,7 @@ async fn search_agent_request_dropped_returns_structured_error() { .await } }); - let req = next_patch_agent_request(&runtime, "search-dropped") - .await - .expect("search request"); + let req = wait_for_patch_agent_request(&runtime, "search-dropped").await; // Drop the oneshot waiter without completing — agent disconnect / channel drop. runtime.shell_clients.cancel_request(&req.request_id).await; let result = task.await.unwrap(); @@ -2945,9 +2925,7 @@ async fn search_timeout_only_without_rg_still_allows_grep_fallback() { .await } }); - let mut req = next_patch_agent_request(&runtime, "search-timeout-fallback") - .await - .expect("search request"); + let mut req = wait_for_patch_agent_request(&runtime, "search-timeout-fallback").await; // Force grep path (no rg in PATH). req.command = format!( "PATH={}; export PATH\n{}", @@ -3371,9 +3349,7 @@ async fn advanced_search_without_rg_returns_structured_capability_error() { .await } }); - let mut req = next_patch_agent_request(&runtime, "search-no-rg") - .await - .expect("advanced search agent request"); + let mut req = wait_for_patch_agent_request(&runtime, "search-no-rg").await; req.command = format!( "PATH={}; export PATH\n{}", shell_escape_simple(&bin.to_string_lossy()), @@ -3425,9 +3401,7 @@ async fn search_project_text_no_matches_returns_empty_matches() { .await } }); - let req = next_patch_agent_request(&runtime, "search-empty") - .await - .expect("search_project_text should enqueue an agent search request"); + let req = wait_for_patch_agent_request(&runtime, "search-empty").await; assert_eq!(req.timeout_secs, 30); complete_agent_request_by_running_locally(&runtime, "search-empty", req).await; let result = task.await.unwrap(); @@ -3483,9 +3457,7 @@ async fn search_project_text_excludes_sensitive_and_build_dirs() { .await } }); - let req = next_patch_agent_request(&runtime, "search-excludes") - .await - .expect("search_project_text should enqueue an agent search request"); + let req = wait_for_patch_agent_request(&runtime, "search-excludes").await; complete_agent_request_by_running_locally(&runtime, "search-excludes", req).await; let result = task.await.unwrap(); @@ -3495,54 +3467,57 @@ async fn search_project_text_excludes_sensitive_and_build_dirs() { } #[tokio::test] -async fn list_project_files_requires_file_read_capability() { - let runtime = runtime_with_agent_project("oe"); - // Default capabilities have file_read = false. - register_agent(&runtime, "oe", None, ShellClientCapabilities::default()).await; - let bootstrap = auth_context(None, true); - let result = runtime - .dispatch_with_auth( - ToolCall::ListProjectFiles { - project: agent_test_project_id("oe"), - session_id: None, - path: None, - limit: None, - }, - Some(&bootstrap), - ) - .await; - assert!(!result.success); - assert!( - result.error.unwrap().contains("file_read"), - "list_project_files should require file_read capability" - ); -} - -#[tokio::test] -async fn project_overview_requires_file_read_capability() { - let runtime = runtime_with_agent_project("overview-capability"); +async fn file_read_project_tools_require_file_read_capability() { + let runtime = runtime_with_agent_project("file-read-capability"); register_agent( &runtime, - "overview-capability", + "file-read-capability", None, ShellClientCapabilities::default(), ) .await; let bootstrap = auth_context(None, true); - let result = runtime - .dispatch_with_auth( + let project = agent_test_project_id("file-read-capability"); + let calls = [ + ( + "list_project_files", + ToolCall::ListProjectFiles { + project: project.clone(), + session_id: None, + path: None, + limit: None, + }, + ), + ( + "project_overview", ToolCall::ProjectOverview { - project: agent_test_project_id("overview-capability"), + project, session_id: None, path: None, max_depth: None, limit: None, }, - Some(&bootstrap), - ) - .await; - assert!(!result.success); - assert!(result.error.unwrap().contains("file_read")); + ), + ]; + + for (tool, call) in calls { + let result = runtime.dispatch_with_auth(call, Some(&bootstrap)).await; + assert!( + !result.success, + "{tool} must reject a Runner without file_read" + ); + assert!( + result + .error + .as_deref() + .is_some_and(|error| error.contains("file_read")), + "{tool}: {:?}", + result.error + ); + } + assert!(next_patch_agent_request(&runtime, "file-read-capability") + .await + .is_none()); } #[tokio::test] @@ -3582,9 +3557,7 @@ async fn project_overview_routes_to_owning_agent_and_returns_structured_metadata .await } }); - let request = next_patch_agent_request(&runtime, "overview-agent") - .await - .expect("project_overview owning-agent request"); + let request = wait_for_patch_agent_request(&runtime, "overview-agent").await; assert_eq!(request.kind, "file_project_overview"); assert!( request.command.is_empty(), @@ -3625,38 +3598,169 @@ async fn project_overview_routes_to_owning_agent_and_returns_structured_metadata } #[tokio::test] -async fn project_overview_rejects_invalid_paths_before_agent_request() { - let runtime = runtime_with_agent_project("overview-path"); +async fn project_read_adapters_reject_out_of_project_paths_before_agent_dispatch() { + let runtime = runtime_with_agent_project("path-boundary"); register_agent( &runtime, - "overview-path", + "path-boundary", None, ShellClientCapabilities { file_read: true, + shell: true, ..Default::default() }, ) .await; let bootstrap = auth_context(None, true); - for path in ["/etc", "../outside"] { - let result = runtime - .dispatch_with_auth( - ToolCall::ProjectOverview { - project: agent_test_project_id("overview-path"), - session_id: None, - path: Some(path.to_string()), - max_depth: None, - limit: None, - }, - Some(&bootstrap), - ) - .await; - assert!(!result.success, "{path} must be rejected"); - assert!(result.error.unwrap().contains("path")); + let project = agent_test_project_id("path-boundary"); + let calls = vec![ + ( + "read_file parent traversal", + ToolCall::ReadFile { + project: project.clone(), + path: "../outside.txt".to_string(), + session_id: None, + start_line: None, + limit: None, + with_line_numbers: None, + }, + None, + ), + ( + "read_file nested parent traversal", + ToolCall::ReadFile { + project: project.clone(), + path: "src/../../outside.txt".to_string(), + session_id: None, + start_line: None, + limit: None, + with_line_numbers: None, + }, + None, + ), + ( + "read_file absolute path", + ToolCall::ReadFile { + project: project.clone(), + path: "/etc/passwd".to_string(), + session_id: None, + start_line: None, + limit: None, + with_line_numbers: None, + }, + None, + ), + ( + "read_file deep parent traversal", + ToolCall::ReadFile { + project: project.clone(), + path: "sub/../../../etc/passwd".to_string(), + session_id: None, + start_line: None, + limit: None, + with_line_numbers: None, + }, + None, + ), + ( + "list_project_files absolute path", + ToolCall::ListProjectFiles { + project: project.clone(), + session_id: None, + path: Some("/etc".to_string()), + limit: None, + }, + None, + ), + ( + "list_project_files parent traversal", + ToolCall::ListProjectFiles { + project: project.clone(), + session_id: None, + path: Some("../outside".to_string()), + limit: None, + }, + None, + ), + ( + "project_overview absolute path", + ToolCall::ProjectOverview { + project: project.clone(), + session_id: None, + path: Some("/etc".to_string()), + max_depth: None, + limit: None, + }, + None, + ), + ( + "project_overview parent traversal", + ToolCall::ProjectOverview { + project: project.clone(), + session_id: None, + path: Some("../outside".to_string()), + max_depth: None, + limit: None, + }, + None, + ), + ( + "search_project_text absolute path", + ToolCall::SearchProjectText { + project: project.clone(), + pattern: "needle".to_string(), + session_id: None, + path: Some("/etc".to_string()), + limit: None, + context_before: None, + context_after: None, + include_globs: None, + exclude_globs: None, + result_mode: None, + timeout_secs: None, + }, + Some("path"), + ), + ( + "search_project_text parent traversal", + ToolCall::SearchProjectText { + project, + pattern: "needle".to_string(), + session_id: None, + path: Some("../outside".to_string()), + limit: None, + context_before: None, + context_after: None, + include_globs: None, + exclude_globs: None, + result_mode: None, + timeout_secs: None, + }, + Some("path"), + ), + ]; + + for (case, call, structured_field) in calls { + let result = runtime.dispatch_with_auth(call, Some(&bootstrap)).await; + assert!(!result.success, "{case} escaped the project boundary"); + let error = result.error.as_deref().unwrap_or(""); + assert!( + error.contains("project-relative") + || error.contains("parent traversal") + || error.contains("path"), + "{case}: {error}" + ); + if let Some(field) = structured_field { + assert_eq!(result.output["code"], "invalid_search_request", "{case}"); + assert_eq!(result.output["field"], field, "{case}"); + } + assert!( + next_patch_agent_request(&runtime, "path-boundary") + .await + .is_none(), + "{case} must reject before Agent dispatch" + ); } - assert!(next_patch_agent_request(&runtime, "overview-path") - .await - .is_none()); } #[tokio::test] @@ -3729,9 +3833,7 @@ async fn search_project_text_context_does_not_enqueue_python_helper() { .await } }); - let req = next_patch_agent_request(&runtime, "search-native") - .await - .expect("search_project_text should enqueue an agent search request"); + let req = wait_for_patch_agent_request(&runtime, "search-native").await; let forbidden = ["python3", "-c"].join(" "); assert!( !req.command.contains(&forbidden), @@ -3783,43 +3885,6 @@ async fn list_project_files_rejects_non_agent_project_id() { assert!(!err.contains("projects.toml"), "{err}"); } -#[tokio::test] -async fn list_project_files_rejects_absolute_or_parent_paths_before_agent_request() { - let runtime = runtime_with_agent_project("oe"); - register_agent( - &runtime, - "oe", - None, - ShellClientCapabilities { - file_read: true, - ..Default::default() - }, - ) - .await; - let bootstrap = auth_context(None, true); - for path in ["/etc", "../outside"] { - let result = runtime - .dispatch_with_auth( - ToolCall::ListProjectFiles { - project: agent_test_project_id("oe"), - session_id: None, - path: Some(path.to_string()), - limit: None, - }, - Some(&bootstrap), - ) - .await; - assert!(!result.success, "path {} should be rejected", path); - let err = result.error.unwrap(); - assert!( - err.contains("project-relative") || err.contains("parent traversal"), - "unexpected error for {}: {}", - path, - err - ); - } -} - #[tokio::test] async fn search_project_text_rejects_empty_pattern() { // Authorization runs before the tool body, so register an agent with @@ -3860,52 +3925,6 @@ async fn search_project_text_rejects_empty_pattern() { assert_eq!(result.output["field"], "pattern"); } -#[tokio::test] -async fn search_project_text_rejects_absolute_or_parent_paths_before_agent_request() { - let runtime = runtime_with_agent_project("oe"); - register_agent( - &runtime, - "oe", - None, - ShellClientCapabilities { - shell: true, - ..Default::default() - }, - ) - .await; - let bootstrap = auth_context(None, true); - for path in ["/etc", "../outside"] { - let result = runtime - .dispatch_with_auth( - ToolCall::SearchProjectText { - project: agent_test_project_id("oe"), - pattern: "needle".to_string(), - session_id: None, - path: Some(path.to_string()), - limit: None, - context_before: None, - context_after: None, - include_globs: None, - exclude_globs: None, - result_mode: None, - timeout_secs: None, - }, - Some(&bootstrap), - ) - .await; - assert!(!result.success, "path {} should be rejected", path); - let err = result.error.unwrap(); - assert!( - err.contains("project-relative") || err.contains("parent traversal"), - "unexpected error for {}: {}", - path, - err - ); - assert_eq!(result.output["code"], "invalid_search_request"); - assert_eq!(result.output["field"], "path"); - } -} - #[test] fn validate_edit_file_path_rejects_unsafe_and_sensitive_paths() { // Safe relative paths accepted. @@ -4360,94 +4379,36 @@ async fn artifact_upload_finish_and_abort_reject_invalid_upload_id_before_resolv } #[tokio::test] -async fn read_file_rejects_parent_traversal_before_reaching_agent() { - // The agent host scopes file ops to `allowed_roots`, which is broader than - // the project, so a traversal that stays inside `allowed_roots` would read - // a file the caller was never granted. The project boundary therefore has - // to be enforced server-side, before the request is queued for the agent. - let runtime = runtime_with_agent_project("traversal-read"); - let caps = ShellClientCapabilities { - file_read: true, - ..Default::default() - }; - register_agent(&runtime, "traversal-read", None, caps).await; - let project = agent_test_project_id("traversal-read"); - - for path in [ - "../outside.txt", - "src/../../outside.txt", - "/etc/passwd", - "sub/../../../etc/passwd", +async fn read_file_routes_safe_and_bulk_skipped_explicit_paths_to_agent() { + for (client_id, path, content) in [ + ("relative-read", "src/main.rs", "fn main() {}\n"), + ("bulk-explicit-read", ".git/HEAD", "ref: refs/heads/main\n"), ] { - let result = runtime - .read_file(project.clone(), path.to_string(), None, None, None) - .await; - assert!( - !result.success, - "read_file accepted out-of-project path {path:?}" - ); - } - - // Nothing may have been queued for the agent for any rejected path. - let queued = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "traversal-read".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - assert!( - queued.is_none(), - "rejected traversal still reached the agent queue: {queued:?}" - ); -} - -#[tokio::test] -async fn read_file_still_routes_project_relative_paths_to_agent() { - let runtime = runtime_with_agent_project("traversal-ok"); - let caps = ShellClientCapabilities { - file_read: true, - ..Default::default() - }; - register_agent(&runtime, "traversal-ok", None, caps).await; - let project = agent_test_project_id("traversal-ok"); - - let runtime_for_task = runtime.clone(); - let project_for_task = project.clone(); - let task = tokio::spawn(async move { - runtime_for_task - .read_file( - project_for_task, - "src/main.rs".to_string(), - None, - None, - None, - ) - .await - }); + let runtime = runtime_with_agent_project(client_id); + register_agent( + &runtime, + client_id, + None, + ShellClientCapabilities { + file_read: true, + ..Default::default() + }, + ) + .await; + let project = agent_test_project_id(client_id); + let task = tokio::spawn({ + let runtime = runtime.clone(); + let path = path.to_string(); + async move { runtime.read_file(project, path, None, None, None).await } + }); - let mut req = None; - for _ in 0..20 { - req = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "traversal-ok".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - if req.is_some() { - break; - } - tokio::task::yield_now().await; + let request = wait_for_patch_agent_request(&runtime, client_id).await; + assert_eq!(request.kind, "file_read", "{path}"); + assert_eq!(request.path.as_deref(), Some(path), "{path}"); + complete_agent_ranged_file_read_request(&runtime, client_id, &request, content).await; + let result = task.await.unwrap(); + assert!(result.success, "{path}: {:?}", result.error); } - let req = req.expect("a project-relative read must still reach the agent"); - assert_eq!(req.kind, "file_read"); - assert_eq!(req.path.as_deref(), Some("src/main.rs")); - task.abort(); } #[tokio::test] @@ -4490,59 +4451,12 @@ async fn read_file_refuses_secret_paths_before_reaching_agent() { ); } - let queued = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "secret-read".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); assert!( - queued.is_none(), - "a refused secret path still reached the agent: {queued:?}" - ); -} - -#[tokio::test] -async fn read_file_still_allows_bulk_tree_paths_by_explicit_path() { - // `.git` and `target` are skipped by bulk operations for cost, not - // secrecy. Reading one by explicit path must keep working. - let runtime = runtime_with_agent_project("bulk-read"); - let caps = ShellClientCapabilities { - file_read: true, - ..Default::default() - }; - register_agent(&runtime, "bulk-read", None, caps).await; - let project = agent_test_project_id("bulk-read"); - - let runtime_for_task = runtime.clone(); - let task = tokio::spawn(async move { - runtime_for_task - .read_file(project, ".git/HEAD".to_string(), None, None, None) + next_patch_agent_request(&runtime, "secret-read") .await - }); - - let mut req = None; - for _ in 0..20 { - req = runtime - .shell_clients - .poll(ShellAgentPollRequest { - client_id: "bulk-read".to_string(), - agent_instance_id: "inst".to_string(), - projects: None, - }) - .await - .unwrap(); - if req.is_some() { - break; - } - tokio::task::yield_now().await; - } - let req = req.expect(".git/HEAD must still reach the agent"); - assert_eq!(req.path.as_deref(), Some(".git/HEAD")); - task.abort(); + .is_none(), + "a refused secret path still reached the agent" + ); } /// `--no-ignore` used to be passed to ripgrep, so a search walked straight diff --git a/src/tool_runtime/tests/git.rs b/src/tool_runtime/tests/git.rs index be7801d8..3eba70f3 100644 --- a/src/tool_runtime/tests/git.rs +++ b/src/tool_runtime/tests/git.rs @@ -66,9 +66,7 @@ async fn git_restore_paths_restores_tracked_filename_containing_target() { } }); - let request = next_patch_agent_request(&runtime, "restore-target-substring") - .await - .expect("git_restore_paths should enqueue an agent process request"); + let request = wait_for_patch_agent_request(&runtime, "restore-target-substring").await; assert_eq!(request.kind, "run_process"); assert!(request.command.is_empty()); let process = request.process.as_ref().expect("typed git restore process"); @@ -118,9 +116,7 @@ async fn git_path_mutations_pass_shell_sensitive_paths_as_literal_argv() { let restore_paths = restore_paths.clone(); async move { runtime.git_restore_paths(project, restore_paths).await } }); - let request = next_patch_agent_request(&runtime, "literal-git-paths") - .await - .expect("git restore should enqueue typed argv"); + let request = wait_for_patch_agent_request(&runtime, "literal-git-paths").await; assert_eq!(request.kind, "run_process"); let process = request.process.as_ref().expect("typed git restore process"); assert_eq!(process.executable, "git"); @@ -154,9 +150,7 @@ async fn git_path_mutations_pass_shell_sensitive_paths_as_literal_argv() { let discard_paths = discard_paths.clone(); async move { runtime.discard_untracked(project, discard_paths).await } }); - let request = next_patch_agent_request(&runtime, "literal-git-paths") - .await - .expect("git clean should enqueue typed argv"); + let request = wait_for_patch_agent_request(&runtime, "literal-git-paths").await; assert_eq!(request.kind, "run_process"); let process = request.process.as_ref().expect("typed git clean process"); assert_eq!(process.executable, "git"); @@ -205,9 +199,7 @@ async fn git_path_mutation_capability_preflight_matches_structured_process_runti } }); - let request = next_patch_agent_request(&runtime, "restore-structured-only") - .await - .expect("structured-process-only Runner should reach typed process dispatch"); + let request = wait_for_patch_agent_request(&runtime, "restore-structured-only").await; assert_eq!(request.kind, "run_process"); assert!(request.command.is_empty()); let process = request.process.as_ref().expect("typed git restore process"); @@ -325,9 +317,7 @@ async fn git_restore_stays_sync_on_structured_job_capable_runner() { } }); - let request = next_patch_agent_request(&runtime, "restore-sync-job-capable") - .await - .expect("git restore should stay on the synchronous process path"); + let request = wait_for_patch_agent_request(&runtime, "restore-sync-job-capable").await; assert_eq!(request.kind, "run_process"); assert!(request.job_id.is_none()); assert!(request.command.is_empty()); @@ -381,9 +371,7 @@ async fn git_restore_replacement_after_dispatch_reports_outcome_unknown_without_ .await } }); - let request = next_patch_agent_request(&runtime, "restore-uncertain") - .await - .expect("git restore should dispatch once"); + let request = wait_for_patch_agent_request(&runtime, "restore-uncertain").await; assert_eq!(request.kind, "run_process"); runtime @@ -715,9 +703,7 @@ async fn run_agent_git_diff_hunks_page( .await } }); - let request = next_patch_agent_request(runtime, client_id) - .await - .expect("git_diff_hunks should enqueue one bounded agent request"); + let request = wait_for_patch_agent_request(runtime, client_id).await; assert_eq!(request.kind, "run_internal_posix_script"); assert_eq!( request.cwd.as_deref(), @@ -776,7 +762,12 @@ async fn run_agent_git_diff_hunks_committed_page( }); let mut scripts = Vec::new(); let mut page_stdout_bytes = 0usize; + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); for _ in 0..16 { + assert!( + tokio::time::Instant::now() < deadline, + "committed git_diff_hunks did not finish within 10 seconds for client {client_id}" + ); if task.is_finished() { break; } @@ -838,7 +829,7 @@ async fn run_agent_git_diff_hunks_committed_page( } assert!( task.is_finished(), - "committed git_diff_hunks did not finish after bounded agent requests" + "committed git_diff_hunks exceeded its 16-request protocol bound for client {client_id}" ); (task.await.unwrap(), page_stdout_bytes, scripts) } @@ -1716,9 +1707,7 @@ async fn git_diff_hunks_committed_drains_bounded_consumer_and_preserves_producer } }); - let scope_request = next_patch_agent_request(&runtime, "committed-producer-failure") - .await - .expect("scope request"); + let scope_request = wait_for_patch_agent_request(&runtime, "committed-producer-failure").await; complete_agent_request_by_running_locally( &runtime, "committed-producer-failure", @@ -1726,9 +1715,8 @@ async fn git_diff_hunks_committed_drains_bounded_consumer_and_preserves_producer ) .await; - let mut page_request = next_patch_agent_request(&runtime, "committed-producer-failure") - .await - .expect("page request"); + let mut page_request = + wait_for_patch_agent_request(&runtime, "committed-producer-failure").await; let page_script = page_request .script .as_ref() @@ -3073,9 +3061,7 @@ async fn show_changes_include_diff_agent_command_does_not_enqueue_python_helper( .await } }); - let req = next_patch_agent_request(&runtime, "show-native") - .await - .expect("show_changes should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(&runtime, "show-native").await; assert_eq!(req.kind, "run_internal_posix_script"); assert!(req.command.is_empty()); let payload = req @@ -3307,9 +3293,7 @@ async fn show_changes_session_event_limit_is_bounded() { .show_changes(project, Some(session_id), None, None, None, Some(999)) .await }); - let req = next_patch_agent_request(&runtime, "show") - .await - .expect("show_changes should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(&runtime, "show").await; let stdout = "## main\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nstatus_exit=0\nrepository_probe=inside_worktree\nrepository_probe_exit=0\nfiles_total=0\nfiles_returned=0\nfiles_truncated=0\nfiles_limit=200\nmodified=0\nadded=0\ndeleted=0\nrenamed=0\ncopied=0\nuntracked=0\nconflicted=0\nstaged=0\nunstaged=0\nstatus_trunc_count=0\nstatus_trunc_bytes=0\nstatus_trunc_path=0\nstatus_bytes=7\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ncommit=abc123\nshort=abc123\nsummary=head\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nhead_exit=0\nhead_truncated=0\nhead_bytes=39\n@@WEBCODEX_SHOW_CHANGES_SEP@@\n\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ndiff_stat_exit=0\ndiff_stat_truncated=0\ndiff_stat_bytes=0\n"; complete_patch_agent_request(&runtime, "show", &req.request_id, 0, stdout, "").await; let result = task.await.unwrap(); @@ -3558,13 +3542,12 @@ async fn show_changes_untracked_sensitive_path_preview_is_skipped() { } #[test] -fn git_diff_hunks_command_rejects_unsafe_paths() { +fn git_diff_hunks_command_is_read_only_and_scoped_to_paths() { let command = git_diff_hunks_command(&["src/lib.rs".to_string()], false).unwrap(); assert!(command.contains("git diff")); assert!(command.contains("--no-ext-diff")); assert!(command.contains("--no-textconv")); assert!(command.contains("--unified=80 -- 'src/lib.rs'")); - assert!(validate_project_relative_path("../outside").is_err()); } #[tokio::test] @@ -3653,9 +3636,7 @@ async fn show_changes_with_session_id_returns_session_block_and_records_call() { .await } }); - let req = next_patch_agent_request(&runtime, "telemetry-show") - .await - .expect("show_changes should enqueue shell request"); + let req = wait_for_patch_agent_request(&runtime, "telemetry-show").await; let stdout = "## main\n M README.md\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nstatus_exit=0\nrepository_probe=inside_worktree\nrepository_probe_exit=0\nfiles_total=1\nfiles_returned=1\nfiles_truncated=0\nfiles_limit=200\nmodified=1\nadded=0\ndeleted=0\nrenamed=0\ncopied=0\nuntracked=0\nconflicted=0\nstaged=0\nunstaged=1\nstatus_trunc_count=0\nstatus_trunc_bytes=0\nstatus_trunc_path=0\nstatus_bytes=20\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ncommit=abc123\nshort=abc123\nsummary=head\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nhead_exit=0\nhead_truncated=0\nhead_bytes=39\n@@WEBCODEX_SHOW_CHANGES_SEP@@\nREADME.md | 1 +\n@@WEBCODEX_SHOW_CHANGES_SEP@@\ndiff_stat_exit=0\ndiff_stat_truncated=0\ndiff_stat_bytes=15\n"; complete_patch_agent_request(&runtime, "telemetry-show", &req.request_id, 0, stdout, "").await; let result = show_task.await.unwrap(); @@ -3774,24 +3755,29 @@ fn split_diff_summary_without_sentinel_returns_all_as_porcelain() { } #[test] -fn git_log_command_is_read_only_and_bounded() { +fn git_read_commands_are_non_mutating_and_log_is_bounded() { assert_eq!(normalize_git_log_limit(None), 20); assert_eq!(normalize_git_log_limit(Some(0)), 20); assert_eq!(normalize_git_log_limit(Some(999)), 100); assert_eq!(normalize_git_log_skip(Some(20_000)), 10_000); - let cmd = git_log_command(21, 7); - assert!(cmd.contains("git log")); - assert!(cmd.contains("-n 22")); - assert!(cmd.contains("--skip 7")); - for forbidden in [ - "apply", "commit", "checkout", "reset", "push", "stash", "merge", "rebase", "rm ", - ] { - assert!( - !cmd.contains(forbidden), - "git_log command must not contain '{}': {}", - forbidden, - cmd - ); + + let log = git_log_command(21, 7); + assert!(log.contains("git log")); + assert!(log.contains("-n 22")); + assert!(log.contains("--skip 7")); + let summary = git_diff_summary_command(); + assert!(summary.contains("git status --porcelain")); + assert!(summary.contains("git diff --stat")); + + for (tool, command) in [("git_log", log), ("git_diff_summary", summary)] { + for forbidden in [ + "apply", "commit", "checkout", "reset", "push", "stash", "merge", "rebase", "rm ", + ] { + assert!( + !command.contains(forbidden), + "{tool} command must not contain {forbidden:?}: {command}" + ); + } } } @@ -3806,25 +3792,6 @@ fn git_log_parser_splits_commits_refs_and_truncation() { assert_eq!(commits[0]["refs"], json!(["HEAD", "main", "v1"])); } -#[test] -fn git_diff_summary_command_is_read_only() { - let cmd = git_diff_summary_command(); - // Must run only read-only git inspection subcommands. - assert!(cmd.contains("git status --porcelain")); - assert!(cmd.contains("git diff --stat")); - // No mutating subcommands may appear. - for forbidden in [ - "apply", "commit", "checkout", "reset", "push", "stash", "merge", "rebase", "rm ", - ] { - assert!( - !cmd.contains(forbidden), - "git_diff_summary command must not contain '{}': {}", - forbidden, - cmd - ); - } -} - #[tokio::test] async fn git_diff_summary_agent_uses_internal_posix_runtime() { let tmp = tempfile::tempdir().unwrap(); @@ -3841,21 +3808,12 @@ async fn git_diff_summary_agent_uses_internal_posix_runtime() { async move { runtime.git_diff_summary(project).await } }); - let request = next_patch_agent_request(&runtime, "summary-internal") - .await - .expect("git_diff_summary should enqueue one internal request"); - assert_eq!(request.kind, "run_internal_posix_script"); - assert!(request.command.is_empty()); - let payload = request - .script - .as_ref() - .expect("git_diff_summary must carry a typed internal script"); + let request = wait_for_patch_agent_request(&runtime, "summary-internal").await; + assert_internal_posix_script_contains(&request, "git status --porcelain"); assert_eq!( - payload.language, - crate::shell_protocol::ShellScriptLanguage::Sh + request.script.as_ref().unwrap().script, + git_diff_summary_command() ); - assert_eq!(payload.script, git_diff_summary_command()); - assert!(payload.args.is_empty()); complete_agent_request_by_running_locally(&runtime, "summary-internal", request).await; let result = task.await.unwrap(); @@ -3907,10 +3865,12 @@ async fn run_git_review_summary_via_agent( .git_review_summary(project, base_commit, head_commit) .await }); - for _ in 0..64 { - if task.is_finished() { - break; - } + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + while !task.is_finished() { + assert!( + tokio::time::Instant::now() < deadline, + "git_review_summary did not finish within 10 seconds for client {client_id}" + ); if let Some(request) = next_patch_agent_request(runtime, client_id).await { assert_eq!(request.kind, "run_internal_posix_script"); assert!(request.command.is_empty()); @@ -3967,13 +3927,9 @@ async fn run_git_review_summary_via_agent( ) .await; } else { - tokio::task::yield_now().await; + tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } - assert!( - task.is_finished(), - "git_review_summary did not finish after bounded agent requests" - ); task.await.unwrap() } @@ -4862,10 +4818,12 @@ async fn run_show_changes_via_agent( .show_changes(project, session_id, Some(include_diff), None, None, None) .await }); - for _ in 0..20 { - if task.is_finished() { - break; - } + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + while !task.is_finished() { + assert!( + tokio::time::Instant::now() < deadline, + "show_changes did not finish within 10 seconds for client {client_id}" + ); if let Some(req) = next_patch_agent_request(runtime, client_id).await { assert_eq!(req.kind, "run_internal_posix_script"); assert!(req.command.is_empty()); @@ -4880,13 +4838,9 @@ async fn run_show_changes_via_agent( assert!(payload.args.is_empty()); complete_agent_request_by_running_locally(runtime, client_id, req).await; } else { - tokio::task::yield_now().await; + tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } - assert!( - task.is_finished(), - "show_changes did not finish after agent requests" - ); task.await.unwrap() } @@ -6055,9 +6009,7 @@ async fn show_changes_runtime_rejects_stat_only_failure_for_both_diff_modes() { .await } }); - let req = next_patch_agent_request(&runtime, "stat-only") - .await - .expect("show_changes should enqueue a shell request"); + let req = wait_for_patch_agent_request(&runtime, "stat-only").await; assert_eq!(req.kind, "run_internal_posix_script"); assert!(req.command.is_empty()); let payload = req @@ -6170,9 +6122,7 @@ async fn show_changes_runtime_rejects_unavailable_diff_stat_observation() { .await } }); - let req = next_patch_agent_request(&runtime, "stat-missing") - .await - .expect("show_changes should enqueue a shell request"); + let req = wait_for_patch_agent_request(&runtime, "stat-missing").await; assert_eq!(req.kind, "run_internal_posix_script"); assert!(req.command.is_empty()); let payload = req @@ -6435,9 +6385,7 @@ async fn show_changes_runtime_propagates_full_diff_failure_as_tool_failure() { .await } }); - let req = next_patch_agent_request(&runtime, "extd") - .await - .expect("show_changes should enqueue a shell request"); + let req = wait_for_patch_agent_request(&runtime, "extd").await; // Run the generated internal script locally with the failing external diff // in the environment so the full `git diff` fails. assert_eq!(req.kind, "run_internal_posix_script"); diff --git a/src/tool_runtime/tests/handoff.rs b/src/tool_runtime/tests/handoff.rs index 9c00911b..195b0db8 100644 --- a/src/tool_runtime/tests/handoff.rs +++ b/src/tool_runtime/tests/handoff.rs @@ -222,9 +222,7 @@ async fn session_handoff_summary_includes_recent_failed_tools() { .await } }); - let req = next_agent_request_for_instance(&runtime, "handoff-fail", "inst") - .await - .expect("read_file should enqueue an agent request"); + let req = wait_for_agent_request_for_instance(&runtime, "handoff-fail", "inst").await; // Return an error to simulate a failed read. complete_patch_agent_request( &runtime, @@ -871,20 +869,20 @@ async fn real_cargo_nonzero_failures_match_validation_failed_expectations() { call_typed_tool_with_metadata(&runtime, &tool_name, arguments, Some(&auth)).await } }); - let mut req = None; - for _ in 0..200 { - req = next_patch_agent_request(&runtime, "cargo-expected-kind").await; - if req.is_some() || task.is_finished() { - break; + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + let req = loop { + if let Some(req) = next_patch_agent_request(&runtime, "cargo-expected-kind").await { + break req; } - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - let req = match req { - Some(req) => req, - None => { + if task.is_finished() { let result = task.await.unwrap(); panic!("cargo tool finished before enqueueing an agent shell request: {result:?}"); } + assert!( + std::time::Instant::now() < deadline, + "cargo validation Agent request readiness timed out" + ); + tokio::time::sleep(std::time::Duration::from_millis(5)).await; }; assert_eq!(req.command, *expected_command); complete_patch_agent_request( @@ -985,15 +983,21 @@ async fn cargo_test_zero_tests_success_is_detected_and_warns_in_handoff() { } }); - let mut req = None; - for _ in 0..200 { - req = next_patch_agent_request(&runtime, "cargo-zero-tests").await; - if req.is_some() || task.is_finished() { - break; + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + let req = loop { + if let Some(req) = next_patch_agent_request(&runtime, "cargo-zero-tests").await { + break req; } + assert!( + !task.is_finished(), + "cargo_test finished before Agent dispatch" + ); + assert!( + std::time::Instant::now() < deadline, + "cargo_test Agent request readiness timed out" + ); tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } - let req = req.expect("cargo_test should enqueue an agent shell request"); + }; assert_eq!(req.command, "cargo test 'missing_filter'"); complete_patch_agent_request( &runtime, @@ -1563,58 +1567,41 @@ async fn session_handoff_summary_only_is_compact() { // 4. Active jobs summary // ========================================================================= +async fn handoff_jobs_projection(runtime: &ToolRuntime, session_id: &str) -> ToolResult { + runtime + .dispatch(ToolCall::SessionHandoffSummary { + session_id: session_id.to_string(), + project: None, + include_workspace: Some(false), + include_checkpoints: Some(false), + include_validation: Some(false), + summary_only: false, + limit: Some(20), + }) + .await +} + #[tokio::test] async fn session_handoff_summary_includes_active_jobs_and_clears_after_stop() { - let runtime = test_runtime(); - let caps = ShellClientCapabilities { - async_shell_jobs: true, - ..Default::default() - }; - let auth = open_auth_context(); - register_agent_projects_for_auth( + let temp = tempfile::tempdir().unwrap(); + let runtime = runtime_with_project(temp.path(), "demo"); + let project = "demo"; + let session = runtime + .sessions + .start_session(Some(project.to_string()), Some("handoff jobs".to_string())); + let job_id = "11111111-2222-3333-4444-555555555551"; + seed_session_projection_job( &runtime, - "handoff-jobs", - &auth, - caps, - vec![registered_project("demo", "/tmp/handoff-jobs-demo")], + temp.path(), + job_id, + project, + &session.session_id, + "running", + "handoff-secret-output\n", ) .await; - let project = "agent:handoff-jobs:demo".to_string(); - let session = runtime - .sessions - .start_session(Some(project.clone()), Some("handoff jobs".to_string())); - let sid = session.session_id.clone(); - let run = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: project.clone(), - command: "printf handoff-secret-output".to_string(), - session_id: Some(sid.clone()), - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - Some(&auth), - ) - .await; - assert!(run.success, "{:?}", run.error); - let job_id = run.output["job_id"].as_str().unwrap().to_string(); - - let active = runtime - .dispatch_with_auth( - ToolCall::SessionHandoffSummary { - session_id: sid.clone(), - project: Some(project.clone()), - include_workspace: Some(false), - include_checkpoints: Some(false), - include_validation: Some(false), - summary_only: false, - limit: Some(20), - }, - Some(&auth), - ) - .await; + + let active = handoff_jobs_projection(&runtime, &session.session_id).await; assert!(active.success, "{:?}", active.error); assert_eq!(active.output["jobs"]["active_count"], 1); assert_eq!(active.output["jobs"]["running_count"], 1); @@ -1634,33 +1621,12 @@ async fn session_handoff_summary_includes_active_jobs_and_clears_after_stop() { let serialized = serde_json::to_string(&active.output["jobs"]).unwrap(); assert!(!serialized.contains("handoff-secret-output")); - let stop = runtime - .dispatch_with_auth( - ToolCall::StopJob { - project: project.clone(), - job_id, - session_id: Some(sid.clone()), - confirm: true, - }, - Some(&auth), - ) - .await; - assert!(stop.success, "{:?}", stop.error); - - let stopped = runtime - .dispatch_with_auth( - ToolCall::SessionHandoffSummary { - session_id: sid, - project: Some(project), - include_workspace: Some(false), - include_checkpoints: Some(false), - include_validation: Some(false), - summary_only: false, - limit: Some(20), - }, - Some(&auth), - ) - .await; + std::fs::write( + temp.path().join(format!(".codex/jobs/{job_id}/status")), + "stopped", + ) + .unwrap(); + let stopped = handoff_jobs_projection(&runtime, &session.session_id).await; assert!(stopped.success, "{:?}", stopped.error); assert_eq!(stopped.output["jobs"]["active_count"], 0); assert_eq!(stopped.output["jobs"]["blocking_active_count"], 0); @@ -1674,75 +1640,26 @@ async fn session_handoff_summary_includes_active_jobs_and_clears_after_stop() { #[tokio::test] async fn session_handoff_summary_treats_stop_requested_as_nonblocking() { - let runtime = test_runtime(); - let caps = ShellClientCapabilities { - async_shell_jobs: true, - ..Default::default() - }; - let auth = open_auth_context(); - register_agent_projects_for_auth( - &runtime, - "handoff-stop-pending", - &auth, - caps, - vec![registered_project("demo", "/tmp/handoff-stop-pending-demo")], - ) - .await; - let project = "agent:handoff-stop-pending:demo".to_string(); + let temp = tempfile::tempdir().unwrap(); + let runtime = runtime_with_project(temp.path(), "demo"); + let project = "demo"; let session = runtime.sessions.start_session( - Some(project.clone()), + Some(project.to_string()), Some("handoff stop pending".to_string()), ); - let sid = session.session_id.clone(); - let run = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: project.clone(), - command: "printf handoff-stop-pending-secret".to_string(), - session_id: Some(sid.clone()), - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - Some(&auth), - ) - .await; - assert!(run.success, "{:?}", run.error); - let job_id = run.output["job_id"].as_str().unwrap().to_string(); - let start_req = next_agent_request_for_client(&runtime, "handoff-stop-pending") - .await - .expect("agent should receive start_job"); - assert_eq!(start_req.kind, "start_job"); - - let stop = runtime - .dispatch_with_auth( - ToolCall::StopJob { - project: project.clone(), - job_id: job_id.clone(), - session_id: Some(sid.clone()), - confirm: true, - }, - Some(&auth), - ) - .await; - assert!(stop.success, "{:?}", stop.error); - assert_eq!(stop.output["status_after"], "stop_requested"); - - let summary = runtime - .dispatch_with_auth( - ToolCall::SessionHandoffSummary { - session_id: sid, - project: Some(project), - include_workspace: Some(false), - include_checkpoints: Some(false), - include_validation: Some(false), - summary_only: false, - limit: Some(20), - }, - Some(&auth), - ) - .await; + let job_id = "11111111-2222-3333-4444-555555555552"; + seed_session_projection_job( + &runtime, + temp.path(), + job_id, + project, + &session.session_id, + "stop_requested", + "handoff-stop-pending-secret\n", + ) + .await; + + let summary = handoff_jobs_projection(&runtime, &session.session_id).await; assert!(summary.success, "{:?}", summary.error); assert_eq!(summary.output["jobs"]["active_count"], 1); assert_eq!(summary.output["jobs"]["running_count"], 0); @@ -3274,20 +3191,18 @@ async fn complete_agent_shell_requests_until_finished( client_id: &str, task: &tokio::task::JoinHandle, ) { - for _ in 0..200 { - if task.is_finished() { - return; - } + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while !task.is_finished() { + assert!( + std::time::Instant::now() < deadline, + "tool did not finish within 10 seconds after Agent requests for {client_id}" + ); if let Some(req) = next_patch_agent_request(runtime, client_id).await { complete_agent_request_by_running_locally(runtime, client_id, req).await; } else { tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } - assert!( - task.is_finished(), - "tool did not finish after agent requests" - ); } async fn handoff_summary(runtime: &ToolRuntime, session_id: &str) -> ToolResult { @@ -3351,9 +3266,7 @@ async fn dispatch_handoff_with_agent( // If include_workspace is true, the internal show_changes call enqueues // an agent shell request. Complete it locally. if include_workspace { - let req = next_patch_agent_request(runtime, client_id) - .await - .expect("handoff workspace should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(runtime, client_id).await; complete_agent_request_by_running_locally(runtime, client_id, req).await; } @@ -3384,9 +3297,7 @@ async fn dispatch_handoff_summary_only_with_agent( }); if include_workspace { - let req = next_patch_agent_request(runtime, client_id) - .await - .expect("handoff workspace should enqueue an agent shell request"); + let req = wait_for_patch_agent_request(runtime, client_id).await; complete_agent_request_by_running_locally(runtime, client_id, req).await; } diff --git a/src/tool_runtime/tests/handoff_brief.rs b/src/tool_runtime/tests/handoff_brief.rs index 8660c71d..d89a606e 100644 --- a/src/tool_runtime/tests/handoff_brief.rs +++ b/src/tool_runtime/tests/handoff_brief.rs @@ -60,19 +60,23 @@ fn add_instruction_for_project( .unwrap(); } -fn empty_jobs() -> Value { +fn jobs_summary(running: u64, recovering: u64, terminal_pending: u64) -> Value { json!({ - "active_count": 0, - "running_count": 0, - "recovering_count": 0, - "terminal_pending_count": 0, - "blocking_active_count": 0, - "nonblocking_active_count": 0, + "active_count": running + terminal_pending, + "running_count": running, + "recovering_count": recovering, + "terminal_pending_count": terminal_pending, + "blocking_active_count": running, + "nonblocking_active_count": terminal_pending, "recent": [], "truncated": false, }) } +fn empty_jobs() -> Value { + jobs_summary(0, 0, 0) +} + fn clean_workspace() -> Value { json!({ "git_available": true, @@ -437,15 +441,7 @@ fn handoff_brief_progress_state_uses_only_proven_blockers() { assert_eq!(conflicted["progress"]["state"], "blocked"); assert_eq!(conflicted["attention"]["workspace_conflict"], true); - let blocking_jobs = json!({ - "active_count": 1, - "running_count": 1, - "recovering_count": 0, - "terminal_pending_count": 0, - "blocking_active_count": 1, - "recent": [], - "truncated": false, - }); + let blocking_jobs = jobs_summary(1, 0, 0); let blocked = brief_for( &store, &session_id, @@ -458,15 +454,7 @@ fn handoff_brief_progress_state_uses_only_proven_blockers() { ); assert_eq!(blocked["progress"]["state"], "blocked"); - let recovering_jobs = json!({ - "active_count": 1, - "running_count": 1, - "recovering_count": 1, - "terminal_pending_count": 0, - "blocking_active_count": 1, - "recent": [], - "truncated": false, - }); + let recovering_jobs = jobs_summary(1, 1, 0); let recovering = brief_for( &store, &session_id, @@ -479,15 +467,7 @@ fn handoff_brief_progress_state_uses_only_proven_blockers() { ); assert_eq!(recovering["progress"]["state"], "blocked"); - let terminal_pending_jobs = json!({ - "active_count": 1, - "running_count": 0, - "recovering_count": 0, - "terminal_pending_count": 1, - "blocking_active_count": 0, - "recent": [], - "truncated": false, - }); + let terminal_pending_jobs = jobs_summary(0, 0, 1); let terminal_pending = brief_for( &store, &session_id, @@ -770,15 +750,7 @@ fn handoff_brief_next_action_priority_is_stable() { }) .unwrap(); let workspace = dirty_workspace(1); - let jobs = json!({ - "active_count": 1, - "running_count": 1, - "recovering_count": 1, - "terminal_pending_count": 0, - "blocking_active_count": 1, - "recent": [], - "truncated": false, - }); + let jobs = jobs_summary(1, 1, 0); let brief = brief_for( &store, @@ -1316,17 +1288,18 @@ async fn finish_and_handoff_surfaces_return_the_same_brief_for_the_same_snapshot .await } }); - for _ in 0..200 { - if finish_task.is_finished() { - break; - } + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + while !finish_task.is_finished() { + assert!( + std::time::Instant::now() < deadline, + "finish_coding_task did not finish within 10 seconds while proving shared handoff brief" + ); if let Some(request) = next_patch_agent_request(&runtime, client_id).await { complete_agent_request_by_running_locally(&runtime, client_id, request).await; } else { - tokio::task::yield_now().await; + tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } - assert!(finish_task.is_finished()); let finish = finish_task.await.unwrap(); assert!(finish.success, "{:?}", finish.error); diff --git a/src/tool_runtime/tests/hygiene.rs b/src/tool_runtime/tests/hygiene.rs index 6705e563..69525d0c 100644 --- a/src/tool_runtime/tests/hygiene.rs +++ b/src/tool_runtime/tests/hygiene.rs @@ -37,10 +37,12 @@ async fn dispatch_hygiene_with_agent( }); let forbidden = ["python3", "-c"].join(" "); - for _ in 0..200 { - if task.is_finished() { - break; - } + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + while !task.is_finished() { + assert!( + tokio::time::Instant::now() < deadline, + "hygiene check did not finish within 10 seconds for client {client_id}" + ); if let Some(req) = next_patch_agent_request(runtime, client_id).await { assert_eq!(req.kind, "run_internal_posix_script"); assert!(req.command.is_empty()); @@ -63,10 +65,6 @@ async fn dispatch_hygiene_with_agent( tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } - assert!( - task.is_finished(), - "hygiene check did not finish after read-only agent requests" - ); task.await.unwrap() } diff --git a/src/tool_runtime/tests/jobs.rs b/src/tool_runtime/tests/jobs.rs index 22250e1d..888f8f03 100644 --- a/src/tool_runtime/tests/jobs.rs +++ b/src/tool_runtime/tests/jobs.rs @@ -62,9 +62,7 @@ async fn run_shell_session_events_record_exit_without_stdio_bodies() { .await } }); - let req = next_patch_agent_request(&runtime, "telemetry-shell") - .await - .expect("run_shell should enqueue success request"); + let req = wait_for_patch_agent_request(&runtime, "telemetry-shell").await; complete_patch_agent_request( &runtime, "telemetry-shell", @@ -104,9 +102,7 @@ async fn run_shell_session_events_record_exit_without_stdio_bodies() { .await } }); - let req = next_patch_agent_request(&runtime, "telemetry-shell") - .await - .expect("run_shell should enqueue failure request"); + let req = wait_for_patch_agent_request(&runtime, "telemetry-shell").await; complete_patch_agent_request( &runtime, "telemetry-shell", @@ -490,9 +486,7 @@ async fn run_shell_via_agent( .run_shell(project, command, timeout_secs, None) .await }); - let req = next_patch_agent_request(&runtime, client_id) - .await - .expect("run_shell should enqueue a shell command"); + let req = wait_for_patch_agent_request(&runtime, client_id).await; if let Some((exit_code, stdout, stderr)) = completion { complete_patch_agent_request( &runtime, @@ -525,9 +519,7 @@ async fn run_shell_via_agent_lifecycle_error( .run_shell(project, "printf lifecycle".to_string(), Some(30), None) .await }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("run_shell should be dispatched"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; runtime .shell_clients .complete(ShellAgentResultPayload { @@ -632,16 +624,7 @@ async fn long_run_shell_hands_off_same_job_once_and_status_log_stop_observe_it() } }); - let start = match next_patch_agent_request(&runtime, client_id).await { - Some(start) => start, - None => { - let early = task.await.unwrap(); - panic!( - "long run_shell ended before durable Job start: success={} error={:?} output={}", - early.success, early.error, early.output - ); - } - }; + let start = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(start.kind, "start_job"); assert_eq!(start.timeout_secs, 120); assert!(start.command.starts_with("exec bash -c ")); @@ -730,9 +713,7 @@ async fn long_run_shell_hands_off_same_job_once_and_status_log_stop_observe_it() ) .await; assert!(stopped.success, "{:?}", stopped.error); - let stop_request = next_patch_agent_request(&runtime, client_id) - .await - .expect("stop_job request"); + let stop_request = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(stop_request.kind, "stop_job"); assert_eq!(stop_request.job_id.as_deref(), Some(job_id.as_str())); update_agent_shell_job( @@ -782,9 +763,7 @@ async fn long_run_shell_fast_terminal_returns_ordinary_result_without_visible_jo .await } }); - let start = next_patch_agent_request(&runtime, client_id) - .await - .expect("durable start request"); + let start = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(start.kind, "start_job"); let job_id = start.job_id.clone().unwrap(); update_agent_shell_job( @@ -837,9 +816,7 @@ async fn run_shell_default_sixty_stays_synchronous_even_with_async_job_capabilit .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("ordinary synchronous run_shell request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request.kind, "run_shell"); assert_eq!(request.timeout_secs, 60); complete_patch_agent_request(&runtime, client_id, &request.request_id, 0, "default", "").await; @@ -872,9 +849,7 @@ async fn long_run_shell_without_async_job_capability_keeps_legacy_sync_dispatch( .await } }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("legacy synchronous request"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; assert_eq!(request.kind, "run_shell"); assert_eq!(request.timeout_secs, 120); complete_patch_agent_request(&runtime, client_id, &request.request_id, 0, "legacy", "").await; @@ -1018,9 +993,7 @@ async fn long_run_shell_job_timeout_is_terminal_and_never_becomes_fake_outcome_u .await } }); - let start = next_patch_agent_request(&runtime, client_id) - .await - .expect("durable Job start"); + let start = wait_for_patch_agent_request(&runtime, client_id).await; let job_id = start.job_id.clone().unwrap(); update_agent_shell_job( &runtime, @@ -1358,9 +1331,7 @@ async fn run_shell_runner_timeout_preserves_known_timeout_state() { .run_shell(project, "sleep 2".to_string(), Some(1), None) .await }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("run_shell should be dispatched"); + let request = wait_for_patch_agent_request(&runtime, client_id).await; runtime .shell_clients .complete(ShellAgentResultPayload { @@ -1409,9 +1380,7 @@ async fn run_shell_transport_disconnect_after_dispatch_reports_unknown_outcome() .run_shell(project, "printf possibly-ran".to_string(), Some(30), None) .await }); - next_patch_agent_request(&runtime, client_id) - .await - .expect("run_shell should be dispatched before disconnect"); + wait_for_patch_agent_request(&runtime, client_id).await; runtime .shell_clients @@ -2134,9 +2103,8 @@ async fn model_facing_stop_job_reports_requested_and_already_stop_requested() { .await; assert!(run.success, "{:?}", run.error); let job_id = run.output["job_id"].as_str().unwrap().to_string(); - let start_req = next_agent_request_for_instance(&runtime, "client-stop-pending", "inst") - .await - .expect("agent should receive start_job"); + let start_req = + wait_for_agent_request_for_instance(&runtime, "client-stop-pending", "inst").await; assert_eq!(start_req.kind, "start_job"); let result = runtime @@ -2534,9 +2502,7 @@ async fn start_agent_runtime_job_in_session( } async fn mark_next_agent_job_running(runtime: &ToolRuntime, client_id: &str) -> String { - let request = next_agent_request_for_instance(runtime, client_id, "inst") - .await - .expect("Agent Job request should be queued"); + let request = wait_for_agent_request_for_instance(runtime, client_id, "inst").await; let job_id = request.job_id.clone().expect("Job request id"); runtime .shell_clients @@ -2634,9 +2600,7 @@ async fn shared_key_runtime_job_tools_filter_agent_jobs_by_auth_group() { let job_b = start_agent_runtime_job(&runtime, "client-b", "proj-b", &shared_b).await; let job_open = start_agent_runtime_job(&runtime, "client-open", "proj-open", &open).await; - let req = next_agent_request_for_instance(&runtime, "client-b", "inst") - .await - .expect("client-b job request should be queued"); + let req = wait_for_agent_request_for_instance(&runtime, "client-b", "inst").await; complete_patch_agent_request(&runtime, "client-b", &req.request_id, 0, "b-out", "b-err").await; let list_a = runtime @@ -2976,9 +2940,7 @@ async fn list_jobs_filters_visible_jobs_by_project_session_and_status_before_lim mark_next_agent_job_running(&runtime, "target-a").await, job_a1_running ); - let completed_request = next_agent_request_for_instance(&runtime, "target-a", "inst") - .await - .expect("second A1 Job request"); + let completed_request = wait_for_agent_request_for_instance(&runtime, "target-a", "inst").await; assert_eq!( completed_request.job_id.as_deref(), Some(job_a1_completed.as_str()) diff --git a/src/tool_runtime/tests/metadata.rs b/src/tool_runtime/tests/metadata.rs index f0875f05..b2cd8a77 100644 --- a/src/tool_runtime/tests/metadata.rs +++ b/src/tool_runtime/tests/metadata.rs @@ -104,6 +104,28 @@ fn list_agents_call() -> ToolCall { } } +fn metadata_agent_registration( + client_id: &str, + protocol: Option<&str>, +) -> ShellClientRegisterRequest { + ShellClientRegisterRequest { + process_started_at: None, + build: None, + job_concurrency_limit: None, + job_inventory: None, + client_id: client_id.to_string(), + agent_instance_id: format!("inst-{client_id}"), + display_name: None, + owner: None, + hostname: None, + host_context: None, + capabilities: None, + projects: None, + agent_protocol_version: protocol.map(str::to_string), + policy: None, + } +} + async fn register_computer_target_for_auth( runtime: &ToolRuntime, client_id: &str, @@ -1258,18 +1280,55 @@ async fn unique_short_agent_project_id_is_resolved_by_runtime_surface() { } #[tokio::test] -async fn agent_run_shell_without_shell_capability_is_rejected() { - let runtime = runtime_with_agent_project("oe"); - let caps = ShellClientCapabilities { - shell: false, - ..Default::default() - }; - register_agent(&runtime, "oe", None, caps).await; +async fn agent_capability_rejection_matrix_names_required_capability() { + enum CapabilityCase { + RunShell, + ReadFile, + RunJob, + GitStatus, + } + + let cases = [ + ( + "cap-shell", + ShellClientCapabilities { + shell: false, + ..Default::default() + }, + CapabilityCase::RunShell, + vec!["does not support shell", "agent client cap-shell"], + ), + ( + "cap-read", + ShellClientCapabilities::default(), + CapabilityCase::ReadFile, + vec!["does not support file_read"], + ), + ( + "cap-job", + ShellClientCapabilities::default(), + CapabilityCase::RunJob, + vec!["does not support async shell jobs"], + ), + ( + "cap-git", + ShellClientCapabilities { + shell: false, + ..Default::default() + }, + CapabilityCase::GitStatus, + vec!["does not support shell or git"], + ), + ]; let bootstrap = auth_context(None, true); - let result = runtime - .dispatch_with_auth( - ToolCall::RunShell { - project: agent_test_project_id("oe"), + + for (client_id, capabilities, case, expected_fragments) in cases { + let runtime = runtime_with_agent_project(client_id); + register_agent(&runtime, client_id, None, capabilities).await; + let project = agent_test_project_id(client_id); + let call = match case { + CapabilityCase::RunShell => ToolCall::RunShell { + project, command: "echo hi".to_string(), session_id: None, timeout_secs: None, @@ -1277,49 +1336,16 @@ async fn agent_run_shell_without_shell_capability_is_rejected() { purpose: None, shell: None, }, - Some(&bootstrap), - ) - .await; - assert!(!result.success); - let err = result.error.unwrap(); - assert!(err.contains("does not support shell"), "{}", err); - assert!(err.contains("agent client oe"), "{}", err); -} - -#[tokio::test] -async fn agent_read_file_without_file_read_capability_is_rejected() { - let runtime = runtime_with_agent_project("oe"); - // Default caps: shell=true, file_read=false. - register_agent(&runtime, "oe", None, ShellClientCapabilities::default()).await; - let bootstrap = auth_context(None, true); - let result = runtime - .dispatch_with_auth( - ToolCall::ReadFile { - project: agent_test_project_id("oe"), + CapabilityCase::ReadFile => ToolCall::ReadFile { + project, path: "README.md".to_string(), session_id: None, start_line: None, limit: None, with_line_numbers: None, }, - Some(&bootstrap), - ) - .await; - assert!(!result.success); - let err = result.error.unwrap(); - assert!(err.contains("does not support file_read"), "{}", err); -} - -#[tokio::test] -async fn agent_run_job_without_async_capability_is_rejected() { - let runtime = runtime_with_agent_project("oe"); - // Default caps: async_jobs=false, async_shell_jobs=false. - register_agent(&runtime, "oe", None, ShellClientCapabilities::default()).await; - let bootstrap = auth_context(None, true); - let result = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: agent_test_project_id("oe"), + CapabilityCase::RunJob => ToolCall::RunJob { + project, command: "echo hi".to_string(), session_id: None, timeout_secs: None, @@ -1327,40 +1353,24 @@ async fn agent_run_job_without_async_capability_is_rejected() { purpose: None, shell: None, }, - Some(&bootstrap), - ) - .await; - assert!(!result.success); - let err = result.error.unwrap(); - assert!(err.contains("does not support async shell jobs"), "{}", err); -} - -#[tokio::test] -async fn agent_git_status_without_shell_or_git_is_rejected() { - let runtime = runtime_with_agent_project("oe"); - register_agent( - &runtime, - "oe", - None, - ShellClientCapabilities { - shell: false, - ..Default::default() - }, - ) - .await; - let bootstrap = auth_context(None, true); - let result = runtime - .dispatch_with_auth( - ToolCall::GitStatus { - project: agent_test_project_id("oe"), + CapabilityCase::GitStatus => ToolCall::GitStatus { + project, session_id: None, }, - Some(&bootstrap), - ) - .await; - assert!(!result.success); - let err = result.error.unwrap(); - assert!(err.contains("does not support shell or git"), "{}", err); + }; + let result = runtime.dispatch_with_auth(call, Some(&bootstrap)).await; + assert!( + !result.success, + "{client_id}: capability gate unexpectedly allowed call" + ); + let error = result.error.unwrap_or_default(); + for fragment in expected_fragments { + assert!( + error.contains(fragment), + "{client_id}: missing {fragment:?} in {error:?}" + ); + } + } } #[tokio::test] @@ -1391,118 +1401,68 @@ async fn agent_tool_unknown_client_returns_unknown_project_error() { } #[tokio::test] -async fn agent_tool_rejects_non_owner_api_key() { - let runtime = runtime_with_agent_project("oe"); - let caps = ShellClientCapabilities { - async_shell_jobs: true, - ..Default::default() - }; - register_agent(&runtime, "oe", Some("alice"), caps).await; - let bob = auth_context(Some("bob"), false); - // Use run_job (async) so the test does not hang if owner check leaked. - let result = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: agent_test_project_id("oe"), - command: "echo hi".to_string(), - session_id: None, - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - Some(&bob), - ) - .await; - assert!(!result.success); - let err = result.error.unwrap(); - assert!(err.contains("owned by alice"), "{}", err); - assert!(err.contains("belongs to bob"), "{}", err); -} - -#[tokio::test] -async fn agent_tool_rejects_missing_auth_context() { - let runtime = runtime_with_agent_project("oe"); - let caps = ShellClientCapabilities { - shell: true, - ..Default::default() - }; - register_agent(&runtime, "oe", Some("alice"), caps).await; - // dispatch_with_auth(None): no owner can be proven for an owned agent. - let result = runtime - .dispatch_with_auth( - ToolCall::RunShell { - project: agent_test_project_id("oe"), - command: "echo hi".to_string(), - session_id: None, - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - None, - ) - .await; - assert!(!result.success); - let err = result.error.unwrap(); - assert!( - err.contains("owned by alice") || err.contains("belongs to anonymous"), - "{}", - err - ); -} - -#[tokio::test] -async fn agent_tool_allows_owner_api_key_for_run_job() { - let runtime = runtime_with_agent_project("oe"); - let caps = ShellClientCapabilities { - async_shell_jobs: true, - ..Default::default() - }; - register_agent(&runtime, "oe", Some("alice"), caps).await; +async fn agent_tool_authority_admission_matrix() { + let runtime = runtime_with_agent_project("authority-agent"); + register_agent( + &runtime, + "authority-agent", + Some("alice"), + ShellClientCapabilities { + async_shell_jobs: true, + ..Default::default() + }, + ) + .await; + let project = agent_test_project_id("authority-agent"); let alice = auth_context(Some("alice"), false); - let result = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: agent_test_project_id("oe"), - command: "echo hi".to_string(), - session_id: None, - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - Some(&alice), - ) - .await; - assert!(result.success, "{:?}", result.error); - assert!(result.output["job_id"].is_string()); -} - -#[tokio::test] -async fn agent_tool_allows_bootstrap_token_for_run_job() { - let runtime = runtime_with_agent_project("oe"); - let caps = ShellClientCapabilities { - async_shell_jobs: true, - ..Default::default() - }; - register_agent(&runtime, "oe", Some("alice"), caps).await; + let bob = auth_context(Some("bob"), false); let bootstrap = auth_context(None, true); - let result = runtime - .dispatch_with_auth( - ToolCall::RunJob { - project: agent_test_project_id("oe"), - command: "echo hi".to_string(), - session_id: None, - timeout_secs: None, - cwd: None, - purpose: None, - shell: None, - }, - Some(&bootstrap), - ) - .await; - assert!(result.success, "{:?}", result.error); + let cases = [ + ("wrong owner", Some(&bob), false), + ("missing auth", None, false), + ("owner PAT", Some(&alice), true), + ("bootstrap", Some(&bootstrap), true), + ]; + + for (label, auth, should_succeed) in cases { + let result = runtime + .dispatch_with_auth( + ToolCall::RunJob { + project: project.clone(), + command: "echo hi".to_string(), + session_id: None, + timeout_secs: None, + cwd: None, + purpose: None, + shell: None, + }, + auth, + ) + .await; + assert_eq!( + result.success, should_succeed, + "{label}: {:?}", + result.error + ); + if should_succeed { + assert!( + result.output["job_id"].is_string(), + "{label}: {}", + result.output + ); + continue; + } + let error = result.error.unwrap_or_default(); + assert!(error.contains("owned by alice"), "{label}: {error}"); + if label == "wrong owner" { + assert!(error.contains("belongs to bob"), "{error}"); + } else { + assert!( + error.contains("belongs to anonymous") || error.contains("owned by alice"), + "{error}" + ); + } + } } #[test] @@ -2024,10 +1984,6 @@ async fn runtime_status_with_no_projects_returns_configured_false() { let out = &result.output; assert_eq!(out["service"], "webcodex"); assert_eq!(out["version"], env!("CARGO_PKG_VERSION")); - assert!(out["build"].is_object()); - assert!(out["build"].get("git_commit").is_some()); - assert!(out["build"].get("git_dirty").is_some()); - assert!(out["build"].get("built_at").is_some()); assert!(out["server_time"].is_i64()); assert!(out["pid"].is_i64()); assert_eq!(out["authority"]["mode"], "trusted_agent"); @@ -2382,7 +2338,25 @@ async fn runtime_status_auth_enabled_reflects_runtime_info() { #[test] fn runtime_info_from_env_reads_webcodex_public_url() { - let _guard = crate::admin_cli::TEST_ENV_LOCK.lock().unwrap(); + let _guard = crate::admin_cli::TEST_ENV_LOCK + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()); + let previous = ["WEBCODEX_TOKEN", "WEBCODEX_PUBLIC_URL"] + .into_iter() + .map(|name| (name, std::env::var_os(name))) + .collect::>(); + struct RestoreEnv(Vec<(&'static str, Option)>); + impl Drop for RestoreEnv { + fn drop(&mut self) { + for (name, value) in self.0.drain(..).rev() { + match value { + Some(value) => std::env::set_var(name, value), + None => std::env::remove_var(name), + } + } + } + } + let _restore = RestoreEnv(previous); std::env::set_var("WEBCODEX_TOKEN", "token"); std::env::set_var("WEBCODEX_PUBLIC_URL", "https://new.example.com"); @@ -2392,34 +2366,18 @@ fn runtime_info_from_env_reads_webcodex_public_url() { info.configured_public_url.as_deref(), Some("https://new.example.com") ); - - std::env::remove_var("WEBCODEX_TOKEN"); - std::env::remove_var("WEBCODEX_PUBLIC_URL"); } #[tokio::test] async fn runtime_status_agent_summary_includes_protocol_version() { - use crate::shell_protocol::ShellClientRegisterRequest; let registry = Arc::new(ShellClientRegistry::default()); - registry - .register(ShellClientRegisterRequest { - process_started_at: None, - build: None, - job_concurrency_limit: Some(4), - job_inventory: None, - client_id: "agent-1".to_string(), - agent_instance_id: "inst".to_string(), - display_name: Some("Workstation".to_string()), - owner: Some("alice".to_string()), - hostname: None, - host_context: None, - capabilities: None, - projects: Some(vec![]), - agent_protocol_version: Some("polling-v1".to_string()), - policy: None, - }) - .await - .unwrap(); + let mut registration = metadata_agent_registration("agent-1", Some("polling-v1")); + registration.agent_instance_id = "inst".to_string(); + registration.job_concurrency_limit = Some(4); + registration.display_name = Some("Workstation".to_string()); + registration.owner = Some("alice".to_string()); + registration.projects = Some(vec![]); + registry.register(registration).await.unwrap(); let runtime = ToolRuntime::new(registry, Arc::new(RuntimeInfo::default())); let result = runtime.dispatch(runtime_status_call()).await; assert!(result.success); @@ -2470,53 +2428,39 @@ async fn runtime_status_agent_summary_includes_protocol_version() { async fn runtime_status_includes_sanitized_policy_summary() { use crate::shell_protocol::{ AgentConfigReloadStatus, AgentPolicySummary, ClaudeCodeProviderStatus, ProviderCallSummary, - ShellClientRegisterRequest, ToolProvidersStatus, + ToolProvidersStatus, }; let registry = Arc::new(ShellClientRegistry::default()); - registry - .register(ShellClientRegisterRequest { - process_started_at: None, - build: None, - job_concurrency_limit: None, - job_inventory: None, - client_id: "policy-agent".to_string(), - agent_instance_id: "inst-p".to_string(), - display_name: None, - owner: Some("alice".to_string()), - hostname: None, - host_context: None, - capabilities: None, - projects: None, - agent_protocol_version: Some("websocket-v1".to_string()), - policy: Some(AgentPolicySummary { - allow_raw_shell: true, - allow_cwd_anywhere: false, - allowed_roots: vec![std::path::PathBuf::from("/root")], - max_timeout_secs: 3600, - max_output_bytes: 262144, - shell_profiles: None, - tool_providers: Some(ToolProvidersStatus { - strategy: "claude_code".to_string(), - claude_code: ClaudeCodeProviderStatus { - enabled: true, - version: Some("test-version".to_string()), - available: true, - process_state: "running".to_string(), - discovered_tool_names: vec!["Edit".to_string()], - capabilities: std::collections::BTreeMap::from([ - ("search_project_text".to_string(), "unmapped".to_string()), - ("edit_file".to_string(), "available".to_string()), - ]), - last_error_code: None, - last_call: None, - }, - config_reload: AgentConfigReloadStatus::default(), - }), - mcp_gateway_providers: None, - }), - }) - .await - .unwrap(); + let mut registration = metadata_agent_registration("policy-agent", Some("websocket-v1")); + registration.agent_instance_id = "inst-p".to_string(); + registration.owner = Some("alice".to_string()); + registration.policy = Some(AgentPolicySummary { + allow_raw_shell: true, + allow_cwd_anywhere: false, + allowed_roots: vec![std::path::PathBuf::from("/root")], + max_timeout_secs: 3600, + max_output_bytes: 262144, + shell_profiles: None, + tool_providers: Some(ToolProvidersStatus { + strategy: "claude_code".to_string(), + claude_code: ClaudeCodeProviderStatus { + enabled: true, + version: Some("test-version".to_string()), + available: true, + process_state: "running".to_string(), + discovered_tool_names: vec!["Edit".to_string()], + capabilities: std::collections::BTreeMap::from([ + ("search_project_text".to_string(), "unmapped".to_string()), + ("edit_file".to_string(), "available".to_string()), + ]), + last_error_code: None, + last_call: None, + }, + config_reload: AgentConfigReloadStatus::default(), + }), + mcp_gateway_providers: None, + }); + registry.register(registration).await.unwrap(); let current_provider = ToolProvidersStatus { strategy: "claude_code".to_string(), claude_code: ClaudeCodeProviderStatus { @@ -2595,8 +2539,7 @@ async fn runtime_status_includes_sanitized_policy_summary() { #[tokio::test] async fn external_provider_discovery_cannot_change_public_tool_or_openapi_surface() { use crate::shell_protocol::{ - AgentPolicySummary, ClaudeCodeProviderStatus, ShellClientRegisterRequest, - ToolProvidersStatus, + AgentPolicySummary, ClaudeCodeProviderStatus, ToolProvidersStatus, }; let before = crate::tool_runtime::registry::registered_tool_specs(); let names_before = before @@ -2612,47 +2555,32 @@ async fn external_provider_discovery_cannot_change_public_tool_or_openapi_surfac .input_schema .clone(); let registry = Arc::new(ShellClientRegistry::default()); - registry - .register(ShellClientRegisterRequest { - process_started_at: None, - build: None, - job_concurrency_limit: None, - job_inventory: None, - client_id: "provider-surface".to_string(), - agent_instance_id: "inst-surface".to_string(), - display_name: None, - owner: None, - hostname: None, - host_context: None, - capabilities: None, - projects: None, - agent_protocol_version: Some("websocket-v1".to_string()), - policy: Some(AgentPolicySummary { - tool_providers: Some(ToolProvidersStatus { - strategy: "claude_code_then_native".to_string(), - claude_code: ClaudeCodeProviderStatus { - enabled: true, - version: Some("2.1.217".to_string()), - available: true, - process_state: "running".to_string(), - discovered_tool_names: ["Edit", "Read", "Bash", "Write", "FutureTool"] - .into_iter() - .map(str::to_string) - .collect(), - capabilities: std::collections::BTreeMap::from([ - ("edit_file".to_string(), "available".to_string()), - ("search_project_text".to_string(), "unmapped".to_string()), - ]), - last_error_code: None, - last_call: None, - }, - config_reload: Default::default(), - }), - ..AgentPolicySummary::default() - }), - }) - .await - .unwrap(); + let mut registration = metadata_agent_registration("provider-surface", Some("websocket-v1")); + registration.agent_instance_id = "inst-surface".to_string(); + registration.policy = Some(AgentPolicySummary { + tool_providers: Some(ToolProvidersStatus { + strategy: "claude_code_then_native".to_string(), + claude_code: ClaudeCodeProviderStatus { + enabled: true, + version: Some("2.1.217".to_string()), + available: true, + process_state: "running".to_string(), + discovered_tool_names: ["Edit", "Read", "Bash", "Write", "FutureTool"] + .into_iter() + .map(str::to_string) + .collect(), + capabilities: std::collections::BTreeMap::from([ + ("edit_file".to_string(), "available".to_string()), + ("search_project_text".to_string(), "unmapped".to_string()), + ]), + last_error_code: None, + last_call: None, + }, + config_reload: Default::default(), + }), + ..AgentPolicySummary::default() + }); + registry.register(registration).await.unwrap(); let runtime = ToolRuntime::new(registry, Arc::new(RuntimeInfo::default())); let status = runtime.dispatch(runtime_status_call()).await; let public_names = status.output["tools"]["names"] @@ -2691,28 +2619,11 @@ async fn external_provider_discovery_cannot_change_public_tool_or_openapi_surfac #[tokio::test] async fn runtime_status_policy_summary_is_null_for_older_agents() { - use crate::shell_protocol::ShellClientRegisterRequest; let registry = Arc::new(ShellClientRegistry::default()); // Older agent: no policy field (None). - registry - .register(ShellClientRegisterRequest { - process_started_at: None, - build: None, - job_concurrency_limit: None, - job_inventory: None, - client_id: "legacy-agent".to_string(), - agent_instance_id: "inst-l".to_string(), - display_name: None, - owner: None, - hostname: None, - host_context: None, - capabilities: None, - projects: None, - agent_protocol_version: None, - policy: None, - }) - .await - .unwrap(); + let mut registration = metadata_agent_registration("legacy-agent", None); + registration.agent_instance_id = "inst-l".to_string(); + registry.register(registration).await.unwrap(); let runtime = ToolRuntime::new(registry, Arc::new(RuntimeInfo::default())); let result = runtime.dispatch(runtime_status_call()).await; assert!(result.success); @@ -2966,36 +2877,23 @@ async fn computer_list_targets_is_minimal_capability_filtered_and_auth_scoped() #[tokio::test] async fn list_agents_includes_sanitized_policy_summary() { - use crate::shell_protocol::{AgentPolicySummary, ShellClientRegisterRequest}; + use crate::shell_protocol::AgentPolicySummary; let registry = Arc::new(ShellClientRegistry::default()); - registry - .register(ShellClientRegisterRequest { - process_started_at: None, - build: None, - job_concurrency_limit: Some(8), - job_inventory: None, - client_id: "list-policy-agent".to_string(), - agent_instance_id: "inst-lp".to_string(), - display_name: None, - owner: Some("alice".to_string()), - hostname: None, - host_context: None, - capabilities: None, - projects: None, - agent_protocol_version: Some("websocket-v1".to_string()), - policy: Some(AgentPolicySummary { - allow_raw_shell: false, - allow_cwd_anywhere: true, - allowed_roots: vec![], - max_timeout_secs: 120, - max_output_bytes: 4096, - shell_profiles: None, - tool_providers: None, - mcp_gateway_providers: None, - }), - }) - .await - .unwrap(); + let mut registration = metadata_agent_registration("list-policy-agent", Some("websocket-v1")); + registration.agent_instance_id = "inst-lp".to_string(); + registration.job_concurrency_limit = Some(8); + registration.owner = Some("alice".to_string()); + registration.policy = Some(AgentPolicySummary { + allow_raw_shell: false, + allow_cwd_anywhere: true, + allowed_roots: vec![], + max_timeout_secs: 120, + max_output_bytes: 4096, + shell_profiles: None, + tool_providers: None, + mcp_gateway_providers: None, + }); + registry.register(registration).await.unwrap(); let runtime = ToolRuntime::new(registry, Arc::new(RuntimeInfo::default())); let result = runtime.dispatch(list_agents_call()).await; assert!(result.success); @@ -3036,27 +2934,13 @@ async fn list_agents_includes_sanitized_policy_summary() { #[tokio::test] async fn runtime_status_distinguishes_stale_registration_from_transport_connection() { use crate::shell_client::TRANSPORT_WEBSOCKET; - use crate::shell_protocol::ShellClientRegisterRequest; let registry = Arc::new(ShellClientRegistry::default()); - registry - .register(ShellClientRegisterRequest { - process_started_at: None, - build: None, - job_concurrency_limit: None, - job_inventory: None, - client_id: "ws-stale".to_string(), - agent_instance_id: "inst".to_string(), - display_name: Some("Stale WS".to_string()), - owner: Some("alice".to_string()), - hostname: None, - host_context: None, - capabilities: None, - projects: Some(vec![]), - agent_protocol_version: Some("websocket-v1".to_string()), - policy: None, - }) - .await - .unwrap(); + let mut registration = metadata_agent_registration("ws-stale", Some("websocket-v1")); + registration.agent_instance_id = "inst".to_string(); + registration.display_name = Some("Stale WS".to_string()); + registration.owner = Some("alice".to_string()); + registration.projects = Some(vec![]); + registry.register(registration).await.unwrap(); registry .set_transport("ws-stale", TRANSPORT_WEBSOCKET) .await @@ -3101,25 +2985,10 @@ async fn runtime_status_distinguishes_stale_registration_from_transport_connecti async fn runtime_status_reflects_websocket_transport_label() { let registry = Arc::new(ShellClientRegistry::default()); let runtime = ToolRuntime::new(registry.clone(), Arc::new(RuntimeInfo::default())); - registry - .register(ShellClientRegisterRequest { - process_started_at: None, - build: None, - job_concurrency_limit: None, - job_inventory: None, - client_id: "ws-agent".to_string(), - agent_instance_id: "inst".to_string(), - display_name: None, - owner: Some("alice".to_string()), - hostname: None, - host_context: None, - capabilities: None, - projects: None, - agent_protocol_version: Some("websocket-v1".to_string()), - policy: None, - }) - .await - .unwrap(); + let mut registration = metadata_agent_registration("ws-agent", Some("websocket-v1")); + registration.agent_instance_id = "inst".to_string(); + registration.owner = Some("alice".to_string()); + registry.register(registration).await.unwrap(); // Flip the transport label the same way the WebSocket handler does. registry .set_transport("ws-agent", crate::shell_client::TRANSPORT_WEBSOCKET) diff --git a/src/tool_runtime/tests/process.rs b/src/tool_runtime/tests/process.rs index a9fd94f5..dd95da3d 100644 --- a/src/tool_runtime/tests/process.rs +++ b/src/tool_runtime/tests/process.rs @@ -244,6 +244,23 @@ async fn complete_process_lifecycle( .unwrap(); } +async fn dispatch_process_until_request( + runtime: &ToolRuntime, + client_id: &str, + call: ToolCall, + auth: crate::auth::AuthContext, +) -> ( + tokio::task::JoinHandle, + crate::shell_protocol::ShellAgentShellRequest, +) { + let task = tokio::spawn({ + let runtime = runtime.clone(); + async move { runtime.dispatch_with_auth(call, Some(&auth)).await } + }); + let request = wait_for_patch_agent_request(runtime, client_id).await; + (task, request) +} + #[tokio::test] async fn run_process_local_direct_executor_preserves_argv_and_stdin_without_a_shell() { let cwd = tempfile::tempdir().unwrap(); @@ -315,9 +332,7 @@ async fn run_process_enqueues_only_typed_argv_and_reports_completed_exit_codes() .await } }); - let request = next_patch_agent_request(&runtime, "process-agent") - .await - .expect("run_process should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "process-agent").await; assert_eq!(request.kind, "run_process"); assert_eq!(request.command, ""); let process = request.process.as_ref().expect("typed process payload"); @@ -501,9 +516,8 @@ async fn detached_process_requires_job_run_and_detach_scopes_before_any_admissio assert!(result.success, "{:?}", result.error); assert_eq!(runtime.shell_clients.list_jobs(Some(10)).await.len(), 1); assert_eq!( - next_patch_agent_request(&runtime, "detached-scope-gate") + wait_for_patch_agent_request(&runtime, "detached-scope-gate") .await - .expect("both scopes dispatch exactly one Job") .kind, "start_detached_process_job" ); @@ -570,9 +584,7 @@ async fn detached_process_idempotency_replays_same_intent_and_rejects_conflict() .await; assert!(first.success, "{:?}", first.error); let job_id = first.output["job_id"].as_str().unwrap().to_string(); - let request = next_patch_agent_request(&runtime, "detached-idempotency") - .await - .expect("first initiation dispatches"); + let request = wait_for_patch_agent_request(&runtime, "detached-idempotency").await; assert_eq!(request.job_id.as_deref(), Some(job_id.as_str())); let replay = runtime @@ -649,9 +661,7 @@ async fn detached_process_lost_initiation_after_server_restart_recovers_same_job .await; assert!(first.success, "{:?}", first.error); let job_id = first.output["job_id"].as_str().unwrap().to_string(); - let request = next_patch_agent_request(&first_runtime, "detached-restart-recovery") - .await - .expect("first initiation dispatches"); + let request = wait_for_patch_agent_request(&first_runtime, "detached-restart-recovery").await; let context = request .job_context .clone() @@ -788,9 +798,7 @@ async fn detached_process_uses_existing_job_identity_and_typed_runner_request() let job_id = result.output["job_id"].as_str().unwrap().to_string(); assert_eq!(result.output["execution_source"], "run_detached_process"); - let request = next_patch_agent_request(&runtime, "detached-product-path") - .await - .expect("detached product path should dispatch one typed Job request"); + let request = wait_for_patch_agent_request(&runtime, "detached-product-path").await; assert_eq!(request.kind, "start_detached_process_job"); assert_eq!(request.job_id.as_deref(), Some(job_id.as_str())); assert!(request.process.is_some()); @@ -808,19 +816,13 @@ async fn run_process_fast_terminal_jobs_project_back_without_visible_duplicates( let auth = auth_context(None, true); for (exit_code, status, expected_success) in [(0, "completed", true), (19, "failed", false)] { - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth(process_call(project, None), Some(&auth)) - .await - } - }); - let request = next_patch_agent_request(&runtime, "process-fast-job") - .await - .expect("hidden process Job should dispatch"); + let (task, request) = dispatch_process_until_request( + &runtime, + "process-fast-job", + process_call(project.clone(), None), + auth.clone(), + ) + .await; assert_eq!(request.kind, "start_process_job"); assert_eq!(request.command, ""); assert!(request.process.is_some()); @@ -911,20 +913,13 @@ async fn run_process_terminal_success_is_sparse_after_full_session_effect_record let session_id = session.session_id.clone(); let auth = auth_context(None, true); - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let session_id = session_id.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth(process_call(project, Some(session_id)), Some(&auth)) - .await - } - }); - let request = next_patch_agent_request(&runtime, "process-sparse-ledger") - .await - .expect("hidden process Job should dispatch"); + let (task, request) = dispatch_process_until_request( + &runtime, + "process-sparse-ledger", + process_call(project.clone(), Some(session_id.clone())), + auth.clone(), + ) + .await; update_process_job( &runtime, "process-sparse-ledger", @@ -1038,19 +1033,13 @@ async fn run_process_fast_terminal_projection_does_not_silently_drop_retained_li "fixture must fit the model-facing terminal projection bound" ); - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth(process_call(project, None), Some(&auth)) - .await - } - }); - let request = next_patch_agent_request(&runtime, "process-fast-log-job") - .await - .expect("hidden process Job should dispatch"); + let (task, request) = dispatch_process_until_request( + &runtime, + "process-fast-log-job", + process_call(project.clone(), None), + auth.clone(), + ) + .await; update_process_job( &runtime, "process-fast-log-job", @@ -1094,19 +1083,13 @@ async fn run_process_fast_prestart_rejection_retains_not_started_through_the_hid let runtime = test_runtime().with_structured_execution_sync_wait(Duration::from_millis(250)); let project = register_process_job_agent(&runtime, "process-prestart-job", temp.path()).await; let auth = auth_context(None, true); - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth(process_call(project, None), Some(&auth)) - .await - } - }); - let request = next_patch_agent_request(&runtime, "process-prestart-job") - .await - .expect("hidden process Job should dispatch"); + let (task, request) = dispatch_process_until_request( + &runtime, + "process-prestart-job", + process_call(project.clone(), None), + auth.clone(), + ) + .await; let queued = runtime .shell_clients .get_hidden_job_for_auth(Some(&auth), request.job_id.as_deref().unwrap()) @@ -1148,35 +1131,26 @@ async fn run_process_slow_handoff_is_queryable_once_and_keeps_the_original_budge let project = register_process_job_agent(&runtime, "process-slow-job", temp.path()).await; let auth = auth_context(None, true); let started = Instant::now(); - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth( - ToolCall::RunProcess { - project, - executable: "argv-helper".to_string(), - args: vec![ - "two words".to_string(), - "$(literal)".to_string(), - "雪".to_string(), - ], - stdin: Some("input\n".to_string()), - session_id: None, - timeout_secs: Some(121), - cwd: None, - purpose: Some(ExecutionPurpose::Diagnostic), - }, - Some(&auth), - ) - .await - } - }); - let request = next_patch_agent_request(&runtime, "process-slow-job") - .await - .expect("typed process Job should dispatch"); + let (task, request) = dispatch_process_until_request( + &runtime, + "process-slow-job", + ToolCall::RunProcess { + project: project.clone(), + executable: "argv-helper".to_string(), + args: vec![ + "two words".to_string(), + "$(literal)".to_string(), + "雪".to_string(), + ], + stdin: Some("input\n".to_string()), + session_id: None, + timeout_secs: Some(121), + cwd: None, + purpose: Some(ExecutionPurpose::Diagnostic), + }, + auth.clone(), + ) + .await; assert_eq!(request.kind, "start_process_job"); assert_eq!(request.command, ""); assert!(request.process.is_some()); @@ -1243,14 +1217,6 @@ async fn run_process_slow_handoff_is_queryable_once_and_keeps_the_original_budge listed.iter().filter(|job| job["job_id"] == job_id).count(), 1 ); - let listed_job = listed - .iter() - .find(|job| job["job_id"] == job_id) - .expect("promoted process Job summary"); - assert_eq!( - listed_job["structured_execution"]["execution_source"], - "run_process" - ); assert!(next_patch_agent_request(&runtime, "process-slow-job") .await .is_none()); @@ -1267,66 +1233,22 @@ async fn run_process_slow_handoff_is_queryable_once_and_keeps_the_original_budge None, ) .await; - let log = runtime + let terminal = runtime .dispatch_with_auth( - ToolCall::JobLog { + ToolCall::JobStatus { job_id: job_id.clone(), - offset: None, - tail_lines: Some(20), - after_observation_token: None, - wait_secs: None, + include_command_preview: false, }, Some(&auth), ) .await; - assert!(log.success, "{:?}", log.error); - assert_eq!(log.output["status"], "completed"); - assert!(log.output["stdout_tail"] - .as_str() - .unwrap() - .contains("same execution complete")); - assert_eq!(log.output["command_execution_state"], "completed"); + assert!(terminal.success, "{:?}", terminal.error); + assert_eq!(terminal.output["status"], "completed"); + assert_eq!(terminal.output["command_execution_state"], "completed"); assert_eq!( - log.output["structured_execution"]["execution_source"], + terminal.output["structured_execution"]["execution_source"], "run_process" ); - let observed = runtime - .dispatch_with_auth( - ToolCall::ObserveJobs { - items: vec![ObserveJobsItem { - job_id: job_id.clone(), - after_observation_token: None, - }], - tail_lines: 20, - wait_secs: None, - }, - Some(&auth), - ) - .await; - assert!(observed.success, "{:?}", observed.error); - let observed_job = &observed.output["items"][0]["output"]; - assert_eq!(observed_job["status"], log.output["status"]); - assert_eq!( - observed_job["command_execution_state"], - log.output["command_execution_state"] - ); - assert_eq!( - observed_job["structured_execution"], - log.output["structured_execution"] - ); - assert_eq!( - observed_job["observation_token"], - log.output["observation_token"] - ); - assert_eq!( - runtime - .shell_clients - .get_job(&job_id) - .await - .unwrap() - .command_execution_state, - Some(ShellCommandExecutionState::Completed) - ); assert!(next_patch_agent_request(&runtime, "process-slow-job") .await .is_none()); @@ -1338,19 +1260,13 @@ async fn stop_job_stops_the_promoted_process_without_starting_a_replacement() { let runtime = test_runtime().with_structured_execution_sync_wait(Duration::from_millis(30)); let project = register_process_job_agent(&runtime, "process-stop-job", temp.path()).await; let auth = auth_context(None, true); - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth(process_call(project, None), Some(&auth)) - .await - } - }); - let start_request = next_patch_agent_request(&runtime, "process-stop-job") - .await - .expect("structured process Job start"); + let (task, start_request) = dispatch_process_until_request( + &runtime, + "process-stop-job", + process_call(project.clone(), None), + auth.clone(), + ) + .await; update_process_job( &runtime, "process-stop-job", @@ -1379,9 +1295,7 @@ async fn stop_job_stops_the_promoted_process_without_starting_a_replacement() { ) .await; assert!(stopped.success, "{:?}", stopped.error); - let stop_request = next_patch_agent_request(&runtime, "process-stop-job") - .await - .expect("existing stop_job API should dispatch"); + let stop_request = wait_for_patch_agent_request(&runtime, "process-stop-job").await; assert_eq!(stop_request.kind, "stop_job"); assert_eq!(stop_request.job_id.as_deref(), Some(job_id.as_str())); update_process_job( @@ -1422,20 +1336,13 @@ async fn promoted_process_inherits_the_initiating_session_without_a_second_tool_ sessions::SessionGuards::default(), ); let auth = auth_context(None, true); - let task = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - let session_id = session.session_id.clone(); - let auth = auth.clone(); - async move { - runtime - .dispatch_with_auth(process_call(project, Some(session_id)), Some(&auth)) - .await - } - }); - let request = next_patch_agent_request(&runtime, "process-session-job") - .await - .expect("Session-bound process Job"); + let (task, request) = dispatch_process_until_request( + &runtime, + "process-session-job", + process_call(project.clone(), Some(session.session_id.clone())), + auth.clone(), + ) + .await; update_process_job( &runtime, "process-session-job", @@ -1527,9 +1434,7 @@ async fn b2_process_runner_uses_direct_sync_and_rejects_durable_only_timeout() { .await } }); - let request = next_patch_agent_request(&runtime, "process-b2") - .await - .expect("B2 direct request"); + let request = wait_for_patch_agent_request(&runtime, "process-b2").await; assert_eq!(request.kind, "run_process"); complete_process_lifecycle( &runtime, @@ -1625,9 +1530,7 @@ async fn run_process_preserves_large_typed_argv_without_shell_parsing() { } }); - let request = next_patch_agent_request(&runtime, "process-large") - .await - .expect("large structured argv should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "process-large").await; assert_eq!(request.command, ""); let process = request.process.as_ref().unwrap(); assert_eq!(process.args, large_args); @@ -1688,9 +1591,7 @@ async fn run_process_batch_rejection_from_runner_has_stable_prestart_contract() .await } }); - let request = next_patch_agent_request(&runtime, "process-batch-rejected") - .await - .expect("run_process should reach the capable Runner"); + let request = wait_for_patch_agent_request(&runtime, "process-batch-rejected").await; complete_process_lifecycle( &runtime, "process-batch-rejected", @@ -1756,9 +1657,7 @@ async fn run_process_transport_uncertainty_and_timeout_preserve_phase_a_truth() .await } }); - next_patch_agent_request(&runtime, "process-lifecycle") - .await - .expect("run_process should dispatch before transport loss"); + wait_for_patch_agent_request(&runtime, "process-lifecycle").await; runtime .shell_clients .reconcile_disconnect("process-lifecycle", "inst") @@ -1794,9 +1693,7 @@ async fn run_process_transport_uncertainty_and_timeout_preserve_phase_a_truth() .await } }); - let request = next_patch_agent_request(&timeout_runtime, "process-timeout") - .await - .expect("run_process timeout should dispatch"); + let request = wait_for_patch_agent_request(&timeout_runtime, "process-timeout").await; complete_process_lifecycle( &timeout_runtime, "process-timeout", @@ -1858,9 +1755,7 @@ async fn run_process_session_default_cwd_applies_without_default_shell() { } }); - let request = next_patch_agent_request(&runtime, "process-context") - .await - .expect("session run_process should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "process-context").await; assert_eq!( request.cwd.as_deref(), Some(frontend.to_string_lossy().as_ref()) @@ -2094,9 +1989,7 @@ async fn run_process_validation_and_inspect_permission_boundaries_fail_closed() .await } }); - let request = next_patch_agent_request(&runtime, "process-guards") - .await - .expect("inspect run_process should enqueue"); + let request = wait_for_patch_agent_request(&runtime, "process-guards").await; assert_eq!( request.sandbox.as_deref(), Some(crate::command_sandbox::INSPECT_SANDBOX_MODE) diff --git a/src/tool_runtime/tests/read_files.rs b/src/tool_runtime/tests/read_files.rs index 2c3fa13b..9299e66c 100644 --- a/src/tool_runtime/tests/read_files.rs +++ b/src/tool_runtime/tests/read_files.rs @@ -19,9 +19,7 @@ async fn next_read_request( runtime: &ToolRuntime, client_id: &str, ) -> crate::shell_protocol::ShellAgentShellRequest { - next_patch_agent_request(runtime, client_id) - .await - .expect("read_files should enqueue a file_read request") + wait_for_patch_agent_request(runtime, client_id).await } async fn complete_read( @@ -30,18 +28,7 @@ async fn complete_read( request: &crate::shell_protocol::ShellAgentShellRequest, content: &str, ) { - let start = request.start_line.unwrap_or(1); - let end = request.end_line.unwrap_or(start); - let limit = end.saturating_sub(start).saturating_add(1); - complete_patch_agent_request( - runtime, - client_id, - &request.request_id, - 0, - &canonical_agent_file_read_range(content, start, limit), - "", - ) - .await; + complete_agent_ranged_file_read_request(runtime, client_id, request, content).await; } #[test] diff --git a/src/tool_runtime/tests/support/agent.rs b/src/tool_runtime/tests/support/agent.rs index 4cee7045..4b71e191 100644 --- a/src/tool_runtime/tests/support/agent.rs +++ b/src/tool_runtime/tests/support/agent.rs @@ -11,6 +11,7 @@ use crate::workspace_checkpoint::{create_workspace_checkpoint, restore_workspace use serde_json::{json, Value}; use std::path::{Path, PathBuf}; use std::sync::Arc; +use std::time::{Duration, Instant}; pub(in crate::tool_runtime::tests) async fn register_agent_project_at_path( runtime: &ToolRuntime, @@ -375,14 +376,17 @@ pub(in crate::tool_runtime::tests) async fn dispatch_checkpoint_with_local_agent runtime.dispatch_with_auth(call, Some(&bootstrap)).await } }); - let mut req = None; - for _ in 0..200 { - req = next_patch_agent_request(runtime, client_id).await; - if req.is_some() || task.is_finished() { - break; + let deadline = Instant::now() + Duration::from_secs(10); + let req = loop { + let request = next_patch_agent_request(runtime, client_id).await; + if request.is_some() || task.is_finished() { + break request; } - tokio::time::sleep(std::time::Duration::from_millis(5)).await; - } + if Instant::now() >= deadline { + panic!("checkpoint Agent request readiness failed for client {client_id} within 10 seconds"); + } + tokio::time::sleep(Duration::from_millis(5)).await; + }; let req = match req { Some(req) => req, None => { @@ -641,6 +645,34 @@ pub(in crate::tool_runtime::tests) async fn next_agent_request_for_instance( None } +pub(in crate::tool_runtime::tests) async fn wait_for_agent_request_for_instance( + runtime: &ToolRuntime, + client_id: &str, + agent_instance_id: &str, +) -> ShellAgentShellRequest { + let deadline = Instant::now() + Duration::from_secs(10); + loop { + if let Some(request) = runtime + .shell_clients + .poll(ShellAgentPollRequest { + client_id: client_id.to_string(), + agent_instance_id: agent_instance_id.to_string(), + projects: None, + }) + .await + .unwrap() + { + return request; + } + if Instant::now() >= deadline { + panic!( + "Agent request readiness failed for client {client_id} instance {agent_instance_id} within 10 seconds" + ); + } + tokio::time::sleep(Duration::from_millis(5)).await; + } +} + pub(in crate::tool_runtime::tests) async fn runtime_with_resolver_projects() -> ToolRuntime { let runtime = test_runtime(); let file_caps = ShellClientCapabilities { @@ -721,6 +753,18 @@ pub(in crate::tool_runtime::tests) async fn next_patch_agent_request( None } +/// Wait for a request that the test requires to be dispatched. Unlike +/// `next_patch_agent_request`, which is intentionally a short probe for +/// negative/no-dispatch assertions, this positive readiness wait uses one +/// absolute wall-clock deadline so scheduler contention cannot turn a fixed +/// yield count into a flaky failure. +pub(in crate::tool_runtime::tests) async fn wait_for_patch_agent_request( + runtime: &ToolRuntime, + client_id: &str, +) -> ShellAgentShellRequest { + wait_for_agent_request_for_instance(runtime, client_id, "inst").await +} + pub(in crate::tool_runtime::tests) fn assert_internal_posix_script_contains( request: &ShellAgentShellRequest, needle: &str, @@ -783,6 +827,30 @@ pub(in crate::tool_runtime::tests) async fn complete_patch_agent_request_for_ins .unwrap(); } +pub(in crate::tool_runtime::tests) async fn complete_agent_ranged_file_read_request( + runtime: &ToolRuntime, + client_id: &str, + request: &ShellAgentShellRequest, + content: &str, +) { + let start = request + .start_line + .expect("ToolRuntime file_read test request must include start_line"); + let end = request + .end_line + .expect("ToolRuntime file_read test request must include end_line"); + let limit = end.saturating_sub(start).saturating_add(1); + complete_patch_agent_request( + runtime, + client_id, + &request.request_id, + 0, + &canonical_agent_file_read_range(content, start, limit), + "", + ) + .await; +} + pub(in crate::tool_runtime::tests) async fn register_agent_with_projects( runtime: &ToolRuntime, client_id: &str, diff --git a/src/tool_runtime/tests/support/files.rs b/src/tool_runtime/tests/support/files.rs index a71ea504..c9640440 100644 --- a/src/tool_runtime/tests/support/files.rs +++ b/src/tool_runtime/tests/support/files.rs @@ -5,7 +5,8 @@ use crate::tool_runtime::git::{ }; use crate::tool_runtime::helpers::{run_command_sync, shell_escape_simple}; use crate::tool_runtime::{ - ApplyFileChangeInput, ApplyFileChangeKind, ApplyTextEditInput, ApplyTextEditKind, ToolRuntime, + ApplyFileChangeInput, ApplyFileChangeKind, ApplyTextEditInput, ApplyTextEditKind, + LocalJobRecord, ToolRuntime, }; use crate::tool_runtime::{LocalJobKiller, TerminateOutcome}; use serde_json::{json, Value}; @@ -133,6 +134,43 @@ pub(in crate::tool_runtime::tests) fn write_fake_job( dir } +/// Seed one session-scoped local Job directly into the runtime for model-facing +/// handoff/finish projection tests. Execution/transport semantics belong to the +/// Job/process suites; projection tests only need durable lifecycle metadata. +pub(in crate::tool_runtime::tests) async fn seed_session_projection_job( + runtime: &ToolRuntime, + root: &Path, + job_id: &str, + project: &str, + session_id: &str, + status: &str, + stdout: &str, +) { + let now = chrono::Utc::now().timestamp(); + let dir = write_fake_job( + root, + job_id, + project, + &root.to_string_lossy(), + status, + stdout, + "", + json!({ + "created_at": now, + "started_at": now, + "max_runtime_secs": 3600, + "session_id": session_id, + "purpose": "test", + }), + ); + let (record, _) = LocalJobRecord::initialize(project.to_string(), dir).unwrap(); + runtime + .local_jobs + .lock() + .await + .insert(job_id.to_string(), record); +} + /// A deterministic fake process-killer for testing timeout/stop invariants. /// Records which (pid, pgid) pairs it was asked to terminate and reports /// AlreadyGone so the runtime persists a terminal status without touching diff --git a/src/tool_runtime/tests/sync_timeout.rs b/src/tool_runtime/tests/sync_timeout.rs index d2af9196..fd6e6d04 100644 --- a/src/tool_runtime/tests/sync_timeout.rs +++ b/src/tool_runtime/tests/sync_timeout.rs @@ -89,75 +89,34 @@ fn structured_validation_sync_grace_is_sixty_seconds() { } #[tokio::test] -async fn cargo_validation_tools_accept_long_total_runtime_budget() { - // Read-only validation tools accept a long total runtime budget (1..=3600); - // they are no longer limited to the 120s synchronous cap. - let runtime = runtime_with_agent_project("sync-timeout-cargo-long") +async fn cargo_fmt_check_accepts_long_total_runtime_budget_and_hands_off() { + // cargo_check and cargo_test long-budget promotion lifecycles are owned by + // validation_handoff.rs. Keep only cargo_fmt(check=true)'s distinct branch. + let client_id = "sync-timeout-cargo-fmt-long"; + let runtime = runtime_with_agent_project(client_id) .with_validation_sync_wait(std::time::Duration::from_millis(10)); let caps = ShellClientCapabilities { async_shell_jobs: true, structured_validation_argv: true, ..Default::default() }; - register_agent(&runtime, "sync-timeout-cargo-long", None, caps).await; - let project = agent_test_project_id("sync-timeout-cargo-long"); - for (tool_name, timeout) in [ - ("cargo_check", 300u64), - ("cargo_test", 1800), - ("cargo_fmt", 300), - ] { - let result = match tool_name { - "cargo_check" => { - runtime - .cargo_check( - project.clone(), - None, - None, - None, - None, - None, - None, - Some(timeout), - ) - .await - } - "cargo_test" => { - runtime - .cargo_test( - project.clone(), - None, - None, - None, - None, - None, - None, - None, - None, - Some(timeout), - ) - .await - } - "cargo_fmt" => { - runtime - .cargo_fmt(project.clone(), None, Some(true), Some(timeout)) - .await - } - _ => unreachable!(), - }; - // These values are within 1..=3600, so the request is accepted and the - // validation is promoted to a Job (the wait window is tiny in tests). - assert!( - result.success, - "{tool_name} long budget should be accepted: {:?}", - result.error - ); - assert!(result.output["promoted_to_job"].as_bool().unwrap_or(false)); - assert_eq!(result.output["effective_timeout_secs"], timeout); - // The promoted Job is immediately queryable. - let job_id = result.output["job_id"].as_str().unwrap().to_string(); - let status = runtime.job_status_for_auth(job_id, false, None).await; - assert!(status.success, "{:?}", status.error); - } + register_agent(&runtime, client_id, None, caps).await; + let project = agent_test_project_id(client_id); + let timeout = 300u64; + + let result = runtime + .cargo_fmt(project, None, Some(true), Some(timeout)) + .await; + assert!( + result.success, + "cargo_fmt(check=true) long budget should be accepted: {:?}", + result.error + ); + assert!(result.output["promoted_to_job"].as_bool().unwrap_or(false)); + assert_eq!(result.output["effective_timeout_secs"], timeout); + let job_id = result.output["job_id"].as_str().unwrap().to_string(); + let status = runtime.job_status_for_auth(job_id, false, None).await; + assert!(status.success, "{:?}", status.error); } #[tokio::test] @@ -223,7 +182,7 @@ async fn cargo_validation_tools_reject_timeout_outside_1_3600() { } #[tokio::test] -async fn cargo_fmt_mutating_timeout_stays_within_120_seconds() { +async fn cargo_fmt_mutating_rejects_timeout_above_120_before_enqueue() { let client_id = "sync-timeout-fmt-mutating"; let runtime = runtime_with_agent_project(client_id); register_agent( @@ -235,30 +194,13 @@ async fn cargo_fmt_mutating_timeout_stays_within_120_seconds() { .await; let project = agent_test_project_id(client_id); - let accepted = tokio::spawn({ - let runtime = runtime.clone(); - let project = project.clone(); - async move { - runtime - .cargo_fmt(project, None, Some(false), Some(120)) - .await - } - }); - let request = next_patch_agent_request(&runtime, client_id) - .await - .expect("cargo_fmt(check=false, timeout=120) should start"); - assert_ne!(request.kind, "start_validation_job"); - complete_patch_agent_request(&runtime, client_id, &request.request_id, 0, "", "").await; - let accepted = accepted.await.unwrap(); - assert!(accepted.success, "{:?}", accepted.error); - assert_eq!(accepted.output["promoted_to_job"], false); - + // The successful 120-second synchronous lifecycle is owned by + // validation_handoff::cargo_fmt_mutating_never_auto_promotes. for check in [Some(false), None] { let rejected = runtime .cargo_fmt(project.clone(), None, check, Some(121)) .await; - assert!(!rejected.success); - assert_eq!(rejected.output["failure_kind"], "invalid_arguments"); + assert_timeout_rejected(&rejected, "cargo_fmt"); assert_no_pending_shell_request(&runtime, client_id).await; } } diff --git a/src/tool_runtime/tests/validation_handoff.rs b/src/tool_runtime/tests/validation_handoff.rs index c17b0cd1..a95b49fb 100644 --- a/src/tool_runtime/tests/validation_handoff.rs +++ b/src/tool_runtime/tests/validation_handoff.rs @@ -39,13 +39,16 @@ async fn wait_for_agent_request( runtime: &ToolRuntime, client_id: &str, ) -> crate::shell_protocol::ShellAgentShellRequest { - for _ in 0..200 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if let Some(request) = next_patch_agent_request(runtime, client_id).await { return request; } - tokio::time::sleep(std::time::Duration::from_millis(5)).await; + if tokio::time::Instant::now() >= deadline { + panic!("agent request was not enqueued within 10 seconds for {client_id}"); + } + tokio::time::sleep(std::time::Duration::from_millis(10)).await; } - panic!("agent request was not enqueued for {client_id}"); } fn assert_agent_observation_upgrades_without_changing_snapshot( @@ -173,14 +176,23 @@ async fn wait_for_local_job_terminal( runtime: &ToolRuntime, job_id: &str, ) -> crate::tool_runtime::ToolResult { - for _ in 0..500 { + let deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(15); + loop { let status = runtime.job_status(job_id.to_string()).await; if status.success && status.output["terminal"].as_bool().unwrap_or(false) { return status; } + let last_observation = format!( + "success={} status={} error={:?}", + status.success, status.output["status"], status.error + ); + if tokio::time::Instant::now() >= deadline { + panic!( + "local validation job did not become terminal within 15 seconds: {job_id}; last observation: {last_observation}" + ); + } tokio::time::sleep(std::time::Duration::from_millis(20)).await; } - panic!("local validation job did not become terminal: {job_id}"); } #[cfg(unix)] @@ -1895,6 +1907,7 @@ async fn cancel_queued_before_handoff_removes_start_request_and_hidden_record() .await } }); + let registration_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); let job_id = loop { if let Some(job_id) = runtime .shell_clients @@ -1905,11 +1918,15 @@ async fn cancel_queued_before_handoff_removes_start_request_and_hidden_record() { break job_id; } + if tokio::time::Instant::now() >= registration_deadline { + panic!("hidden validation job was not registered within 10 seconds for {client_id}"); + } tokio::task::yield_now().await; }; task.abort(); let _ = task.await; - for _ in 0..100 { + let cleanup_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if runtime .shell_clients .hidden_job_ids_for_test() @@ -1918,6 +1935,11 @@ async fn cancel_queued_before_handoff_removes_start_request_and_hidden_record() { break; } + if tokio::time::Instant::now() >= cleanup_deadline { + panic!( + "hidden validation job {job_id} was not removed within 10 seconds after cancellation for {client_id}" + ); + } tokio::task::yield_now().await; } assert!(runtime @@ -2801,10 +2823,16 @@ mod tests { assert_eq!(handoff.output["promoted_to_job"], true); let job_id = handoff.output["job_id"].as_str().unwrap().to_string(); + let child_pid_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(10); let child_pid = loop { if let Ok(value) = std::fs::read_to_string(tmp.path().join("child.pid")) { break value.trim().parse::().unwrap(); } + if tokio::time::Instant::now() >= child_pid_deadline { + panic!( + "descendant fixture did not publish child.pid within 10 seconds for local validation job {job_id}" + ); + } tokio::time::sleep(std::time::Duration::from_millis(20)).await; }; let stopped = runtime.stop_job(job_id.clone()).await; @@ -2817,15 +2845,13 @@ mod tests { .and_then(|stat| stat.split_whitespace().nth(2).map(str::to_string)) .is_some_and(|state| state != "Z" && state != "X") }; - for _ in 0..200 { - if !descendant_is_running() { - break; - } + let reap_deadline = tokio::time::Instant::now() + std::time::Duration::from_secs(5); + while descendant_is_running() && tokio::time::Instant::now() < reap_deadline { tokio::time::sleep(std::time::Duration::from_millis(10)).await; } assert!( !descendant_is_running(), - "descendant process remained executable after stop_job" + "descendant process {child_pid} remained executable after stop_job for {job_id}" ); } diff --git a/src/tool_runtime/tests/work_on_project.rs b/src/tool_runtime/tests/work_on_project.rs index 8659efaf..2cf7d272 100644 --- a/src/tool_runtime/tests/work_on_project.rs +++ b/src/tool_runtime/tests/work_on_project.rs @@ -153,13 +153,18 @@ async fn dispatch_recording_startup_requests( .await } }); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let mut request_kinds = Vec::new(); - for _ in 0..800 { + loop { if task.is_finished() { break; } + assert!( + std::time::Instant::now() < deadline, + "coding startup did not finish within 10 seconds; serviced requests: {request_kinds:?}" + ); let Some(request) = next_patch_agent_request(runtime, client_id).await else { - tokio::time::sleep(std::time::Duration::from_millis(2)).await; + tokio::time::sleep(std::time::Duration::from_millis(5)).await; continue; }; request_kinds.push(request.kind.clone()); @@ -185,10 +190,6 @@ async fn dispatch_recording_startup_requests( complete_agent_request_by_running_locally(runtime, client_id, request).await; } } - assert!( - task.is_finished(), - "coding startup did not finish after servicing Runner requests: {request_kinds:?}" - ); (task.await.unwrap(), request_kinds) } @@ -216,11 +217,16 @@ async fn dispatch_startup_without_window( .await } }); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); while !task.is_finished() { + assert!( + std::time::Instant::now() < deadline, + "coding startup without window did not finish within 10 seconds for client {client_id}" + ); if let Some(request) = next_patch_agent_request(runtime, client_id).await { complete_agent_request_by_running_locally(runtime, client_id, request).await; } else { - tokio::time::sleep(std::time::Duration::from_millis(2)).await; + tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } task.await.unwrap() @@ -240,10 +246,15 @@ async fn dispatch_with_path_runner( let auth = auth_context(None, true); async move { runtime.dispatch_with_auth(call, Some(&auth)).await } }); - for _ in 0..400 { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if task.is_finished() { break; } + assert!( + std::time::Instant::now() < deadline, + "path-based coding call did not finish within 10 seconds for client {client_id}" + ); if let Some(request) = next_patch_agent_request(runtime, client_id).await { if request.kind == "resolve_or_register_project" { let payload: Value = @@ -283,10 +294,6 @@ async fn dispatch_with_path_runner( tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } - assert!( - task.is_finished(), - "path-based coding call did not finish after servicing Runner requests" - ); task.await.unwrap() } @@ -1490,10 +1497,15 @@ async fn path_source_cross_project_recording_session_reports_resolved_mismatch() } }); - for _ in 0..500 { + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); + loop { if task.is_finished() { break; } + assert!( + std::time::Instant::now() < deadline, + "kernel path bootstrap did not finish within 10 seconds for client {target_client}" + ); if let Some(request) = next_patch_agent_request(&runtime, target_client).await { if request.kind == "resolve_or_register_project" { let payload: Value = @@ -1533,7 +1545,6 @@ async fn path_source_cross_project_recording_session_reports_resolved_mismatch() tokio::time::sleep(std::time::Duration::from_millis(5)).await; } } - assert!(task.is_finished(), "kernel path bootstrap did not finish"); let outcome = task.await.unwrap(); assert!(outcome.success); let result = outcome.result.expect("work_on_project result"); @@ -2754,10 +2765,15 @@ async fn start_coding_task_standard_repository_overview_timeout_is_nonblocking() }); // Service the git/instruction probes but never the overview request. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let mut overview_request = None; while !task.is_finished() { + assert!( + std::time::Instant::now() < deadline, + "overview-timeout startup did not finish within 10 seconds" + ); let Some(request) = next_patch_agent_request(&runtime, "wop-timeout").await else { - tokio::time::sleep(std::time::Duration::from_millis(2)).await; + tokio::time::sleep(std::time::Duration::from_millis(5)).await; continue; }; if request.kind == "file_project_overview" { @@ -2855,10 +2871,15 @@ async fn dispatch_start_coding_task_with_overview_stdout( } }); + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(10); let mut overview_request_id = None; while !task.is_finished() { + assert!( + std::time::Instant::now() < deadline, + "overview startup did not finish within 10 seconds for client {client_id}" + ); let Some(request) = next_patch_agent_request(runtime, client_id).await else { - tokio::time::sleep(std::time::Duration::from_millis(2)).await; + tokio::time::sleep(std::time::Duration::from_millis(5)).await; continue; }; if request.kind == "file_project_overview" {