From 96e5ceb4fb581da8bd3e6deb1460ef9abe5fff2d Mon Sep 17 00:00:00 2001 From: Hare Date: Mon, 12 Oct 2026 03:20:02 +0900 Subject: [PATCH] fix(ticket): replay Assign receipts before new work constraints --- crates/workspace-server/src/server.rs | 23 +-- .../src/server_assignment_tracking_tests.rs | 157 ++++++++++++++++++ crates/workspace-server/src/store.rs | 16 ++ ...26-10-11-subworker-advisory-surface-gap.md | 8 + 4 files changed, 187 insertions(+), 17 deletions(-) diff --git a/crates/workspace-server/src/server.rs b/crates/workspace-server/src/server.rs index 81249d6d7..402c3c24c 100644 --- a/crates/workspace-server/src/server.rs +++ b/crates/workspace-server/src/server.rs @@ -13853,21 +13853,6 @@ async fn apply_set_ticket_assignment( }; let assignment = match role { TicketAssignmentRole::Orchestrator => { - if api - .store - .get_active_ticket_role_assignment( - &workspace_id, - &ticket.id, - TicketAssignmentRole::Worker, - )? - .is_some() - { - return Err(Error::TicketAssignmentConflict( - "Orchestrator assignment conflicts with an active Worker assignment" - .to_string(), - ) - .into()); - } if let Some(source) = source.as_ref() { api.store.set_current_ticket_orchestrator_assignment( &record, @@ -48321,8 +48306,10 @@ mod tests { input.workflow_state = Some(TicketState::Open); set_test_ticket_target(&mut input, "test-repository", "develop"); let ticket = browser_ticket_backend(&api).unwrap().create(input).unwrap(); - let old_coder = seed_live_test_coder_assignment(&api, &ticket.id, "old-coder").await; + // The retained delegation precedes actual Coder work. A new Assign + // during that work would be a different operation and must be refused. assign_test_orchestrator(&api, &ticket.id); + let old_coder = seed_live_test_coder_assignment(&api, &ticket.id, "old-coder").await; let old_orchestrator_id = format!("orchestrator-{}", ticket.id); let Json(started) = scoped_start_workspace_orchestrator( State(api.clone()), @@ -49147,6 +49134,9 @@ mod tests { assigned_by: "test-user".to_string(), assigned_at: TEST_CREATED_AT.to_string(), }; + // Seed an existing delegation before starting the Worker; both may + // remain current, but a fresh delegation during Worker work is guarded. + assign_test_orchestrator(&api, &ticket_id); api.store .set_current_ticket_worker_assignment( &assignment, @@ -49156,7 +49146,6 @@ mod tests { false, ) .unwrap(); - assign_test_orchestrator(&api, &ticket_id); let path = || ScopedRecordPath { workspace_id: TEST_WORKSPACE_ID.to_string(), id: ticket_id.clone(), diff --git a/crates/workspace-server/src/server_assignment_tracking_tests.rs b/crates/workspace-server/src/server_assignment_tracking_tests.rs index 69cc88c88..138fae02b 100644 --- a/crates/workspace-server/src/server_assignment_tracking_tests.rs +++ b/crates/workspace-server/src/server_assignment_tracking_tests.rs @@ -1,5 +1,162 @@ use super::*; +#[tokio::test] +async fn runtime_and_account_assign_retries_return_saved_result_after_clear_and_coder_start() { + for runtime_source in [true, false] { + let directory = tempfile::tempdir().unwrap(); + let (api, _, identity, source, ticket) = tracking_fixture(directory.path()).await; + let token = seed_test_api_token(api.store.as_ref(), "assign-replay-owner"); + let request = |method: &str, path: &str, body: Vec| { + if runtime_source { + runtime_source_request(&identity, Some(&source.worker_id), method, path, body) + } else { + Request::builder() + .method(method) + .uri(path) + .header(CONTENT_TYPE, "application/json") + .header("authorization", format!("Bearer {token}")) + .body(Body::from(body)) + .unwrap() + } + }; + let app = build_router(api.clone()); + let path = format!("/api/w/{TEST_WORKSPACE_ID}/tickets/{ticket}/assignments/orchestrator"); + let input = |key: &str| { + serde_json::to_vec(&serde_json::json!({ + "operation_id": key, + "principal": {"kind":"workspace_agent","agent_key":"workspace-orchestrator"} + })) + .unwrap() + }; + let saved = tracking_response( + app.clone(), + request("PUT", &path, input("saved-assign")), + StatusCode::OK, + ) + .await; + let assignment = saved["assignment"]["assignment_id"].as_str().unwrap(); + tracking_response( + app.clone(), + request( + "DELETE", + &assignment_clear_path(&ticket, assignment, "clear-before-coder"), + Vec::new(), + ), + StatusCode::OK, + ) + .await; + let Json(candidate) = create_workspace_worker( + State(api.clone()), + HeaderMap::new(), + Json(CreateWorkspaceWorkerRequest { + runtime_id: source.runtime_id.clone(), + display_name: "Coder candidate".into(), + singleton_key: None, + profile: Some("builtin:coder".into()), + ticket_assignment: None, + initial_submit: Vec::new(), + workdir_attachments: Vec::new(), + feature_connections: Default::default(), + control_operation_id: None, + }), + ) + .await + .unwrap(); + let worker_path = format!("/api/w/{TEST_WORKSPACE_ID}/tickets/{ticket}/assignments/worker"); + let worker_input = serde_json::to_vec(&serde_json::json!({ + "operation_id":"start-coder", "principal":{"kind":"worker", "runtime_id":candidate.runtime_id, "worker_id":candidate.worker_id} + })).unwrap(); + tracking_response( + app.clone(), + request("PUT", &worker_path, worker_input), + StatusCode::OK, + ) + .await; + let active = api + .store + .get_active_ticket_role_assignment( + TEST_WORKSPACE_ID, + &ticket, + TicketAssignmentRole::Worker, + ) + .unwrap() + .unwrap(); + let retained = api + .store + .get_ticket_assignment_tracking(TEST_WORKSPACE_ID, &ticket, assignment) + .unwrap() + .unwrap(); + assert!(!retained.current); + let counts = || { + let conn = rusqlite::Connection::open(&api.config.database_path).unwrap(); + [ + "ticket_worker_assignments", + "ticket_current_worker_assignments", + "ticket_worker_assignment_events", + "ticket_assignment_operations", + "typed_ticket_assignment_tracking_history", + "typed_ticket_assignment_tracking_operations", + "ticket_assignment_work_releases", + ] + .map(|table| { + conn.query_row( + &format!("SELECT count(*) FROM {table} WHERE workspace_id=?1"), + [TEST_WORKSPACE_ID], + |row| row.get::<_, i64>(0), + ) + .unwrap() + }) + }; + let before = counts(); + assert_eq!( + tracking_response( + app.clone(), + request("PUT", &path, input("saved-assign")), + StatusCode::OK + ) + .await, + saved + ); + // Only a genuinely new operation is refused; a receipt never restores + // the cleared current pointer, initializes tracking, or starts work. + tracking_response( + app, + request("PUT", &path, input("new-assign")), + StatusCode::CONFLICT, + ) + .await; + assert_eq!(counts(), before); + assert_eq!( + api.store + .get_ticket_assignment_tracking(TEST_WORKSPACE_ID, &ticket, assignment) + .unwrap() + .unwrap(), + retained + ); + assert_eq!( + api.store + .get_active_ticket_role_assignment( + TEST_WORKSPACE_ID, + &ticket, + TicketAssignmentRole::Worker + ) + .unwrap() + .unwrap(), + active + ); + assert!( + api.store + .get_current_ticket_role_assignment( + TEST_WORKSPACE_ID, + &ticket, + TicketAssignmentRole::Orchestrator + ) + .unwrap() + .is_none() + ); + } +} + fn tracking_path(ticket: &str) -> String { format!("/workspaces/{TEST_WORKSPACE_ID}/tickets/{ticket}/assignment-tracking") } diff --git a/crates/workspace-server/src/store.rs b/crates/workspace-server/src/store.rs index 458a860b3..c88faeec8 100644 --- a/crates/workspace-server/src/store.rs +++ b/crates/workspace-server/src/store.rs @@ -11456,6 +11456,22 @@ impl SqliteWorkspaceStore { return Ok(persisted); } + // This is a constraint on a new delegation, not on a saved result. + // Authenticate the source and resolve its full-input receipt first; + // work that began after the original Assign must not block replay. + if record.role == TicketAssignmentRole::Orchestrator { + let active_worker: bool = tx.query_row( + "SELECT EXISTS(SELECT 1 FROM ticket_active_worker_assignments + WHERE workspace_id=?1 AND ticket_id=?2 AND role='worker')", + params![record.workspace_id, record.ticket_id], + |row| row.get(0), + )?; + if active_worker { + return Err(Error::TicketAssignmentConflict( + "Orchestrator assignment conflicts with an active Worker assignment".into(), + )); + } + } ensure_no_competing_ticket_claim(&tx, &record.workspace_id, &record.ticket_id, record.principal.worker().as_ref(), operation_id)?; let current_sql = ticket_role_assignment_select_sql( diff --git a/docs/report/2026-10-11-subworker-advisory-surface-gap.md b/docs/report/2026-10-11-subworker-advisory-surface-gap.md index 4f68f1c84..bbfa5e815 100644 --- a/docs/report/2026-10-11-subworker-advisory-surface-gap.md +++ b/docs/report/2026-10-11-subworker-advisory-surface-gap.md @@ -13,3 +13,11 @@ T-741のdomain/store担当とserver認可/router担当を並行委任した。Wo - 付与しない場合は、短い契約確定チェックポイントを分担の最初に設けることが現状の回避策になる。これは一般的な自動停止・polling・再起動の許可ではない。 本記録はこのSessionのtool提供面の観測であり、Backendの全環境に同機能がないと断定するものではない。 + +## Trusted Reviewer起動時の前提 + +同じSessionで、`builtin:reviewer`と明示MRのreview bindingを指定し、codeをread-only委任して起動を試みたところ、Backendから「writable delegated scopeが必要」、続いて「explicit command grantが必要」と引数検証で拒否された。Tool説明のread-only Reviewerという記述だけでは、この起動前提を予測できなかった。 + +最終的に、code全体のread scopeに加えて専用の空scratch `.review-t741` だけをwrite委任し、commandを明示grantして起動できた。source・Git metadataへのwriteを委任せず、Reviewerの調査・一時出力をscratchに限定した。拒否されたreview authorityを別経路で代替したものではなく、trusted review bindingはBackendが生成してMRに記録した。 + +必要なwrite/commandとcode read-onlyの両立例を、起動前のTool説明に示すと不要な引数試行を避けられる。また、Reviewerの最初の判定提出はseverity `medium`がHTTP 422で拒否され、許容値が`blocker/major/minor/note`と判明した。未記録をMR正本で確認し、新しいreview turnで適切なseverityによる独立判定を提出した。Role側の提出回数制約と引数validation failure後の返却・再開方法も、許容値とともに明示すると扱いやすい。