Hare/yoi
1
1
Fork 0
Code Actions Packages Releases Activity

Merge ticket 00001KZYAEWJT target and MergeResult authority

Browse Source 
# Conflicts:
#	crates/merge-request/src/lib.rs
#	crates/merge-request/tests/store.rs
This commit is contained in:
Hare
2026-08-16 18:57:41 +09:00
parent 4f042cae84 aab7346ee4
commit c878bc44fb
9 changed files with 2468 additions and 506 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/merge-request/src/lib.rs
+1501 -338
View File
File diff suppressed because it is too large Load Diff
crates/merge-request/tests/store.rs
+219 -105
View File
@@ -44,7 +44,6 @@ fn revision(id: &str, ordinal: u64, head: &str) -> MergeRequestRevision {
ordinal,
base_commit: "base".into(),
head_commit: head.into(),
head_tree: format!("tree-{head}"),
diff_digest: format!("sha256:diff-{head}"),
changed_paths: vec!["src/lib.rs".into()],
summary: format!("revision {id}"),
@@ -58,6 +57,7 @@ fn open(store: &SqliteMergeRequestStore) {
merge_request_id: "MR1".into(),
ticket_id: "T1".into(),
repository_id: "repo".into(),
target_ref_selector: "develop".into(),
revision: revision("V1", 1, "h1"),
authenticated_runtime_id: "R1".into(),
authenticated_worker_id: "W1".into(),
@@ -79,6 +79,7 @@ fn attempt(store: &SqliteMergeRequestStore, id: &str, revision: &str, token: &st
attempt_id: id.into(),
ticket_id: "T1".into(),
revision_id: revision.into(),
merge_result_id: None,
parent_assignment_id: "A1".into(),
parent_runtime_id: "R1".into(),
parent_worker_id: "W1".into(),
@@ -97,6 +98,7 @@ fn review(
store.submit_review(SubmitReview {
ticket_id: "T1".into(),
revision_id: revision.into(),
merge_result_id: None,
capability_token: token.into(),
decision,
body: "evidence".into(),
@@ -105,6 +107,34 @@ fn review(
})
}
fn record_result(
store: &SqliteMergeRequestStore,
revision: &str,
target: &str,
source: &str,
result: &str,
strategy: MergeStrategy,
resolution: MergeResolution,
operation: &str,
) -> RecordMergeResultOutcome {
store
.record_merge_result(RecordMergeResult {
merge_result_id: format!("result-{operation}"),
ticket_id: "T1".into(),
expected_revision_id: revision.into(),
target_commit: target.into(),
source_commit: source.into(),
result_commit: result.into(),
strategy,
resolution,
operation_id: operation.into(),
actor_runtime_id: "runtime-orchestrator".into(),
actor_worker_id: "workspace-orchestrator".into(),
created_at: "2026-07-26T00:00:02Z".into(),
})
.unwrap()
}
#[test]
fn storage_allows_multiple_merge_requests_for_one_ticket() {
let (_dir, store) = setup();
@@ -114,6 +144,7 @@ fn storage_allows_multiple_merge_requests_for_one_ticket() {
merge_request_id: "MR2".into(),
ticket_id: "T1".into(),
repository_id: "repo".into(),
target_ref_selector: "develop".into(),
revision: revision("V2", 1, "h2"),
authenticated_runtime_id: "R1".into(),
authenticated_worker_id: "W1".into(),
@@ -133,6 +164,162 @@ fn storage_allows_multiple_merge_requests_for_one_ticket() {
);
}
#[test]
fn merge_result_is_idempotent_target_fenced_and_non_ff_reviewed_independently() {
let (_tmp, store) = setup();
open(&store);
attempt(
&store,
"attempt-source",
"V1",
"token-source",
"child-source",
);
review(&store, "V1", "token-source", ReviewDecision::Approve).unwrap();
let first = record_result(
&store,
"V1",
"T0",
"h1",
"h1",
MergeStrategy::FastForward,
MergeResolution::None,
"op-ff",
);
assert!(!first.replayed);
let replay = record_result(
&store,
"V1",
"T0",
"h1",
"h1",
MergeStrategy::FastForward,
MergeResolution::None,
"op-ff",
);
assert!(replay.replayed);
assert_eq!(
first.merge_result.merge_result_id,
replay.merge_result.merge_result_id
);
let ready = store
.readiness_for_ticket_with_target("T1", Some("T0"))
.unwrap();
assert!(ready.ready, "{:?}", ready.blockers);
assert_eq!(ready.merge_result_id.as_deref(), Some("result-op-ff"));
let stale = store
.readiness_for_ticket_with_target("T1", Some("T1"))
.unwrap();
assert!(!stale.ready);
assert!(
stale
.blockers
.iter()
.any(|blocker| blocker.contains("target moved"))
);
let changed = store.record_merge_result(RecordMergeResult {
merge_result_id: "result-conflict".into(),
ticket_id: "T1".into(),
expected_revision_id: "V1".into(),
target_commit: "different".into(),
source_commit: "h1".into(),
result_commit: "h1".into(),
strategy: MergeStrategy::FastForward,
resolution: MergeResolution::None,
operation_id: "op-ff".into(),
actor_runtime_id: "runtime-orchestrator".into(),
actor_worker_id: "workspace-orchestrator".into(),
created_at: "2026-07-26T00:00:03Z".into(),
});
assert!(matches!(
changed,
Err(MergeRequestError::MergeResultOperationConflict)
));
record_result(
&store,
"V1",
"T1",
"h1",
"M1",
MergeStrategy::Merge,
MergeResolution::ConflictsResolved,
"op-merge",
);
let pending = store
.readiness_for_ticket_with_target("T1", Some("T1"))
.unwrap();
assert!(!pending.ready);
assert_eq!(
pending.merge_result_review_status,
Some(ReviewStatus::Pending)
);
store
.register_reviewer_child_session(RegisterReviewerChildSession {
parent_runtime_id: "runtime-orchestrator".into(),
parent_worker_id: "workspace-orchestrator".into(),
child_session_id: "child-merge".into(),
now: "2026-07-26T00:00:04Z".into(),
})
.unwrap();
store
.register_review_attempt(RegisterReviewAttempt {
attempt_id: "attempt-merge".into(),
ticket_id: "T1".into(),
revision_id: "V1".into(),
merge_result_id: Some("result-op-merge".into()),
parent_assignment_id: "A1".into(),
parent_runtime_id: "runtime-orchestrator".into(),
parent_worker_id: "workspace-orchestrator".into(),
child_session_id: "child-merge".into(),
capability_token: "token-merge".into(),
now: "2026-07-26T00:00:04Z".into(),
})
.unwrap();
store
.submit_review(SubmitReview {
ticket_id: "T1".into(),
revision_id: "V1".into(),
merge_result_id: Some("result-op-merge".into()),
capability_token: "token-merge".into(),
decision: ReviewDecision::Approve,
body: "merge evidence is valid".into(),
findings: vec![],
now: "2026-07-26T00:00:05Z".into(),
})
.unwrap();
let approved = store
.readiness_for_ticket_with_target("T1", Some("T1"))
.unwrap();
assert!(approved.ready, "{:?}", approved.blockers);
assert_eq!(
approved.merge_result_review_status,
Some(ReviewStatus::Approved)
);
let applied = store
.show_for_ticket_with_target("T1", Some("M1"))
.unwrap()
.unwrap();
assert_eq!(
applied
.applied_merge_result
.as_ref()
.map(|result| result.target_status),
Some(MergeResultTargetStatus::Applied)
);
assert!(
store
.readiness_for_ticket_with_target("T1", Some("M1"))
.unwrap()
.ready
);
}
#[test]
fn bounded_context_rejects_oversized_revision_evidence() {
let (_dir, store) = setup();
@@ -142,6 +329,7 @@ fn bounded_context_rejects_oversized_revision_evidence() {
merge_request_id: "MR1".into(),
ticket_id: "T1".into(),
repository_id: "repo".into(),
target_ref_selector: "develop".into(),
revision: oversized,
authenticated_runtime_id: "R1".into(),
authenticated_worker_id: "W1".into(),
@@ -157,67 +345,24 @@ fn bounded_context_rejects_oversized_revision_evidence() {
}
#[test]
fn rejected_v6_schema_missing_diff_digest_is_archived_before_fresh_v7() {
fn v6_legacy_schema_fails_closed_without_archiving() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("legacy.db");
let conn = Connection::open(&path).unwrap();
conn.execute_batch(
"CREATE TABLE merge_request_schema_migrations(version INTEGER PRIMARY KEY,name TEXT NOT NULL,applied_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP);\
INSERT INTO merge_request_schema_migrations(version,name) VALUES(6,'rejected_merge_request_v6');\
CREATE TABLE repositories(workspace_id TEXT NOT NULL,repository_id TEXT NOT NULL,PRIMARY KEY(workspace_id,repository_id));\
CREATE TABLE typed_tickets(workspace_id TEXT NOT NULL,ticket_id TEXT NOT NULL,workflow_state TEXT NOT NULL,workflow_state_explicit INTEGER NOT NULL DEFAULT 1,updated_at TEXT NOT NULL,PRIMARY KEY(workspace_id,ticket_id));\
CREATE TABLE ticket_worker_assignments(workspace_id TEXT NOT NULL,ticket_id TEXT NOT NULL,assignment_id TEXT NOT NULL,runtime_id TEXT NOT NULL,worker_id TEXT NOT NULL,PRIMARY KEY(workspace_id,ticket_id,assignment_id));\
CREATE TABLE merge_requests(workspace_id TEXT NOT NULL,merge_request_id TEXT NOT NULL,repository_id TEXT NOT NULL,state TEXT NOT NULL,lifecycle_generation INTEGER NOT NULL,current_revision_id TEXT NOT NULL,created_at TEXT NOT NULL,updated_at TEXT NOT NULL,PRIMARY KEY(workspace_id,merge_request_id));\
CREATE TABLE merge_request_ticket_relations(workspace_id TEXT NOT NULL,merge_request_id TEXT NOT NULL,ticket_id TEXT NOT NULL,relation_kind TEXT NOT NULL,created_at TEXT NOT NULL,PRIMARY KEY(workspace_id,merge_request_id,ticket_id));\
CREATE TABLE merge_request_revisions(workspace_id TEXT NOT NULL,merge_request_id TEXT NOT NULL,revision_id TEXT NOT NULL,ordinal INTEGER NOT NULL,base_commit TEXT NOT NULL,head_commit TEXT NOT NULL,head_tree TEXT NOT NULL,assignment_id TEXT NOT NULL,created_at TEXT NOT NULL,PRIMARY KEY(workspace_id,merge_request_id,revision_id));",
CREATE TABLE merge_requests(workspace_id TEXT NOT NULL,merge_request_id TEXT NOT NULL,repository_id TEXT NOT NULL,state TEXT NOT NULL,lifecycle_generation INTEGER NOT NULL,current_revision_id TEXT NOT NULL,created_at TEXT NOT NULL,updated_at TEXT NOT NULL,PRIMARY KEY(workspace_id,merge_request_id));",
).unwrap();
drop(conn);
let store = SqliteMergeRequestStore::open(&path, "ws-a").unwrap();
assert!(store.show_for_ticket("missing").unwrap().is_none());
let conn = Connection::open(&path).unwrap();
let archived: i64 = conn.query_row("SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='legacy_v6_merge_requests'",[],|row|row.get(0)).unwrap();
assert_eq!(archived, 1);
for table in [
"merge_request_review_attempts",
"merge_request_completion_operations",
] {
let present: i64 = conn
.query_row(
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name=?1",
params![table],
|row| row.get(0),
)
.unwrap();
assert_eq!(present, 1);
}
}
#[test]
fn interrupted_legacy_archive_with_empty_recreated_table_resumes() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("interrupted.db");
let error = SqliteMergeRequestStore::open(&path, "ws-a").unwrap_err();
assert!(
error
.to_string()
.contains("unsupported legacy merge request schema version 6")
);
let conn = Connection::open(&path).unwrap();
conn.execute_batch(
"CREATE TABLE merge_request_schema_migrations(version INTEGER PRIMARY KEY,name TEXT NOT NULL,applied_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP);\
INSERT INTO merge_request_schema_migrations(version,name) VALUES(6,'rejected_merge_request_v6');\
CREATE TABLE merge_requests(workspace_id TEXT NOT NULL,merge_request_id TEXT NOT NULL,ticket_id TEXT NOT NULL);\
CREATE TABLE merge_request_review_findings(workspace_id TEXT NOT NULL,attempt_id TEXT NOT NULL,ordinal INTEGER NOT NULL,severity TEXT NOT NULL,code TEXT,path TEXT,line INTEGER,body TEXT NOT NULL);\
CREATE TABLE legacy_v6_merge_request_review_findings(workspace_id TEXT NOT NULL,attempt_id TEXT NOT NULL,ordinal INTEGER NOT NULL,severity TEXT NOT NULL,code TEXT,path TEXT,line INTEGER,body TEXT NOT NULL);\
INSERT INTO legacy_v6_merge_request_review_findings VALUES('ws-a','AT1',0,'warning',NULL,NULL,NULL,'preserved evidence');",
)
.unwrap();
drop(conn);
SqliteMergeRequestStore::open(&path, "ws-a").unwrap();
let conn = Connection::open(&path).unwrap();
let archived_body: String = conn
.query_row(
"SELECT body FROM legacy_v6_merge_request_review_findings",
[],
|row| row.get(0),
)
.unwrap();
assert_eq!(archived_body, "preserved evidence");
let version: i64 = conn
.query_row(
"SELECT MAX(version) FROM merge_request_schema_migrations",
@@ -225,59 +370,33 @@ fn interrupted_legacy_archive_with_empty_recreated_table_resumes() {
|row| row.get(0),
)
.unwrap();
assert_eq!(version, 8);
drop(conn);
SqliteMergeRequestStore::open(&path, "ws-a").unwrap();
}
#[test]
fn conflicting_legacy_archive_rolls_back_all_table_renames() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("conflict.db");
let conn = Connection::open(&path).unwrap();
conn.execute_batch(
"CREATE TABLE merge_request_schema_migrations(version INTEGER PRIMARY KEY,name TEXT NOT NULL,applied_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP);\
INSERT INTO merge_request_schema_migrations(version,name) VALUES(6,'rejected_merge_request_v6');\
CREATE TABLE merge_requests(workspace_id TEXT NOT NULL,merge_request_id TEXT NOT NULL,ticket_id TEXT NOT NULL);\
CREATE TABLE merge_request_review_findings(body TEXT NOT NULL);\
CREATE TABLE merge_request_reviews(body TEXT NOT NULL);\
INSERT INTO merge_request_reviews VALUES('unarchived evidence');\
CREATE TABLE legacy_v6_merge_request_reviews(body TEXT NOT NULL);",
)
.unwrap();
let error = migrate(&conn).unwrap_err();
assert!(error.to_string().contains(
"legacy archive table legacy_v6_merge_request_reviews already exists while merge_request_reviews still contains data"
));
let current_findings: i64 = conn
assert_eq!(version, 6);
let original: i64 = conn
.query_row(
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='merge_request_review_findings'",
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='merge_requests'",
[],
|row| row.get(0),
)
.unwrap();
let archived_findings: i64 = conn
let archived: i64 = conn
.query_row(
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='legacy_v6_merge_request_review_findings'",
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name LIKE 'legacy_v6_%'",
[],
|row| row.get(0),
)
.unwrap();
assert_eq!(current_findings, 1);
assert_eq!(archived_findings, 0);
assert_eq!(original, 1);
assert_eq!(archived, 0);
}
#[test]
fn v7_completion_operations_are_preserved_as_legacy_assigned_coder_authority() {
fn v7_schema_is_rejected_without_mutating_completion_evidence() {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("v7.db");
let conn = Connection::open(&path).unwrap();
conn.execute_batch(
"CREATE TABLE merge_request_schema_migrations(version INTEGER PRIMARY KEY,name TEXT NOT NULL,applied_at TEXT NOT NULL DEFAULT CURRENT_TIMESTAMP);\
INSERT INTO merge_request_schema_migrations(version,name) VALUES(7,'fresh_bounded_context_authority');\
CREATE TABLE repositories(workspace_id TEXT NOT NULL,repository_id TEXT NOT NULL,PRIMARY KEY(workspace_id,repository_id));\
CREATE TABLE typed_tickets(workspace_id TEXT NOT NULL,ticket_id TEXT NOT NULL,workflow_state TEXT NOT NULL,workflow_state_explicit INTEGER NOT NULL DEFAULT 1,updated_at TEXT NOT NULL,PRIMARY KEY(workspace_id,ticket_id));\
INSERT INTO typed_tickets VALUES('ws-a','T1','done',1,'t');\
CREATE TABLE merge_request_completion_operations(workspace_id TEXT NOT NULL,operation_id TEXT NOT NULL,ticket_id TEXT NOT NULL,revision_id TEXT NOT NULL,assignment_id TEXT NOT NULL,fingerprint TEXT NOT NULL,status TEXT NOT NULL CHECK(status IN ('pending','completed')),result_ticket_state TEXT,created_at TEXT NOT NULL,updated_at TEXT NOT NULL,PRIMARY KEY(workspace_id,operation_id),FOREIGN KEY(workspace_id,ticket_id) REFERENCES typed_tickets(workspace_id,ticket_id));\
@@ -285,25 +404,19 @@ fn v7_completion_operations_are_preserved_as_legacy_assigned_coder_authority() {
).unwrap();
drop(conn);
SqliteMergeRequestStore::open(&path, "ws-a").unwrap();
let conn = Connection::open(&path).unwrap();
let row: (String, String, Option<String>, Option<String>, String) = conn
.query_row(
"SELECT authority_kind,implementation_assignment_id,completion_actor_runtime_id,completion_actor_worker_id,fingerprint FROM merge_request_completion_operations WHERE operation_id='legacy-op'",
[],
|row| Ok((row.get(0)?, row.get(1)?, row.get(2)?, row.get(3)?, row.get(4)?)),
)
.unwrap();
assert_eq!(
row,
(
"legacy_assigned_coder".into(),
"A1".into(),
None,
None,
"legacy-fingerprint".into()
)
let error = SqliteMergeRequestStore::open(&path, "ws-a").unwrap_err();
assert!(
error
.to_string()
.contains("unsupported legacy merge request schema version 7")
);
let conn = Connection::open(&path).unwrap();
let row: (String, String) = conn.query_row(
"SELECT assignment_id,fingerprint FROM merge_request_completion_operations WHERE operation_id='legacy-op'",
[],
|row| Ok((row.get(0)?, row.get(1)?)),
).unwrap();
assert_eq!(row, ("A1".into(), "legacy-fingerprint".into()));
let version: i64 = conn
.query_row(
"SELECT MAX(version) FROM merge_request_schema_migrations",
@@ -311,7 +424,7 @@ fn v7_completion_operations_are_preserved_as_legacy_assigned_coder_authority() {
|row| row.get(0),
)
.unwrap();
assert_eq!(version, 8);
assert_eq!(version, 7);
}
#[test]
@@ -431,6 +544,7 @@ fn spoof_self_approval_replay_and_cross_workspace_are_rejected() {
attempt_id: "bad".into(),
ticket_id: "T1".into(),
revision_id: "V1".into(),
merge_result_id: None,
parent_assignment_id: "A1".into(),
parent_runtime_id: "R1".into(),
parent_worker_id: "W1".into(),
crates/worker/src/feature/builtin/merge_request.rs
+74 -4
View File
@@ -12,6 +12,7 @@ pub const MERGE_REQUEST_COMMON_TOOL_NAMES: &[&str] = &[
"MergeRequestReadinessCheck",
"MergeRequestOpen",
"MergeRequestAddRevision",
"MergeRequestRecordMergeResult",
"MergeRequestComplete",
];
pub const MERGE_REQUEST_REVIEW_TOOL_NAME: &str = "MergeRequestReviewSubmit";
@@ -21,6 +22,7 @@ enum Kind {
Readiness,
Open,
AddRevision,
RecordMergeResult,
Complete,
Review,
}
@@ -41,7 +43,6 @@ struct OpenInput {
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
@@ -55,7 +56,6 @@ struct AddRevisionInput {
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
@@ -63,6 +63,30 @@ struct AddRevisionInput {
summary: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct RecordMergeResultInput {
ticket: String,
expected_current_revision_id: String,
operation_id: String,
target_commit: String,
source_commit: String,
result_commit: String,
strategy: MergeStrategyInput,
resolution: MergeResolutionInput,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
enum MergeStrategyInput {
FastForward,
Merge,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
enum MergeResolutionInput {
None,
Clean,
ConflictsResolved,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct CompleteInput {
ticket: String,
operation_id: String,
@@ -101,6 +125,7 @@ impl Kind {
Self::Readiness => "MergeRequestReadinessCheck",
Self::Open => "MergeRequestOpen",
Self::AddRevision => "MergeRequestAddRevision",
Self::RecordMergeResult => "MergeRequestRecordMergeResult",
Self::Complete => "MergeRequestComplete",
Self::Review => "MergeRequestReviewSubmit",
}
@@ -113,6 +138,7 @@ impl Kind {
Self::Show | Self::Readiness => json!(schemars::schema_for!(ShowInput)),
Self::Open => json!(schemars::schema_for!(OpenInput)),
Self::AddRevision => json!(schemars::schema_for!(AddRevisionInput)),
Self::RecordMergeResult => json!(schemars::schema_for!(RecordMergeResultInput)),
Self::Complete => json!(schemars::schema_for!(CompleteInput)),
Self::Review => json!(schemars::schema_for!(ReviewInput)),
}
@@ -158,7 +184,7 @@ impl Tool for MergeRequestTool {
WorkspaceRequestMethod::Post,
format!("/api/w/{workspace_id}/tickets/{}/merge-request", v.ticket),
Some(
json!({"repository_id":v.repository_id,"revision_id":v.revision_id,"base_commit":v.base_commit,"head_commit":v.head_commit,"head_tree":v.head_tree,"diff_digest":v.diff_digest,"changed_paths":v.changed_paths,"summary":v.summary}),
json!({"repository_id":v.repository_id,"revision_id":v.revision_id,"base_commit":v.base_commit,"head_commit":v.head_commit,"diff_digest":v.diff_digest,"changed_paths":v.changed_paths,"summary":v.summary}),
),
)
}
@@ -172,7 +198,30 @@ impl Tool for MergeRequestTool {
v.ticket
),
Some(
json!({"expected_current_revision_id":v.expected_current_revision_id,"revision_id":v.revision_id,"base_commit":v.base_commit,"head_commit":v.head_commit,"head_tree":v.head_tree,"diff_digest":v.diff_digest,"changed_paths":v.changed_paths,"summary":v.summary}),
json!({"expected_current_revision_id":v.expected_current_revision_id,"revision_id":v.revision_id,"base_commit":v.base_commit,"head_commit":v.head_commit,"diff_digest":v.diff_digest,"changed_paths":v.changed_paths,"summary":v.summary}),
),
)
}
Kind::RecordMergeResult => {
let v: RecordMergeResultInput = parse(input)?;
nonempty(&v.ticket)?;
let strategy = match v.strategy {
MergeStrategyInput::FastForward => "fast_forward",
MergeStrategyInput::Merge => "merge",
};
let resolution = match v.resolution {
MergeResolutionInput::None => "none",
MergeResolutionInput::Clean => "clean",
MergeResolutionInput::ConflictsResolved => "conflicts_resolved",
};
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/merge-results",
v.ticket
),
Some(
json!({"expected_current_revision_id":v.expected_current_revision_id,"operation_id":v.operation_id,"target_commit":v.target_commit,"source_commit":v.source_commit,"result_commit":v.result_commit,"strategy":strategy,"resolution":resolution}),
),
)
}
@@ -261,6 +310,7 @@ pub fn common_tools(client: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
definition(client.clone(), Kind::Readiness),
definition(client.clone(), Kind::Open),
definition(client.clone(), Kind::AddRevision),
definition(client.clone(), Kind::RecordMergeResult),
definition(client, Kind::Complete),
]
}
@@ -288,6 +338,9 @@ pub fn description(name: &str) -> Option<&'static str> {
"MergeRequestAddRevision" => {
Some("Append an immutable revision; prior approval cannot carry to the new revision.")
}
"MergeRequestRecordMergeResult" => Some(
"Record validated immutable integration evidence for the current source revision and target tip.",
),
"MergeRequestComplete" => {
Some("CAS-complete an approved revision with operation-id replay and crash fencing.")
}
@@ -297,3 +350,20 @@ pub fn description(name: &str) -> Option<&'static str> {
_ => None,
}
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn merge_request_tool_contract_omits_tree_hashes_and_exposes_merge_result() {
let open = serde_json::to_string(&schemars::schema_for!(OpenInput)).unwrap();
let add = serde_json::to_string(&schemars::schema_for!(AddRevisionInput)).unwrap();
let result = serde_json::to_string(&schemars::schema_for!(RecordMergeResultInput)).unwrap();
assert!(!open.contains("head_tree"));
assert!(!add.contains("head_tree"));
assert!(result.contains("fast_forward"));
assert!(result.contains("conflicts_resolved"));
assert!(MERGE_REQUEST_COMMON_TOOL_NAMES.contains(&"MergeRequestRecordMergeResult"));
}
}
crates/worker/src/spawn/tool.rs
+12 -2
View File
@@ -68,6 +68,9 @@ struct SubWorkerSpawnInput {
struct ReviewerHandoffInput {
ticket_id: String,
revision_id: String,
/// Optional immutable MergeResult subject. Omit for source-revision review.
#[serde(default)]
merge_result_id: Option<String>,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -422,6 +425,7 @@ impl Tool for SubWorkerSpawnTool {
(
review.ticket_id.clone(),
review.revision_id.clone(),
review.merge_result_id.clone(),
uuid::Uuid::now_v7().to_string(),
format!(
"{}{}",
@@ -431,7 +435,9 @@ impl Tool for SubWorkerSpawnTool {
)
});
let child_workspace_context =
if let Some((ticket_id, revision_id, _, capability_token)) = &reviewer_attempt {
if let Some((ticket_id, revision_id, merge_result_id, _, capability_token)) =
&reviewer_attempt
{
let workspace_id =
self.workspace_context
.workspace_id()
@@ -453,6 +459,7 @@ impl Tool for SubWorkerSpawnTool {
ReviewerAttemptContext {
ticket_id: ticket_id.clone(),
revision_id: revision_id.clone(),
merge_result_id: merge_result_id.clone(),
},
capability_token.clone(),
));
@@ -547,7 +554,9 @@ impl Tool for SubWorkerSpawnTool {
}
};
if let Some((ticket_id, revision_id, attempt_id, capability_token)) = &reviewer_attempt {
if let Some((ticket_id, revision_id, merge_result_id, attempt_id, capability_token)) =
&reviewer_attempt
{
let workspace_id = self.workspace_context.workspace_id().ok_or_else(|| {
ToolError::ExecutionFailed("reviewer attempt lost Workspace identity".to_string())
})?;
@@ -579,6 +588,7 @@ impl Tool for SubWorkerSpawnTool {
let body = serde_json::json!({
"attempt_id": attempt_id,
"revision_id": revision_id,
"merge_result_id": merge_result_id,
"child_session_id": child_session_id,
"capability_token": capability_token,
});
crates/worker/src/worker.rs
+10
View File
@@ -249,6 +249,7 @@ pub trait WorkspaceClient: std::fmt::Debug + Send + Sync {
pub struct ReviewerAttemptContext {
pub ticket_id: String,
pub revision_id: String,
pub merge_result_id: Option<String>,
}
#[derive(Debug)]
@@ -310,6 +311,14 @@ impl WorkspaceClient for ReviewerChildWorkspaceClient {
"revision_id".to_string(),
serde_json::Value::String(self.context.revision_id.clone()),
);
object.insert(
"merge_result_id".to_string(),
self.context
.merge_result_id
.clone()
.map(serde_json::Value::String)
.unwrap_or(serde_json::Value::Null),
);
object.insert(
"capability_token".to_string(),
serde_json::Value::String(self.capability_token.clone()),
@@ -453,6 +462,7 @@ mod reviewer_client_tests {
ReviewerAttemptContext {
ticket_id: "T1".into(),
revision_id: "V1".into(),
merge_result_id: None,
},
"secret".into(),
);
crates/workspace-server/src/repositories.rs
+240
View File
@@ -83,10 +83,27 @@ pub struct GitCommitSummary {
pub refs: Vec<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct MergeTargetObservation {
pub selector: RepositorySelector,
pub commit: String,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct CommitObservation {
pub commit: String,
pub parents: Vec<String>,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum RepositoryLookupError {
UnknownRepository { id: RepositoryId },
UnsupportedProvider { id: RepositoryId, provider: String },
MissingDefaultSelector { id: RepositoryId },
InvalidSelector { id: RepositoryId, selector: String },
CommitNotFound { id: RepositoryId, commit: String },
InvalidCommitRelation { id: RepositoryId, detail: String },
ProviderFailure { id: RepositoryId, operation: String },
}
#[derive(Debug, Clone)]
@@ -167,6 +184,121 @@ impl RepositoryRegistryReader {
})
}
pub fn observe_merge_target(
&self,
id: &str,
requested_selector: Option<&str>,
) -> Result<MergeTargetObservation, RepositoryLookupError> {
let repository = self.merge_repository(id)?;
let selector = requested_selector
.map(str::trim)
.filter(|value| !value.is_empty())
.map(str::to_owned)
.or_else(|| repository.default_selector.clone())
.ok_or_else(|| RepositoryLookupError::MissingDefaultSelector { id: id.to_string() })?;
if selector.starts_with('-') || selector.as_bytes().contains(&0) {
return Err(RepositoryLookupError::InvalidSelector {
id: id.to_string(),
selector,
});
}
let spec = format!("{selector}^{{commit}}");
let commit = merge_git_stdout(
repository,
"resolve target",
&["rev-parse", "--verify", "--end-of-options", &spec],
)?
.lines()
.next()
.unwrap_or_default()
.trim()
.to_owned();
if commit.is_empty() {
return Err(RepositoryLookupError::InvalidSelector {
id: id.to_string(),
selector,
});
}
Ok(MergeTargetObservation { selector, commit })
}
pub fn observe_commit(
&self,
id: &str,
commit: &str,
) -> Result<CommitObservation, RepositoryLookupError> {
let repository = self.merge_repository(id)?;
let commit = commit.trim();
if commit.is_empty() || commit.starts_with('-') {
return Err(RepositoryLookupError::CommitNotFound {
id: id.to_string(),
commit: commit.into(),
});
}
let line = merge_git_stdout(
repository,
"read commit",
&[
"show",
"--no-patch",
"--format=%H %P",
"--end-of-options",
commit,
],
)?;
let mut parts = line.split_whitespace();
let canonical = parts.next().unwrap_or_default().to_owned();
if canonical.is_empty() {
return Err(RepositoryLookupError::CommitNotFound {
id: id.to_string(),
commit: commit.into(),
});
}
Ok(CommitObservation {
commit: canonical,
parents: parts.map(str::to_owned).collect(),
})
}
pub fn ensure_ancestor(
&self,
id: &str,
ancestor: &str,
descendant: &str,
) -> Result<(), RepositoryLookupError> {
let repository = self.merge_repository(id)?;
let status = Command::new("git")
.arg("-C")
.arg(&repository.path)
.args(["merge-base", "--is-ancestor", ancestor, descendant])
.status()
.map_err(|_| RepositoryLookupError::ProviderFailure {
id: id.into(),
operation: "check commit ancestry".into(),
})?;
if status.success() {
Ok(())
} else {
Err(RepositoryLookupError::InvalidCommitRelation {
id: id.into(),
detail: format!("commit {ancestor} is not an ancestor of {descendant}"),
})
}
}
fn merge_repository(&self, id: &str) -> Result<&ConfiguredRepository, RepositoryLookupError> {
let repository = self
.find(id)
.ok_or_else(|| RepositoryLookupError::UnknownRepository { id: id.into() })?;
if repository.provider != "git" {
return Err(RepositoryLookupError::UnsupportedProvider {
id: id.into(),
provider: repository.provider.clone(),
});
}
Ok(repository)
}
fn find(&self, id: &str) -> Option<&ConfiguredRepository> {
self.repositories
.iter()
@@ -256,6 +388,19 @@ impl RepositoryRegistryReader {
}
}
fn merge_git_stdout(
repository: &ConfiguredRepository,
operation: &str,
args: &[&str],
) -> Result<String, RepositoryLookupError> {
git_stdout(&repository.path, args.iter().copied()).map_err(|_| {
RepositoryLookupError::ProviderFailure {
id: repository.id.clone(),
operation: operation.into(),
}
})
}
fn git_stdout<'a, I>(repository_path: &PathBuf, args: I) -> Result<String, String>
where
I: IntoIterator<Item = &'a str>,
@@ -422,6 +567,101 @@ mod tests {
assert_eq!(projection.diagnostics[0].code, "repository_config_empty");
}
#[test]
fn merge_evidence_is_resolved_by_repository_identity() {
let temp = tempfile::tempdir().unwrap();
let path = temp.path();
assert!(
Command::new("git")
.args(["init", "-b", "main"])
.arg(path)
.status()
.unwrap()
.success()
);
for args in [
vec!["config", "user.email", "test@example.com"],
vec!["config", "user.name", "Test"],
] {
assert!(
Command::new("git")
.arg("-C")
.arg(path)
.args(args)
.status()
.unwrap()
.success()
);
}
std::fs::write(path.join("file.txt"), "base\n").unwrap();
assert!(
Command::new("git")
.arg("-C")
.arg(path)
.args(["add", "file.txt"])
.status()
.unwrap()
.success()
);
assert!(
Command::new("git")
.arg("-C")
.arg(path)
.args(["commit", "-m", "base"])
.status()
.unwrap()
.success()
);
let base = git_stdout(&path.to_path_buf(), ["rev-parse", "HEAD"])
.unwrap()
.trim()
.to_string();
assert!(
Command::new("git")
.arg("-C")
.arg(path)
.args(["checkout", "-b", "feature"])
.status()
.unwrap()
.success()
);
std::fs::write(path.join("file.txt"), "base\nfeature\n").unwrap();
assert!(
Command::new("git")
.arg("-C")
.arg(path)
.args(["commit", "-am", "feature"])
.status()
.unwrap()
.success()
);
let source = git_stdout(&path.to_path_buf(), ["rev-parse", "HEAD"])
.unwrap()
.trim()
.to_string();
let reader = RepositoryRegistryReader::new(vec![ConfiguredRepository {
id: "main".into(),
display_name: Some("Main".into()),
provider: "git".into(),
path: path.to_path_buf(),
uri: path.display().to_string(),
default_selector: Some("main".into()),
}]);
let target = reader.observe_merge_target("main", None).unwrap();
assert_eq!(target.selector, "main");
assert_eq!(target.commit, base);
assert_eq!(
reader.observe_commit("main", &source).unwrap().parents,
vec![base.clone()]
);
reader.ensure_ancestor("main", &base, &source).unwrap();
assert!(matches!(
reader.ensure_ancestor("main", &source, &base),
Err(RepositoryLookupError::InvalidCommitRelation { .. })
));
}
#[test]
fn unknown_repository_is_not_resolved_from_fallback() {
let reader = RepositoryRegistryReader::new(Vec::new());
crates/workspace-server/src/server.rs
+279 -20
View File
@@ -94,8 +94,8 @@ use crate::observation::{
use crate::profile_settings::UpdateWorkspaceMetadataRequest;
use crate::records::{ObjectiveDetail, ProjectRecordList, TicketDetail};
use crate::repositories::{
ConfiguredRepository, RepositoryListProjection, RepositoryLogRead, RepositoryLookupError,
RepositoryRegistryReader, RepositorySummary,
ConfiguredRepository, MergeTargetObservation, RepositoryListProjection, RepositoryLogRead,
RepositoryLookupError, RepositoryRegistryReader, RepositorySummary,
};
use crate::resource_broker::BackendResourceBroker;
use crate::runtime_subscription::RuntimeSubscriptionBroker;
@@ -1290,6 +1290,10 @@ pub fn build_router(api: WorkspaceApi) -> Router {
"/api/w/{workspace_id}/tickets/{id}/merge-request/revisions",
post(scoped_add_merge_request_revision),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/merge-request/merge-results",
post(scoped_record_merge_request_result),
)
.route(
"/api/w/{workspace_id}/internal/reviewer-child-sessions",
post(scoped_register_reviewer_child_session),
@@ -3521,7 +3525,6 @@ struct OpenMergeRequestRequest {
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
@@ -3535,7 +3538,6 @@ struct AddMergeRequestRevisionRequest {
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
@@ -3543,6 +3545,17 @@ struct AddMergeRequestRevisionRequest {
summary: String,
}
#[derive(Debug, serde::Deserialize)]
struct RecordMergeResultRequest {
expected_current_revision_id: String,
operation_id: String,
target_commit: String,
source_commit: String,
result_commit: String,
strategy: merge_request::MergeStrategy,
resolution: merge_request::MergeResolution,
}
#[derive(Debug, serde::Deserialize)]
struct RegisterReviewerChildSessionRequest {
child_session_id: String,
@@ -3552,6 +3565,8 @@ struct RegisterReviewerChildSessionRequest {
struct RegisterMergeRequestReviewAttemptRequest {
attempt_id: String,
revision_id: String,
#[serde(default)]
merge_result_id: Option<String>,
child_session_id: String,
capability_token: String,
}
@@ -3559,6 +3574,8 @@ struct RegisterMergeRequestReviewAttemptRequest {
#[derive(Debug, serde::Deserialize)]
struct SubmitMergeRequestReviewRequest {
revision_id: String,
#[serde(default)]
merge_result_id: Option<String>,
capability_token: String,
decision: merge_request::ReviewDecision,
#[serde(default)]
@@ -3612,14 +3629,89 @@ fn merge_request_store(
.map_err(Into::into)
}
fn repository_merge_evidence_error(error: RepositoryLookupError) -> ApiError {
Error::InvalidInput(format!(
"repository merge evidence validation failed: {error:?}"
))
.into()
}
fn validate_open_merge_request_evidence(
api: &WorkspaceApi,
ticket_id: &str,
repository_id: &str,
base_commit: &str,
head_commit: &str,
) -> ApiResult<(String, String, MergeTargetObservation)> {
let ticket = browser_ticket_backend(api)?
.show(TicketIdOrSlug::Id(ticket_id.into()))
.map_err(Error::from)?;
if ticket.meta.repository_id.as_deref() != Some(repository_id) {
return Err(Error::InvalidInput(
"Merge Request repository must match the authoritative Ticket target".into(),
)
.into());
}
let reader = api.repository_reader();
let target = reader
.observe_merge_target(repository_id, ticket.meta.ref_selector.as_deref())
.map_err(repository_merge_evidence_error)?;
let base = reader
.observe_commit(repository_id, base_commit)
.map_err(repository_merge_evidence_error)?;
let source = reader
.observe_commit(repository_id, head_commit)
.map_err(repository_merge_evidence_error)?;
reader
.ensure_ancestor(repository_id, &base.commit, &source.commit)
.map_err(repository_merge_evidence_error)?;
Ok((base.commit, source.commit, target))
}
fn validate_revision_evidence(
api: &WorkspaceApi,
repository_id: &str,
base_commit: &str,
head_commit: &str,
) -> ApiResult<(String, String)> {
let reader = api.repository_reader();
let base = reader
.observe_commit(repository_id, base_commit)
.map_err(repository_merge_evidence_error)?;
let source = reader
.observe_commit(repository_id, head_commit)
.map_err(repository_merge_evidence_error)?;
reader
.ensure_ancestor(repository_id, &base.commit, &source.commit)
.map_err(repository_merge_evidence_error)?;
Ok((base.commit, source.commit))
}
fn observe_merge_request_target(
api: &WorkspaceApi,
mr: &merge_request::MergeRequest,
) -> Option<String> {
let selector = mr.target_ref_selector.as_deref()?;
api.repository_reader()
.observe_merge_target(&mr.repository_id, Some(selector))
.ok()
.map(|target| target.commit)
}
async fn scoped_show_merge_request(
State(api): State<WorkspaceApi>,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
) -> ApiResult<Json<merge_request::MergeRequest>> {
let workspace_id = parse_workspace_id(&workspace_id)?;
let store = merge_request_store(&api, &workspace_id)?;
let current = store.show_for_ticket(&ticket_id)?.ok_or_else(|| {
Error::from(merge_request::MergeRequestError::NotFound(
ticket_id.clone(),
))
})?;
let target_commit = observe_merge_request_target(&api, &current);
let value = store
.show_for_ticket(&ticket_id)?
.show_for_ticket_with_target(&ticket_id, target_commit.as_deref())?
.ok_or_else(|| Error::from(merge_request::MergeRequestError::NotFound(ticket_id)))?;
Ok(Json(value))
}
@@ -3629,9 +3721,17 @@ async fn scoped_merge_request_readiness(
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
) -> ApiResult<Json<merge_request::MergeRequestReadiness>> {
let workspace_id = parse_workspace_id(&workspace_id)?;
Ok(Json(
merge_request_store(&api, &workspace_id)?.readiness_for_ticket(&ticket_id)?,
let store = merge_request_store(&api, &workspace_id)?;
let current = store.show_for_ticket(&ticket_id)?.ok_or_else(|| {
Error::from(merge_request::MergeRequestError::NotFound(
ticket_id.clone(),
))
})?;
let target_commit = observe_merge_request_target(&api, &current);
Ok(Json(store.readiness_for_ticket_with_target(
&ticket_id,
target_commit.as_deref(),
)?))
}
async fn scoped_open_merge_request(
@@ -3657,13 +3757,19 @@ async fn scoped_open_merge_request(
)
.into());
}
let (base_commit, source_commit, target) = validate_open_merge_request_evidence(
&api,
&ticket_id,
&input.repository_id,
&input.base_commit,
&input.head_commit,
)?;
let now = Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true);
let revision = merge_request::MergeRequestRevision {
revision_id: input.revision_id,
ordinal: 1,
base_commit: input.base_commit,
head_commit: input.head_commit,
head_tree: input.head_tree,
base_commit,
head_commit: source_commit,
diff_digest: input.diff_digest,
changed_paths: input.changed_paths,
summary: input.summary,
@@ -3675,6 +3781,7 @@ async fn scoped_open_merge_request(
merge_request_id: format!("mr_{}", Uuid::now_v7().simple()),
ticket_id,
repository_id: input.repository_id,
target_ref_selector: target.selector,
revision,
authenticated_runtime_id: source.runtime_id,
authenticated_worker_id: source.worker_id,
@@ -3714,6 +3821,12 @@ async fn scoped_add_merge_request_revision(
ticket_id.clone(),
))
})?;
let (base_commit, head_commit) = validate_revision_evidence(
&api,
&current.repository_id,
&input.base_commit,
&input.head_commit,
)?;
let now = Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true);
let mr =
merge_request_store(&api, &workspace_id)?.add_revision(merge_request::AddRevision {
@@ -3722,9 +3835,8 @@ async fn scoped_add_merge_request_revision(
revision: merge_request::MergeRequestRevision {
revision_id: input.revision_id,
ordinal: current.current_revision.ordinal + 1,
base_commit: input.base_commit,
head_commit: input.head_commit,
head_tree: input.head_tree,
base_commit,
head_commit,
diff_digest: input.diff_digest,
changed_paths: input.changed_paths,
summary: input.summary,
@@ -3738,6 +3850,105 @@ async fn scoped_add_merge_request_revision(
Ok(Json(mr))
}
async fn scoped_record_merge_request_result(
State(api): State<WorkspaceApi>,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
headers: HeaderMap,
Json(input): Json<RecordMergeResultRequest>,
) -> ApiResult<Json<merge_request::RecordMergeResultOutcome>> {
require_workspace_access(&workspace_id, &api)?;
let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?;
require_online_workspace_orchestrator_source(&api, &source)?;
let store = merge_request_store(&api, &workspace_id)?;
let mr = store.show_for_ticket(&ticket_id)?.ok_or_else(|| {
Error::from(merge_request::MergeRequestError::NotFound(
ticket_id.clone(),
))
})?;
if mr.current_revision.revision_id != input.expected_current_revision_id {
return Err(
Error::from(merge_request::MergeRequestError::StaleRevision {
expected: input.expected_current_revision_id,
current: mr.current_revision.revision_id,
})
.into(),
);
}
let target = api
.repository_reader()
.observe_merge_target(&mr.repository_id, mr.target_ref_selector.as_deref())
.map_err(repository_merge_evidence_error)?;
let supplied_target = api
.repository_reader()
.observe_commit(&mr.repository_id, &input.target_commit)
.map_err(repository_merge_evidence_error)?;
let supplied_source = api
.repository_reader()
.observe_commit(&mr.repository_id, &input.source_commit)
.map_err(repository_merge_evidence_error)?;
let supplied_result = api
.repository_reader()
.observe_commit(&mr.repository_id, &input.result_commit)
.map_err(repository_merge_evidence_error)?;
if target.commit != supplied_target.commit {
return Err(Error::InvalidInput(
"MergeResult target_commit is not the current target tip".into(),
)
.into());
}
if mr.current_revision.head_commit != supplied_source.commit {
return Err(Error::InvalidInput(
"MergeResult source_commit is not the current source revision".into(),
)
.into());
}
match input.strategy {
merge_request::MergeStrategy::FastForward => {
if input.resolution != merge_request::MergeResolution::None
|| supplied_result.commit != supplied_source.commit
{
return Err(Error::InvalidInput(
"fast-forward MergeResult must use the source commit and resolution=none"
.into(),
)
.into());
}
api.repository_reader()
.ensure_ancestor(&mr.repository_id, &target.commit, &supplied_source.commit)
.map_err(repository_merge_evidence_error)?;
}
merge_request::MergeStrategy::Merge => {
if input.resolution == merge_request::MergeResolution::None {
return Err(Error::InvalidInput(
"merge MergeResult requires clean or conflicts_resolved resolution".into(),
)
.into());
}
if supplied_result.parents.len() != 2
|| !supplied_result.parents.contains(&target.commit)
|| !supplied_result.parents.contains(&supplied_source.commit)
{
return Err(Error::InvalidInput("merge result commit must have exactly the target and source commits as parents".into()).into());
}
}
}
let outcome = store.record_merge_result(merge_request::RecordMergeResult {
merge_result_id: format!("MRG-{}", Uuid::new_v4()),
ticket_id,
expected_revision_id: mr.current_revision.revision_id,
target_commit: target.commit,
source_commit: supplied_source.commit,
result_commit: supplied_result.commit,
strategy: input.strategy,
resolution: input.resolution,
operation_id: input.operation_id,
actor_runtime_id: source.runtime_id,
actor_worker_id: source.worker_id,
created_at: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
})?;
Ok(Json(outcome))
}
async fn scoped_register_reviewer_child_session(
State(api): State<WorkspaceApi>,
headers: HeaderMap,
@@ -3773,7 +3984,9 @@ async fn scoped_register_merge_request_review_attempt(
.ok_or_else(|| {
Error::TicketAssignmentConflict("Ticket has no current assigned Coder".into())
})?;
if assignment.worker.runtime_id != source.runtime_id
if input.merge_result_id.is_some() {
require_online_workspace_orchestrator_source(&api, &source)?;
} else if assignment.worker.runtime_id != source.runtime_id
|| assignment.worker.worker_id != source.worker_id
{
return Err(Error::TicketAssignmentConflict(
@@ -3786,6 +3999,7 @@ async fn scoped_register_merge_request_review_attempt(
attempt_id: input.attempt_id,
ticket_id,
revision_id: input.revision_id,
merge_result_id: input.merge_result_id,
parent_assignment_id: assignment.assignment_id,
parent_runtime_id: source.runtime_id,
parent_worker_id: source.worker_id,
@@ -3807,6 +4021,7 @@ async fn scoped_submit_merge_request_review(
merge_request_store(&api, &workspace_id)?.submit_review(merge_request::SubmitReview {
ticket_id,
revision_id: input.revision_id,
merge_result_id: input.merge_result_id,
capability_token: input.capability_token,
decision: input.decision,
body: input.body,
@@ -3884,16 +4099,41 @@ async fn scoped_confirm_merge_request(
require_workspace_access(&workspace_id, &api)?;
reject_non_browser_merge_auth(&headers)?;
let actor = require_actor(&api, &headers).await?;
let mr = merge_request_store(&api, &workspace_id)?.confirm_merge(
merge_request::MergeConfirmation {
let store = merge_request_store(&api, &workspace_id)?;
let current = store.show_for_ticket(&ticket_id)?.ok_or_else(|| {
Error::from(merge_request::MergeRequestError::NotFound(
ticket_id.clone(),
))
})?;
let target_commit = observe_merge_request_target(&api, &current);
let observed = store
.show_for_ticket_with_target(&ticket_id, target_commit.as_deref())?
.ok_or_else(|| {
Error::from(merge_request::MergeRequestError::NotFound(
ticket_id.clone(),
))
})?;
if observed.applied_merge_result.is_none() {
return Err(Error::InvalidInput(
"Merge Request result is not the current target tip; target update is a separate prerequisite operation".into(),
).into());
}
let readiness = store.readiness_for_ticket_with_target(&ticket_id, target_commit.as_deref())?;
if !readiness.ready {
return Err(Error::InvalidInput(format!(
"Merge Request is not integration-ready: {}",
readiness.blockers.join("; ")
))
.into());
}
let mr = store.confirm_merge(merge_request::MergeConfirmation {
ticket_id,
expected_revision_id: input.expected_revision_id,
authenticated_account_id: actor.account_id,
actor_kind: "user".to_string(),
explicit_confirmation: input.explicit_confirmation,
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
},
)?;
})?;
Ok(Json(mr))
}
@@ -11128,6 +11368,21 @@ fn repository_lookup<T>(result: std::result::Result<T, RepositoryLookupError>) -
}],
)
}
other => {
let message = format!("repository evidence validation failed: {other:?}");
ApiError::with_diagnostics(
Error::RuntimeOperationFailed {
runtime_id: "workspace-repository-registry".to_string(),
code: "repository_evidence_invalid".to_string(),
message: message.clone(),
},
vec![RuntimeDiagnostic {
code: "repository_evidence_invalid".to_string(),
severity: DiagnosticSeverity::Error,
message,
}],
)
}
})
}
@@ -12193,12 +12448,13 @@ mod tests {
merge_request_id: "MR-server-completion".into(),
ticket_id: ticket.id.clone(),
repository_id: TEST_REPOSITORY_ID.into(),
target_ref_selector: "develop".into(),
revision: merge_request::MergeRequestRevision {
revision_id: "V1".into(),
ordinal: 1,
base_commit: "base".into(),
head_commit: "head".into(),
head_tree: "tree".into(),
diff_digest: "sha256:diff".into(),
changed_paths: vec!["src/lib.rs".into()],
summary: "approved revision".into(),
@@ -12223,6 +12479,7 @@ mod tests {
attempt_id: "attempt".into(),
ticket_id: ticket.id.clone(),
revision_id: "V1".into(),
merge_result_id: None,
parent_assignment_id: assignment.assignment_id.clone(),
parent_runtime_id: coder.worker_ref.runtime_id.clone(),
parent_worker_id: coder.worker_ref.worker_id.clone(),
@@ -12235,6 +12492,7 @@ mod tests {
.submit_review(merge_request::SubmitReview {
ticket_id: ticket.id.clone(),
revision_id: "V1".into(),
merge_result_id: None,
capability_token: "review-token".into(),
decision: merge_request::ReviewDecision::Approve,
body: "approved".into(),
@@ -16755,8 +17013,9 @@ mod tests {
);
assert!(!default_root.starts_with(workspace_root.join(".yoi")));
let config = ServerConfig::local_dev(workspace_root, test_identity())
let mut config = ServerConfig::local_dev(workspace_root, test_identity())
.with_embedded_runtime_store_root(default_root.clone());
config.database_path = ServerConfig::server_database_path_for_data_dir(&data_dir);
let app = build_router(
WorkspaceApi::new_with_execution_backend(
config,
docs/report/merge-request-schema-generation-skew-00001KZYAEWJT.md
+50
View File
@@ -0,0 +1,50 @@
# Merge Request schema generation skew blocks Coder review handoff
Ticket: `00001KZYAEWJT`
## Summary
The Ticket implementation committed a new immutable Merge Request schema revision that removes caller-supplied `head_tree`, adds target authority, and adds immutable `MergeResult` evidence. All intended source changes were committed on a clean Workdir before opening the MR revision.
`MergeRequestOpen` then failed with HTTP 409:
```text
merge request error: merge request database error: schema drift: table merge_request_revisions is missing required column head_tree
```
The authoritative database has already lost the legacy `head_tree` column, while the live Server generation serving Worker tools still verifies the previous schema and requires that column. The Coder cannot repair this through the Workdir or typed MR tools, and restarting the live dogfood Server/Runtime would violate the active Worker safety boundary.
## Impact
- The exact committed source revision cannot be opened as an immutable MR revision.
- The required trusted `builtin:reviewer` handoff cannot be created because it requires a current MR revision id.
- The Ticket cannot proceed to review or `MergeRequestComplete` even though source validation passed.
- Retrying the same typed tool cannot converge while the live binary and database schema remain on different generations.
## Evidence
Attempted MR evidence before the report-only follow-up commit:
- base: `11e4d536c50eef45801ea755cd8ef83add933749`
- head: `040ec49087b940803062425590b00ab3b3a78b7c`
- tree: `202df4a6e30bf437e451b8677b555f438108c5e7`
- diff digest: `sha256:45a0a0b1b916572d6804aece3f6db555d9748602131b17dd3727afb727fa8791`
- requested revision: `00001KZYAEWJT-r1-040ec490`
The current final source evidence must be recomputed after this report commit before retrying MR creation.
## Improvement
Schema authority changes used by live typed tools need generation fencing:
1. Server startup should verify the database schema before accepting MR tool traffic and report its executable/schema generation.
2. A schema-migrating generation must not share the authority database with an older running generation that still enforces the previous shape.
3. MR tool failures should report the running Server generation and observed schema version/columns in bounded diagnostics.
4. Deployment should atomically sequence migration and executable activation, or preserve a deliberate compatibility reader during the transition; a stale binary must never continue serving writes after the incompatible migration commits.
5. Coder review handoff should remain fail-closed, as it did here; it must not bypass immutable MR authority or fabricate review evidence when MR creation is unavailable.
## Resolution and follow-up
The deployment/bootstrap path was subsequently corrected and the user restarted the dogfood control plane. The live schema-generation blocker cleared without the Coder restarting its own Server or Runtime. The attempted revision evidence above is historical incident evidence only and is not a current MR candidate.
Work then resumed on the assigned Coder branch under the Ticket's stricter migration contract: fresh databases materialize one latest v9 baseline, only an exact v8 schema may migrate automatically, older/unversioned/drifted states fail closed, and v8-to-v9 DDL, evidence copy, foreign-key validation, and marker advancement commit atomically. Final Git and MR evidence is recorded separately on the Ticket and immutable MR revision.
web/workspace/src/routes/w/[workspaceId]/tickets/[ticketId]/+page.svelte
+49 -3
View File
@@ -20,8 +20,31 @@
type MergeRequestDetail = {
state: "draft" | "open" | "closed" | "merged";
review_status: "pending" | "approved" | "changes_requested";
current_revision: { revision_id: string; head_commit: string; head_tree: string; diff_digest: string; changed_paths: string[]; summary: string };
target_ref_selector?: string | null;
target_status: "known" | "unknown";
observed_target_commit?: string | null;
current_revision: { revision_id: string; head_commit: string; diff_digest: string; changed_paths: string[]; summary: string };
current_review?: { decision: string; body: string; reviewer_effective_profile: string } | null;
current_merge_result?: {
merge_result_id: string;
target_commit: string;
source_commit: string;
result_commit: string;
strategy: "fast_forward" | "merge";
resolution: "none" | "clean" | "conflicts_resolved";
target_status: "current" | "applied" | "stale" | "unknown";
review_status: "pending" | "approved" | "changes_requested";
} | null;
applied_merge_result?: {
merge_result_id: string;
target_commit: string;
source_commit: string;
result_commit: string;
strategy: "fast_forward" | "merge";
resolution: "none" | "clean" | "conflicts_resolved";
target_status: "current" | "applied" | "stale" | "unknown";
review_status: "pending" | "approved" | "changes_requested";
} | null;
merged_at?: string | null;
};
@@ -368,15 +391,38 @@
<p class="workspace-callout is-error">{data.mergeRequest.error}</p>
{:else if mergeRequest}
<p><strong>{mergeRequest.state}</strong> · {mergeRequest.review_status}</p>
<p>Target <code>{mergeRequest.target_ref_selector ?? "unknown"}</code> · {mergeRequest.target_status}</p>
{#if mergeRequest.observed_target_commit}<p>Target tip <code>{mergeRequest.observed_target_commit}</code></p>{/if}
<p><code>{mergeRequest.current_revision.revision_id}</code></p>
<p>Head <code>{mergeRequest.current_revision.head_commit}</code></p>
{#if mergeRequest.current_merge_result}
<p>
MergeResult <code>{mergeRequest.current_merge_result.merge_result_id}</code> ·
{mergeRequest.current_merge_result.strategy} / {mergeRequest.current_merge_result.resolution} ·
{mergeRequest.current_merge_result.target_status} / {mergeRequest.current_merge_result.review_status}
</p>
<p>Result <code>{mergeRequest.current_merge_result.result_commit}</code></p>
{:else if mergeRequest.applied_merge_result}
<p>
Applied MergeResult <code>{mergeRequest.applied_merge_result.merge_result_id}</code> ·
{mergeRequest.applied_merge_result.strategy} / {mergeRequest.applied_merge_result.resolution} ·
{mergeRequest.applied_merge_result.review_status}
</p>
<p>Result <code>{mergeRequest.applied_merge_result.result_commit}</code> is the current target tip.</p>
{:else if mergeRequest.state === "open"}
<p class="workspace-callout">No current MergeResult has been recorded for this target tip.</p>
{/if}
{#if mergeRequest.current_revision.summary}<p>{mergeRequest.current_revision.summary}</p>{/if}
{#if mergeRequest.current_review}
<p><strong>{mergeRequest.current_review.decision}</strong> by {mergeRequest.current_review.reviewer_effective_profile}</p>
{#if mergeRequest.current_review.body}<RichMarkdown text={mergeRequest.current_review.body} />{/if}
{/if}
{#if mergeRequest.state === "open" && mergeRequest.review_status === "approved"}
<label><input type="checkbox" bind:checked={confirmMerge} /> Explicitly confirm merge of this revision</label>
{#if mergeRequest.state === "open"
&& mergeRequest.review_status === "approved"
&& mergeRequest.applied_merge_result?.target_status === "applied"
&& (mergeRequest.applied_merge_result.strategy === "fast_forward"
|| mergeRequest.applied_merge_result.review_status === "approved")}
<label><input type="checkbox" bind:checked={confirmMerge} /> Explicitly confirm the externally applied result</label>
<button class="workspace-primary-button" type="button" disabled={!confirmMerge || busy !== null} onclick={mergeConfirmedRevision}>Confirm merge</button>
{/if}
{:else}