Compare commits
8
Commits
4f042cae84
...
46a44b232b
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
46a44b232b | ||
|
|
2cf4c57813 | ||
|
|
41534b215a | ||
|
|
5ea2792df7 | ||
|
|
1479148f84 | ||
|
|
7256d80514 | ||
|
|
8e73d755d3 | ||
|
|
027f60d262 |
+971
-395
File diff suppressed because it is too large
Load Diff
+162
-436
@@ -1,5 +1,7 @@
|
||||
use merge_request::*;
|
||||
use rusqlite::{Connection, params};
|
||||
use std::sync::{Arc, Barrier};
|
||||
use std::thread;
|
||||
use tempfile::TempDir;
|
||||
|
||||
fn setup() -> (TempDir, SqliteMergeRequestStore) {
|
||||
@@ -38,13 +40,13 @@ fn setup() -> (TempDir, SqliteMergeRequestStore) {
|
||||
let store = SqliteMergeRequestStore::open(&path, "ws-a").unwrap();
|
||||
(dir, store)
|
||||
}
|
||||
|
||||
fn revision(id: &str, ordinal: u64, head: &str) -> MergeRequestRevision {
|
||||
MergeRequestRevision {
|
||||
revision_id: id.into(),
|
||||
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}"),
|
||||
@@ -52,456 +54,199 @@ fn revision(id: &str, ordinal: u64, head: &str) -> MergeRequestRevision {
|
||||
created_at: format!("t{ordinal}"),
|
||||
}
|
||||
}
|
||||
|
||||
fn open(store: &SqliteMergeRequestStore) {
|
||||
store
|
||||
.open_merge_request(OpenMergeRequest {
|
||||
merge_request_id: "MR1".into(),
|
||||
ticket_id: "T1".into(),
|
||||
repository_id: "repo".into(),
|
||||
revision: revision("V1", 1, "h1"),
|
||||
target_ref_selector: "refs/heads/develop".into(),
|
||||
revision: revision("V1", 1, "head"),
|
||||
authenticated_runtime_id: "R1".into(),
|
||||
authenticated_worker_id: "W1".into(),
|
||||
now: "t1".into(),
|
||||
})
|
||||
.unwrap();
|
||||
}
|
||||
fn attempt(store: &SqliteMergeRequestStore, id: &str, revision: &str, token: &str, child: &str) {
|
||||
|
||||
fn attempt(store: &SqliteMergeRequestStore, revision: &str, token: &str) {
|
||||
let child = format!("child-{revision}");
|
||||
store
|
||||
.register_reviewer_child_session(RegisterReviewerChildSession {
|
||||
parent_runtime_id: "R1".into(),
|
||||
parent_worker_id: "W1".into(),
|
||||
child_session_id: child.into(),
|
||||
now: "t".into(),
|
||||
child_session_id: child.clone(),
|
||||
now: "t2".into(),
|
||||
})
|
||||
.unwrap();
|
||||
store
|
||||
.register_review_attempt(RegisterReviewAttempt {
|
||||
attempt_id: id.into(),
|
||||
attempt_id: format!("attempt-{revision}"),
|
||||
ticket_id: "T1".into(),
|
||||
revision_id: revision.into(),
|
||||
parent_assignment_id: "A1".into(),
|
||||
parent_runtime_id: "R1".into(),
|
||||
parent_worker_id: "W1".into(),
|
||||
child_session_id: child.into(),
|
||||
child_session_id: child,
|
||||
capability_token: token.into(),
|
||||
now: "t".into(),
|
||||
})
|
||||
.unwrap();
|
||||
}
|
||||
fn review(
|
||||
store: &SqliteMergeRequestStore,
|
||||
revision: &str,
|
||||
token: &str,
|
||||
decision: ReviewDecision,
|
||||
) -> Result<MergeRequestReview> {
|
||||
store.submit_review(SubmitReview {
|
||||
ticket_id: "T1".into(),
|
||||
revision_id: revision.into(),
|
||||
capability_token: token.into(),
|
||||
decision,
|
||||
body: "evidence".into(),
|
||||
findings: vec![],
|
||||
now: "tr".into(),
|
||||
})
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn storage_allows_multiple_merge_requests_for_one_ticket() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
store
|
||||
.open_merge_request(OpenMergeRequest {
|
||||
merge_request_id: "MR2".into(),
|
||||
ticket_id: "T1".into(),
|
||||
repository_id: "repo".into(),
|
||||
revision: revision("V2", 1, "h2"),
|
||||
authenticated_runtime_id: "R1".into(),
|
||||
authenticated_worker_id: "W1".into(),
|
||||
now: "t2".into(),
|
||||
})
|
||||
.unwrap();
|
||||
let conn = Connection::open(store.db_path()).unwrap();
|
||||
let count:i64=conn.query_row("SELECT COUNT(*) FROM merge_request_ticket_relations WHERE workspace_id='ws-a' AND ticket_id='T1'",[],|row|row.get(0)).unwrap();
|
||||
assert_eq!(count, 2);
|
||||
assert_eq!(
|
||||
store
|
||||
.show_for_ticket("T1")
|
||||
.unwrap()
|
||||
.unwrap()
|
||||
.merge_request_id,
|
||||
"MR2"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn bounded_context_rejects_oversized_revision_evidence() {
|
||||
let (_dir, store) = setup();
|
||||
let mut oversized = revision("V1", 1, "h1");
|
||||
oversized.changed_paths = (0..=1_000).map(|i| format!("src/{i}.rs")).collect();
|
||||
let result = store.open_merge_request(OpenMergeRequest {
|
||||
merge_request_id: "MR1".into(),
|
||||
ticket_id: "T1".into(),
|
||||
repository_id: "repo".into(),
|
||||
revision: oversized,
|
||||
authenticated_runtime_id: "R1".into(),
|
||||
authenticated_worker_id: "W1".into(),
|
||||
now: "t".into(),
|
||||
});
|
||||
assert!(matches!(
|
||||
result,
|
||||
Err(MergeRequestError::TooLarge {
|
||||
field: "revision.changed_paths",
|
||||
..
|
||||
fn approve(store: &SqliteMergeRequestStore, revision: &str, token: &str) {
|
||||
attempt(store, revision, token);
|
||||
store
|
||||
.submit_review(SubmitReview {
|
||||
ticket_id: "T1".into(),
|
||||
revision_id: revision.into(),
|
||||
capability_token: token.into(),
|
||||
decision: ReviewDecision::Approve,
|
||||
body: "approved".into(),
|
||||
findings: vec![],
|
||||
now: "t3".into(),
|
||||
})
|
||||
));
|
||||
.unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn rejected_v6_schema_missing_diff_digest_is_archived_before_fresh_v7() {
|
||||
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));",
|
||||
).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);
|
||||
fn completion(operation_id: &str) -> CompleteMergeRequest {
|
||||
CompleteMergeRequest {
|
||||
operation_id: operation_id.into(),
|
||||
ticket_id: "T1".into(),
|
||||
expected_revision_id: "V1".into(),
|
||||
target_commit: "base".into(),
|
||||
source_commit: "head".into(),
|
||||
result_commit: "head".into(),
|
||||
strategy: MergeStrategy::FastForward,
|
||||
resolution: MergeResolution::None,
|
||||
implementation_assignment_id: "A1".into(),
|
||||
completion_actor_runtime_id: "OR".into(),
|
||||
completion_actor_worker_id: "OW".into(),
|
||||
now: "t4".into(),
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn interrupted_legacy_archive_with_empty_recreated_table_resumes() {
|
||||
let dir = tempfile::tempdir().unwrap();
|
||||
let path = dir.path().join("interrupted.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(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",
|
||||
[],
|
||||
|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
|
||||
.query_row(
|
||||
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='merge_request_review_findings'",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
let archived_findings: i64 = conn
|
||||
.query_row(
|
||||
"SELECT COUNT(*) FROM sqlite_master WHERE type='table' AND name='legacy_v6_merge_request_review_findings'",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(current_findings, 1);
|
||||
assert_eq!(archived_findings, 0);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn v7_completion_operations_are_preserved_as_legacy_assigned_coder_authority() {
|
||||
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));\
|
||||
INSERT INTO merge_request_completion_operations VALUES('ws-a','legacy-op','T1','V1','A1','legacy-fingerprint','completed','done','t','t');",
|
||||
).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 version: i64 = conn
|
||||
.query_row(
|
||||
"SELECT MAX(version) FROM merge_request_schema_migrations",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(version, 8);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn request_changes_new_revision_resets_and_exact_completion_replay_converges() {
|
||||
fn target_movement_does_not_invalidate_source_revision_approval() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
attempt(&store, "AT1", "V1", "tok1", "child1");
|
||||
review(&store, "V1", "tok1", ReviewDecision::RequestChanges).unwrap();
|
||||
assert_eq!(
|
||||
store.show_for_ticket("T1").unwrap().unwrap().review_status,
|
||||
ReviewStatus::ChangesRequested
|
||||
);
|
||||
approve(&store, "V1", "token-v1");
|
||||
for target in ["base", "advanced-target"] {
|
||||
let readiness = store
|
||||
.readiness_for_ticket_with_target("T1", Some(target))
|
||||
.unwrap();
|
||||
assert!(
|
||||
readiness.ready,
|
||||
"target movement must not invalidate source approval"
|
||||
);
|
||||
assert_eq!(readiness.review_status, ReviewStatus::Approved);
|
||||
assert_eq!(readiness.observed_target_commit.as_deref(), Some(target));
|
||||
}
|
||||
store
|
||||
.add_revision(AddRevision {
|
||||
ticket_id: "T1".into(),
|
||||
expected_current_revision_id: "V1".into(),
|
||||
revision: revision("V2", 2, "h2"),
|
||||
revision: revision("V2", 2, "head2"),
|
||||
authenticated_runtime_id: "R1".into(),
|
||||
authenticated_worker_id: "W1".into(),
|
||||
now: "t2".into(),
|
||||
now: "t5".into(),
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
store.show_for_ticket("T1").unwrap().unwrap().review_status,
|
||||
store.readiness_for_ticket("T1").unwrap().review_status,
|
||||
ReviewStatus::Pending
|
||||
);
|
||||
assert!(review(&store, "V1", "tok1", ReviewDecision::Approve).is_err());
|
||||
attempt(&store, "AT2", "V2", "tok2", "child2");
|
||||
review(&store, "V2", "tok2", ReviewDecision::Approve).unwrap();
|
||||
let input = CompleteMergeRequest {
|
||||
operation_id: "OP1".into(),
|
||||
ticket_id: "T1".into(),
|
||||
expected_revision_id: "V2".into(),
|
||||
implementation_assignment_id: "A1".into(),
|
||||
completion_actor_runtime_id: "OR".into(),
|
||||
completion_actor_worker_id: "OW".into(),
|
||||
now: "tc".into(),
|
||||
};
|
||||
let first = store.complete(input.clone()).unwrap();
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn completion_records_one_final_merge_outcome_and_replays_idempotently() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
approve(&store, "V1", "token-v1");
|
||||
let first = store.complete(completion("OP1")).unwrap();
|
||||
assert!(!first.replayed);
|
||||
let replay = store.complete(input).unwrap();
|
||||
assert!(replay.replayed);
|
||||
assert!(matches!(
|
||||
store.confirm_merge(MergeConfirmation {
|
||||
ticket_id: "T1".into(),
|
||||
expected_revision_id: "V2".into(),
|
||||
authenticated_account_id: "runtime".into(),
|
||||
actor_kind: "worker".into(),
|
||||
explicit_confirmation: true,
|
||||
now: "tm".into()
|
||||
}),
|
||||
Err(MergeRequestError::MergeConfirmationRequired)
|
||||
));
|
||||
let merged = store
|
||||
.confirm_merge(MergeConfirmation {
|
||||
ticket_id: "T1".into(),
|
||||
expected_revision_id: "V2".into(),
|
||||
authenticated_account_id: "account-1".into(),
|
||||
actor_kind: "user".into(),
|
||||
explicit_confirmation: true,
|
||||
now: "tm".into(),
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(first.ticket_state, "done");
|
||||
let merged = store.show_for_ticket("T1").unwrap().unwrap();
|
||||
assert_eq!(merged.state, MergeRequestState::Merged);
|
||||
assert_eq!(merged.merged_revision_id.as_deref(), Some("V1"));
|
||||
assert_eq!(merged.merged_target_commit.as_deref(), Some("base"));
|
||||
assert_eq!(merged.merged_result_commit.as_deref(), Some("head"));
|
||||
assert_eq!(merged.merge_strategy, Some(MergeStrategy::FastForward));
|
||||
assert_eq!(merged.merge_resolution, Some(MergeResolution::None));
|
||||
assert_eq!(merged.merged_by_runtime_id.as_deref(), Some("OR"));
|
||||
assert_eq!(merged.merged_by_worker_id.as_deref(), Some("OW"));
|
||||
assert!(store.complete(completion("OP1")).unwrap().replayed);
|
||||
let mut conflicting = completion("OP1");
|
||||
conflicting.target_commit = "other".into();
|
||||
assert!(matches!(
|
||||
store.complete(conflicting),
|
||||
Err(MergeRequestError::OperationConflict)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn completion_rejects_invalid_or_non_current_source_outcomes_without_side_effects() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
approve(&store, "V1", "token-v1");
|
||||
let mut invalid_ff = completion("bad-ff");
|
||||
invalid_ff.result_commit = "different".into();
|
||||
assert!(matches!(
|
||||
store.complete(invalid_ff),
|
||||
Err(MergeRequestError::InvalidMergeOutcome(_))
|
||||
));
|
||||
let mut invalid_merge = completion("bad-merge");
|
||||
invalid_merge.strategy = MergeStrategy::Merge;
|
||||
assert!(matches!(
|
||||
store.complete(invalid_merge),
|
||||
Err(MergeRequestError::InvalidMergeOutcome(_))
|
||||
));
|
||||
let mut wrong_source = completion("wrong-source");
|
||||
wrong_source.source_commit = "not-approved".into();
|
||||
wrong_source.result_commit = "not-approved".into();
|
||||
assert!(matches!(
|
||||
store.complete(wrong_source),
|
||||
Err(MergeRequestError::InvalidMergeOutcome(_))
|
||||
));
|
||||
assert_eq!(
|
||||
store.show_for_ticket("T1").unwrap().unwrap().state,
|
||||
MergeRequestState::Open
|
||||
);
|
||||
let conn = Connection::open(store.db_path()).unwrap();
|
||||
assert_eq!(
|
||||
conn.query_row(
|
||||
"SELECT workflow_state FROM typed_tickets WHERE workspace_id='ws-a' AND ticket_id='T1'",
|
||||
[],
|
||||
|r| r.get::<_, String>(0)
|
||||
|row| row.get::<_, String>(0)
|
||||
)
|
||||
.unwrap(),
|
||||
"done"
|
||||
"inprogress"
|
||||
);
|
||||
assert_eq!(
|
||||
conn.query_row(
|
||||
"SELECT COUNT(*) FROM typed_ticket_events WHERE workspace_id='ws-a' AND ticket_id='T1'",
|
||||
[],
|
||||
|r| r.get::<_, i64>(0)
|
||||
)
|
||||
.unwrap(),
|
||||
1
|
||||
);
|
||||
assert_eq!(
|
||||
conn.query_row(
|
||||
"SELECT authority_kind || ':' || implementation_assignment_id || ':' || completion_actor_runtime_id || ':' || completion_actor_worker_id FROM merge_request_completion_operations WHERE workspace_id='ws-a' AND operation_id='OP1'",
|
||||
[],
|
||||
|r| r.get::<_, String>(0)
|
||||
)
|
||||
.unwrap(),
|
||||
"workspace_orchestrator:A1:OR:OW"
|
||||
);
|
||||
assert_eq!(
|
||||
conn.query_row(
|
||||
"SELECT author FROM typed_ticket_events WHERE workspace_id='ws-a' AND ticket_id='T1' AND kind='state_changed'",
|
||||
[],
|
||||
|r| r.get::<_, String>(0)
|
||||
)
|
||||
.unwrap(),
|
||||
"worker:OR:OW"
|
||||
);
|
||||
let authority: String = conn
|
||||
.query_row(
|
||||
"SELECT value FROM typed_ticket_event_attributes WHERE workspace_id='ws-a' AND ticket_id='T1' AND key='completion_authority'",
|
||||
[],
|
||||
|r| r.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(authority, "workspace_orchestrator");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn spoof_self_approval_replay_and_cross_workspace_are_rejected() {
|
||||
fn concurrent_completion_converges_on_one_operation() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
let mut bad = RegisterReviewAttempt {
|
||||
attempt_id: "bad".into(),
|
||||
ticket_id: "T1".into(),
|
||||
revision_id: "V1".into(),
|
||||
parent_assignment_id: "A1".into(),
|
||||
parent_runtime_id: "R1".into(),
|
||||
parent_worker_id: "W1".into(),
|
||||
child_session_id: "W1".into(),
|
||||
capability_token: "bad".into(),
|
||||
now: "t".into(),
|
||||
};
|
||||
assert!(matches!(
|
||||
store.register_review_attempt(bad.clone()),
|
||||
Err(MergeRequestError::SelfApproval)
|
||||
));
|
||||
bad.child_session_id = "child".into();
|
||||
assert!(matches!(
|
||||
store.register_review_attempt(bad),
|
||||
Err(MergeRequestError::InvalidReviewer)
|
||||
));
|
||||
attempt(&store, "AT", "V1", "secret", "child");
|
||||
assert!(review(&store, "V1", "spoof", ReviewDecision::Approve).is_err());
|
||||
review(&store, "V1", "secret", ReviewDecision::Approve).unwrap();
|
||||
assert!(review(&store, "V1", "secret", ReviewDecision::Approve).is_err());
|
||||
let other = SqliteMergeRequestStore::open_verified(store.db_path(), "ws-b").unwrap();
|
||||
assert!(other.show_for_ticket("T1").unwrap().is_none());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn reopen_resets_approval_and_merge_requires_authenticated_explicit_user() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
attempt(&store, "AT", "V1", "token", "child");
|
||||
review(&store, "V1", "token", ReviewDecision::Approve).unwrap();
|
||||
store.close("T1", "V1", "tc").unwrap();
|
||||
let reopened = store.reopen("T1", "V1", "tr").unwrap();
|
||||
assert_eq!(reopened.review_status, ReviewStatus::Pending);
|
||||
let denied = store.confirm_merge(MergeConfirmation {
|
||||
ticket_id: "T1".into(),
|
||||
expected_revision_id: "V1".into(),
|
||||
authenticated_account_id: "user".into(),
|
||||
actor_kind: "user".into(),
|
||||
explicit_confirmation: false,
|
||||
now: "tm".into(),
|
||||
});
|
||||
assert!(matches!(
|
||||
denied,
|
||||
Err(MergeRequestError::MergeConfirmationRequired)
|
||||
));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn concurrent_exact_completion_replays_commit_one_ticket_side_effect() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
attempt(&store, "AT", "V1", "token", "child");
|
||||
review(&store, "V1", "token", ReviewDecision::Approve).unwrap();
|
||||
let input = CompleteMergeRequest {
|
||||
operation_id: "OP-concurrent".into(),
|
||||
ticket_id: "T1".into(),
|
||||
expected_revision_id: "V1".into(),
|
||||
implementation_assignment_id: "A1".into(),
|
||||
completion_actor_runtime_id: "OR".into(),
|
||||
completion_actor_worker_id: "OW".into(),
|
||||
now: "t".into(),
|
||||
};
|
||||
let left_store = store.clone();
|
||||
let left_input = input.clone();
|
||||
let left = std::thread::spawn(move || left_store.complete(left_input));
|
||||
let right_store = store.clone();
|
||||
let right = std::thread::spawn(move || right_store.complete(input));
|
||||
let outcomes = [
|
||||
left.join().unwrap().unwrap(),
|
||||
right.join().unwrap().unwrap(),
|
||||
];
|
||||
approve(&store, "V1", "token-v1");
|
||||
let path = store.db_path().to_path_buf();
|
||||
let barrier = Arc::new(Barrier::new(3));
|
||||
let mut handles = Vec::new();
|
||||
for _ in 0..2 {
|
||||
let path = path.clone();
|
||||
let barrier = barrier.clone();
|
||||
handles.push(thread::spawn(move || {
|
||||
let store = SqliteMergeRequestStore::open_verified(path, "ws-a").unwrap();
|
||||
barrier.wait();
|
||||
store.complete(completion("OP-concurrent"))
|
||||
}));
|
||||
}
|
||||
barrier.wait();
|
||||
let outcomes: Vec<_> = handles
|
||||
.into_iter()
|
||||
.map(|handle| handle.join().unwrap().unwrap())
|
||||
.collect();
|
||||
assert_eq!(
|
||||
outcomes.iter().filter(|outcome| !outcome.replayed).count(),
|
||||
1
|
||||
@@ -510,63 +255,44 @@ fn concurrent_exact_completion_replays_commit_one_ticket_side_effect() {
|
||||
outcomes.iter().filter(|outcome| outcome.replayed).count(),
|
||||
1
|
||||
);
|
||||
let conn = Connection::open(store.db_path()).unwrap();
|
||||
let events: i64 = conn
|
||||
.query_row(
|
||||
"SELECT COUNT(*) FROM typed_ticket_events WHERE workspace_id='ws-a' AND ticket_id='T1'",
|
||||
[],
|
||||
|row| row.get(0),
|
||||
)
|
||||
.unwrap();
|
||||
assert_eq!(events, 1);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn operation_key_mismatch_and_actor_or_assignment_change_are_fenced() {
|
||||
fn reviewer_attempt_is_bound_to_direct_child_and_current_assignment() {
|
||||
let (_dir, store) = setup();
|
||||
open(&store);
|
||||
attempt(&store, "AT", "V1", "token", "child");
|
||||
review(&store, "V1", "token", ReviewDecision::Approve).unwrap();
|
||||
let mut input = CompleteMergeRequest {
|
||||
operation_id: "OP".into(),
|
||||
store
|
||||
.register_reviewer_child_session(RegisterReviewerChildSession {
|
||||
parent_runtime_id: "R1".into(),
|
||||
parent_worker_id: "W1".into(),
|
||||
child_session_id: "child".into(),
|
||||
now: "t2".into(),
|
||||
})
|
||||
.unwrap();
|
||||
store
|
||||
.register_review_attempt(RegisterReviewAttempt {
|
||||
attempt_id: "attempt".into(),
|
||||
ticket_id: "T1".into(),
|
||||
revision_id: "V1".into(),
|
||||
parent_assignment_id: "A1".into(),
|
||||
parent_runtime_id: "R1".into(),
|
||||
parent_worker_id: "W1".into(),
|
||||
child_session_id: "child".into(),
|
||||
capability_token: "token".into(),
|
||||
now: "t2".into(),
|
||||
})
|
||||
.unwrap();
|
||||
let wrong_token = store.submit_review(SubmitReview {
|
||||
ticket_id: "T1".into(),
|
||||
expected_revision_id: "V1".into(),
|
||||
implementation_assignment_id: "A1".into(),
|
||||
completion_actor_runtime_id: "OR".into(),
|
||||
completion_actor_worker_id: "OW".into(),
|
||||
now: "t".into(),
|
||||
};
|
||||
let conn = Connection::open(store.db_path()).unwrap();
|
||||
conn.execute(
|
||||
"UPDATE ticket_current_worker_assignments SET assignment_id='A2',runtime_id='R2',worker_id='W2' WHERE workspace_id='ws-a' AND ticket_id='T1'",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
revision_id: "V1".into(),
|
||||
capability_token: "wrong".into(),
|
||||
decision: ReviewDecision::Approve,
|
||||
body: "approved".into(),
|
||||
findings: vec![],
|
||||
now: "t3".into(),
|
||||
});
|
||||
assert!(matches!(
|
||||
store.complete(input.clone()),
|
||||
Err(MergeRequestError::AssignmentMismatch)
|
||||
));
|
||||
conn.execute(
|
||||
"UPDATE ticket_current_worker_assignments SET assignment_id='A1',runtime_id='R1',worker_id='W1' WHERE workspace_id='ws-a' AND ticket_id='T1'",
|
||||
[],
|
||||
)
|
||||
.unwrap();
|
||||
store.complete(input.clone()).unwrap();
|
||||
input.completion_actor_worker_id = "other".into();
|
||||
assert!(matches!(
|
||||
store.complete(input.clone()),
|
||||
Err(MergeRequestError::OperationConflict)
|
||||
));
|
||||
input.completion_actor_worker_id = "OW".into();
|
||||
input.implementation_assignment_id = "A2".into();
|
||||
assert!(matches!(
|
||||
store.complete(input.clone()),
|
||||
Err(MergeRequestError::OperationConflict)
|
||||
));
|
||||
input.implementation_assignment_id = "A1".into();
|
||||
input.expected_revision_id = "other".into();
|
||||
assert!(matches!(
|
||||
store.complete(input),
|
||||
Err(MergeRequestError::OperationConflict)
|
||||
wrong_token,
|
||||
Err(MergeRequestError::InvalidReviewAttempt)
|
||||
));
|
||||
}
|
||||
|
||||
@@ -41,7 +41,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 +54,6 @@ struct AddRevisionInput {
|
||||
revision_id: String,
|
||||
base_commit: String,
|
||||
head_commit: String,
|
||||
head_tree: String,
|
||||
diff_digest: String,
|
||||
#[serde(default)]
|
||||
changed_paths: Vec<String>,
|
||||
@@ -67,6 +65,24 @@ struct CompleteInput {
|
||||
ticket: String,
|
||||
operation_id: String,
|
||||
expected_revision_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 ReviewInput {
|
||||
@@ -158,7 +174,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,13 +188,22 @@ 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::Complete => {
|
||||
let v: CompleteInput = 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!(
|
||||
@@ -186,7 +211,7 @@ impl Tool for MergeRequestTool {
|
||||
v.ticket
|
||||
),
|
||||
Some(
|
||||
json!({"operation_id":v.operation_id,"expected_revision_id":v.expected_revision_id}),
|
||||
json!({"operation_id":v.operation_id,"expected_revision_id":v.expected_revision_id,"target_commit":v.target_commit,"source_commit":v.source_commit,"result_commit":v.result_commit,"strategy":strategy,"resolution":resolution}),
|
||||
),
|
||||
)
|
||||
}
|
||||
@@ -297,3 +322,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_candidate_result_tool() {
|
||||
let open = serde_json::to_string(&schemars::schema_for!(OpenInput)).unwrap();
|
||||
let add = serde_json::to_string(&schemars::schema_for!(AddRevisionInput)).unwrap();
|
||||
let complete = serde_json::to_string(&schemars::schema_for!(CompleteInput)).unwrap();
|
||||
assert!(!open.contains("head_tree"));
|
||||
assert!(!add.contains("head_tree"));
|
||||
assert!(complete.contains("result_commit"));
|
||||
assert!(complete.contains("conflicts_resolved"));
|
||||
assert!(!MERGE_REQUEST_COMMON_TOOL_NAMES.contains(&"MergeRequestRecordMergeResult"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -94,10 +94,6 @@ pub enum Error {
|
||||
},
|
||||
#[error("unknown local repository `{0}`")]
|
||||
UnknownRepository(String),
|
||||
#[error(
|
||||
"merge confirmation requires an authenticated Browser session; API tokens and Worker actors are not accepted"
|
||||
)]
|
||||
BrowserMergeConfirmationRequired,
|
||||
#[error(
|
||||
"Merge Request reopen requires an authenticated Browser session and explicit confirmation"
|
||||
)]
|
||||
|
||||
@@ -83,10 +83,52 @@ 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 },
|
||||
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,
|
||||
},
|
||||
TargetMoved {
|
||||
id: RepositoryId,
|
||||
selector: String,
|
||||
expected: String,
|
||||
observed: Option<String>,
|
||||
},
|
||||
ProviderFailure {
|
||||
id: RepositoryId,
|
||||
operation: String,
|
||||
},
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone)]
|
||||
@@ -167,6 +209,155 @@ 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() })?;
|
||||
let selector = normalize_target_branch_selector(id, &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}"),
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
pub fn update_merge_target(
|
||||
&self,
|
||||
id: &str,
|
||||
selector: &str,
|
||||
expected_target: &str,
|
||||
result_commit: &str,
|
||||
) -> Result<(), RepositoryLookupError> {
|
||||
let repository = self.merge_repository(id)?;
|
||||
let target_ref = normalize_target_branch_selector(id, selector)?;
|
||||
self.observe_commit(id, result_commit)?;
|
||||
let status = Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(&repository.path)
|
||||
.args([
|
||||
"update-ref",
|
||||
target_ref.as_str(),
|
||||
result_commit,
|
||||
expected_target,
|
||||
])
|
||||
.status()
|
||||
.map_err(|_| RepositoryLookupError::ProviderFailure {
|
||||
id: id.into(),
|
||||
operation: "guarded target update".into(),
|
||||
})?;
|
||||
if status.success() {
|
||||
return Ok(());
|
||||
}
|
||||
let observed = self
|
||||
.observe_merge_target(id, Some(selector))
|
||||
.ok()
|
||||
.map(|target| target.commit);
|
||||
Err(RepositoryLookupError::TargetMoved {
|
||||
id: id.into(),
|
||||
selector: selector.into(),
|
||||
expected: expected_target.into(),
|
||||
observed,
|
||||
})
|
||||
}
|
||||
|
||||
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 +447,50 @@ impl RepositoryRegistryReader {
|
||||
}
|
||||
}
|
||||
|
||||
fn normalize_target_branch_selector(
|
||||
id: &str,
|
||||
selector: &str,
|
||||
) -> Result<String, RepositoryLookupError> {
|
||||
let selector = selector.trim();
|
||||
let target_ref = if selector.starts_with("refs/heads/") {
|
||||
selector.to_owned()
|
||||
} else if selector.starts_with("refs/") {
|
||||
return Err(RepositoryLookupError::InvalidSelector {
|
||||
id: id.into(),
|
||||
selector: selector.into(),
|
||||
});
|
||||
} else {
|
||||
format!("refs/heads/{selector}")
|
||||
};
|
||||
let valid_ref = Command::new("git")
|
||||
.args(["check-ref-format", target_ref.as_str()])
|
||||
.status()
|
||||
.map_err(|_| RepositoryLookupError::ProviderFailure {
|
||||
id: id.into(),
|
||||
operation: "validate target branch ref".into(),
|
||||
})?;
|
||||
if !valid_ref.success() || selector.starts_with('-') || selector.as_bytes().contains(&0) {
|
||||
return Err(RepositoryLookupError::InvalidSelector {
|
||||
id: id.into(),
|
||||
selector: selector.into(),
|
||||
});
|
||||
}
|
||||
Ok(target_ref)
|
||||
}
|
||||
|
||||
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 +657,124 @@ 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();
|
||||
assert!(
|
||||
Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(path)
|
||||
.args(["update-ref", "refs/tags/main", &source])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success()
|
||||
);
|
||||
|
||||
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", Some("main")).unwrap();
|
||||
assert_eq!(target.selector, "refs/heads/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();
|
||||
reader
|
||||
.update_merge_target("main", "main", &base, &source)
|
||||
.unwrap();
|
||||
assert_eq!(
|
||||
reader
|
||||
.observe_merge_target("main", Some("refs/heads/main"))
|
||||
.unwrap()
|
||||
.commit,
|
||||
source
|
||||
);
|
||||
assert!(matches!(
|
||||
reader.update_merge_target("main", "refs/heads/main", &base, &base),
|
||||
Err(RepositoryLookupError::TargetMoved { .. })
|
||||
));
|
||||
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());
|
||||
|
||||
@@ -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;
|
||||
@@ -1310,10 +1310,6 @@ pub fn build_router(api: WorkspaceApi) -> Router {
|
||||
"/api/w/{workspace_id}/tickets/{id}/merge-request/reopen",
|
||||
post(scoped_reopen_merge_request),
|
||||
)
|
||||
.route(
|
||||
"/api/w/{workspace_id}/tickets/{id}/merge-request/merge",
|
||||
post(scoped_confirm_merge_request),
|
||||
)
|
||||
.route(
|
||||
"/api/w/{workspace_id}/tickets/{id}/workflow/close",
|
||||
post(scoped_close_ticket_record),
|
||||
@@ -3521,7 +3517,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 +3530,6 @@ struct AddMergeRequestRevisionRequest {
|
||||
revision_id: String,
|
||||
base_commit: String,
|
||||
head_commit: String,
|
||||
head_tree: String,
|
||||
diff_digest: String,
|
||||
#[serde(default)]
|
||||
changed_paths: Vec<String>,
|
||||
@@ -3571,6 +3565,11 @@ struct SubmitMergeRequestReviewRequest {
|
||||
struct CompleteMergeRequestRequest {
|
||||
operation_id: String,
|
||||
expected_revision_id: String,
|
||||
target_commit: String,
|
||||
source_commit: String,
|
||||
result_commit: String,
|
||||
strategy: merge_request::MergeStrategy,
|
||||
resolution: merge_request::MergeResolution,
|
||||
}
|
||||
|
||||
#[derive(Debug, serde::Deserialize)]
|
||||
@@ -3579,12 +3578,6 @@ struct RevisionTransitionRequest {
|
||||
explicit_confirmation: bool,
|
||||
}
|
||||
|
||||
#[derive(Debug, serde::Deserialize)]
|
||||
struct ConfirmMergeRequestRequest {
|
||||
expected_revision_id: String,
|
||||
explicit_confirmation: bool,
|
||||
}
|
||||
|
||||
fn parse_workspace_id(value: &str) -> ApiResult<String> {
|
||||
if value.trim().is_empty() {
|
||||
return Err(Error::InvalidInput("workspace_id must not be empty".to_string()).into());
|
||||
@@ -3612,14 +3605,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, ¤t);
|
||||
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 +3697,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, ¤t);
|
||||
Ok(Json(store.readiness_for_ticket_with_target(
|
||||
&ticket_id,
|
||||
target_commit.as_deref(),
|
||||
)?))
|
||||
}
|
||||
|
||||
async fn scoped_open_merge_request(
|
||||
@@ -3657,13 +3733,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 +3757,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 +3797,12 @@ async fn scoped_add_merge_request_revision(
|
||||
ticket_id.clone(),
|
||||
))
|
||||
})?;
|
||||
let (base_commit, head_commit) = validate_revision_evidence(
|
||||
&api,
|
||||
¤t.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 +3811,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,
|
||||
@@ -3832,18 +3920,176 @@ async fn scoped_complete_merge_request(
|
||||
.ok_or_else(|| {
|
||||
Error::TicketAssignmentConflict("Ticket has no current assigned Coder".into())
|
||||
})?;
|
||||
let outcome = merge_request_store(&api, &workspace_id)?.complete(
|
||||
merge_request::CompleteMergeRequest {
|
||||
operation_id: input.operation_id,
|
||||
ticket_id,
|
||||
expected_revision_id: input.expected_revision_id,
|
||||
implementation_assignment_id: assignment.assignment_id,
|
||||
completion_actor_runtime_id: source.runtime_id,
|
||||
completion_actor_worker_id: source.worker_id,
|
||||
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
|
||||
},
|
||||
)?;
|
||||
Ok(Json(outcome))
|
||||
let store = merge_request_store(&api, &workspace_id)?;
|
||||
let mr = store
|
||||
.show_for_ticket(&ticket_id)?
|
||||
.ok_or_else(|| merge_request::MergeRequestError::NotFound(ticket_id.clone()))?;
|
||||
if mr.current_revision.revision_id != input.expected_revision_id {
|
||||
return Err(merge_request::MergeRequestError::StaleRevision {
|
||||
expected: input.expected_revision_id,
|
||||
current: mr.current_revision.revision_id,
|
||||
}
|
||||
.into());
|
||||
}
|
||||
if mr.review_status != merge_request::ReviewStatus::Approved {
|
||||
return Err(merge_request::MergeRequestError::NotApproved.into());
|
||||
}
|
||||
if input.source_commit != mr.current_revision.head_commit {
|
||||
return Err(merge_request::MergeRequestError::InvalidMergeOutcome(
|
||||
"source commit does not match the current approved revision".into(),
|
||||
)
|
||||
.into());
|
||||
}
|
||||
if mr.state == merge_request::MergeRequestState::Merged {
|
||||
return Ok(Json(store.complete(
|
||||
merge_request::CompleteMergeRequest {
|
||||
operation_id: input.operation_id,
|
||||
ticket_id,
|
||||
expected_revision_id: mr.current_revision.revision_id,
|
||||
target_commit: input.target_commit,
|
||||
source_commit: input.source_commit,
|
||||
result_commit: input.result_commit,
|
||||
strategy: input.strategy,
|
||||
resolution: input.resolution,
|
||||
implementation_assignment_id: assignment.assignment_id,
|
||||
completion_actor_runtime_id: source.runtime_id,
|
||||
completion_actor_worker_id: source.worker_id,
|
||||
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
|
||||
},
|
||||
)?));
|
||||
}
|
||||
let selector = mr
|
||||
.target_ref_selector
|
||||
.as_deref()
|
||||
.ok_or(merge_request::MergeRequestError::UnknownTarget)?;
|
||||
let repositories = api.repository_reader();
|
||||
let observed_target = repositories
|
||||
.observe_merge_target(&mr.repository_id, Some(selector))
|
||||
.map_err(repository_merge_evidence_error)?;
|
||||
let source_commit = repositories
|
||||
.observe_commit(&mr.repository_id, &input.source_commit)
|
||||
.map_err(repository_merge_evidence_error)?;
|
||||
if source_commit.commit != input.source_commit {
|
||||
return Err(Error::InvalidInput("source commit must be canonical".into()).into());
|
||||
}
|
||||
let result_commit = repositories
|
||||
.observe_commit(&mr.repository_id, &input.result_commit)
|
||||
.map_err(repository_merge_evidence_error)?;
|
||||
if result_commit.commit != input.result_commit {
|
||||
return Err(Error::InvalidInput("result commit must be canonical".into()).into());
|
||||
}
|
||||
match input.strategy {
|
||||
merge_request::MergeStrategy::FastForward => {
|
||||
if input.resolution != merge_request::MergeResolution::None
|
||||
|| input.result_commit != input.source_commit
|
||||
{
|
||||
return Err(merge_request::MergeRequestError::InvalidMergeOutcome(
|
||||
"fast-forward result must equal the approved source and use resolution=none"
|
||||
.into(),
|
||||
)
|
||||
.into());
|
||||
}
|
||||
repositories
|
||||
.ensure_ancestor(
|
||||
&mr.repository_id,
|
||||
&input.target_commit,
|
||||
&input.source_commit,
|
||||
)
|
||||
.map_err(repository_merge_evidence_error)?;
|
||||
}
|
||||
merge_request::MergeStrategy::Merge => {
|
||||
if input.resolution == merge_request::MergeResolution::None
|
||||
|| result_commit.parents
|
||||
!= vec![input.target_commit.clone(), input.source_commit.clone()]
|
||||
{
|
||||
return Err(merge_request::MergeRequestError::InvalidMergeOutcome(
|
||||
"merge result must have the expected target and approved source as its two ordered parents"
|
||||
.into(),
|
||||
)
|
||||
.into());
|
||||
}
|
||||
}
|
||||
}
|
||||
let target_was_already_updated = observed_target.commit == input.result_commit;
|
||||
if observed_target.commit != input.target_commit && !target_was_already_updated {
|
||||
return Err(Error::InvalidInput(format!(
|
||||
"Merge Request target moved: expected {}, observed {}",
|
||||
input.target_commit, observed_target.commit
|
||||
))
|
||||
.into());
|
||||
}
|
||||
if !target_was_already_updated {
|
||||
repositories
|
||||
.update_merge_target(
|
||||
&mr.repository_id,
|
||||
selector,
|
||||
&input.target_commit,
|
||||
&input.result_commit,
|
||||
)
|
||||
.map_err(repository_merge_evidence_error)?;
|
||||
}
|
||||
let verified_target = repositories.observe_merge_target(&mr.repository_id, Some(selector));
|
||||
let verified_result = matches!(
|
||||
verified_target.as_ref(),
|
||||
Ok(target) if target.commit == input.result_commit
|
||||
);
|
||||
if !verified_result {
|
||||
if !target_was_already_updated {
|
||||
if let Err(rollback_error) = repositories.update_merge_target(
|
||||
&mr.repository_id,
|
||||
selector,
|
||||
&input.result_commit,
|
||||
&input.target_commit,
|
||||
) {
|
||||
return Err(Error::InvalidInput(format!(
|
||||
"post-update target verification failed and guarded rollback also failed: verification={verified_target:?}; rollback={rollback_error:?}"
|
||||
))
|
||||
.into());
|
||||
}
|
||||
}
|
||||
return Err(Error::InvalidInput(format!(
|
||||
"post-update target verification failed: expected result {}, observed {:?}",
|
||||
input.result_commit,
|
||||
verified_target
|
||||
.as_ref()
|
||||
.map(|target| target.commit.as_str())
|
||||
.map_err(|error| error)
|
||||
))
|
||||
.into());
|
||||
}
|
||||
let completion = merge_request::CompleteMergeRequest {
|
||||
operation_id: input.operation_id,
|
||||
ticket_id,
|
||||
expected_revision_id: mr.current_revision.revision_id,
|
||||
target_commit: input.target_commit.clone(),
|
||||
source_commit: input.source_commit,
|
||||
result_commit: input.result_commit.clone(),
|
||||
strategy: input.strategy,
|
||||
resolution: input.resolution,
|
||||
implementation_assignment_id: assignment.assignment_id,
|
||||
completion_actor_runtime_id: source.runtime_id,
|
||||
completion_actor_worker_id: source.worker_id,
|
||||
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
|
||||
};
|
||||
match store.complete(completion) {
|
||||
Ok(outcome) => Ok(Json(outcome)),
|
||||
Err(error) => {
|
||||
if !target_was_already_updated {
|
||||
if let Err(rollback_error) = repositories.update_merge_target(
|
||||
&mr.repository_id,
|
||||
selector,
|
||||
&input.result_commit,
|
||||
&input.target_commit,
|
||||
) {
|
||||
return Err(Error::InvalidInput(format!(
|
||||
"merge finalization failed after target update and guarded rollback also failed: finalization={error}; rollback={rollback_error:?}"
|
||||
))
|
||||
.into());
|
||||
}
|
||||
}
|
||||
Err(error.into())
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
async fn scoped_reopen_merge_request(
|
||||
@@ -3854,8 +4100,7 @@ async fn scoped_reopen_merge_request(
|
||||
) -> ApiResult<Json<merge_request::MergeRequest>> {
|
||||
let workspace_id = parse_workspace_id(&workspace_id)?;
|
||||
require_workspace_access(&workspace_id, &api)?;
|
||||
reject_non_browser_merge_auth(&headers)
|
||||
.map_err(|_| Error::BrowserReopenConfirmationRequired)?;
|
||||
reject_non_browser_reopen_auth(&headers)?;
|
||||
let _actor = require_actor(&api, &headers).await?;
|
||||
if !input.explicit_confirmation {
|
||||
return Err(Error::BrowserReopenConfirmationRequired.into());
|
||||
@@ -3867,36 +4112,13 @@ async fn scoped_reopen_merge_request(
|
||||
)?))
|
||||
}
|
||||
|
||||
fn reject_non_browser_merge_auth(headers: &HeaderMap) -> Result<()> {
|
||||
fn reject_non_browser_reopen_auth(headers: &HeaderMap) -> Result<()> {
|
||||
if headers.contains_key("authorization") {
|
||||
return Err(Error::BrowserMergeConfirmationRequired);
|
||||
return Err(Error::BrowserReopenConfirmationRequired);
|
||||
}
|
||||
Ok(())
|
||||
}
|
||||
|
||||
async fn scoped_confirm_merge_request(
|
||||
State(api): State<WorkspaceApi>,
|
||||
headers: HeaderMap,
|
||||
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
|
||||
Json(input): Json<ConfirmMergeRequestRequest>,
|
||||
) -> ApiResult<Json<merge_request::MergeRequest>> {
|
||||
let workspace_id = parse_workspace_id(&workspace_id)?;
|
||||
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 {
|
||||
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))
|
||||
}
|
||||
|
||||
async fn scoped_close_ticket_record(
|
||||
State(api): State<WorkspaceApi>,
|
||||
AxumPath((workspace_id, id)): AxumPath<(String, String)>,
|
||||
@@ -11128,6 +11350,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,
|
||||
}],
|
||||
)
|
||||
}
|
||||
})
|
||||
}
|
||||
|
||||
@@ -11335,9 +11572,7 @@ impl ApiError {
|
||||
impl IntoResponse for ApiError {
|
||||
fn into_response(self) -> Response {
|
||||
let status = match &self.error {
|
||||
Error::BrowserMergeConfirmationRequired | Error::BrowserReopenConfirmationRequired => {
|
||||
StatusCode::FORBIDDEN
|
||||
}
|
||||
Error::BrowserReopenConfirmationRequired => StatusCode::FORBIDDEN,
|
||||
Error::TicketAssignmentConflict(_)
|
||||
| Error::WorkdirAttachmentConflict(_)
|
||||
| Error::WorkspaceConfigConflict(_) => StatusCode::CONFLICT,
|
||||
@@ -11501,14 +11736,14 @@ mod tests {
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn merge_confirmation_rejects_api_token_actor_before_session_resolution() {
|
||||
fn reopen_confirmation_rejects_api_token_actor_before_session_resolution() {
|
||||
let mut headers = HeaderMap::new();
|
||||
headers.insert("authorization", "Bearer api-token".parse().unwrap());
|
||||
assert!(matches!(
|
||||
reject_non_browser_merge_auth(&headers),
|
||||
Err(Error::BrowserMergeConfirmationRequired)
|
||||
reject_non_browser_reopen_auth(&headers),
|
||||
Err(Error::BrowserReopenConfirmationRequired)
|
||||
));
|
||||
assert!(reject_non_browser_merge_auth(&HeaderMap::new()).is_ok());
|
||||
assert!(reject_non_browser_reopen_auth(&HeaderMap::new()).is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -12157,6 +12392,40 @@ mod tests {
|
||||
async fn merge_request_completion_endpoint_rejects_coder_and_accepts_orchestrator() {
|
||||
let workspace = tempfile::tempdir().unwrap();
|
||||
init_clean_git_workspace(workspace.path());
|
||||
let git_value = |args: &[&str]| {
|
||||
let output = std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(workspace.path())
|
||||
.args(args)
|
||||
.output()
|
||||
.unwrap();
|
||||
assert!(output.status.success());
|
||||
String::from_utf8(output.stdout).unwrap().trim().to_string()
|
||||
};
|
||||
let target_commit = git_value(&["rev-parse", "HEAD"]);
|
||||
let target_ref = git_value(&["symbolic-ref", "HEAD"]);
|
||||
std::fs::write(workspace.path().join("README.md"), "merge source\n").unwrap();
|
||||
for args in [&["add", "README.md"][..], &["commit", "-m", "source"][..]] {
|
||||
assert!(
|
||||
std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(workspace.path())
|
||||
.args(args)
|
||||
.status()
|
||||
.unwrap()
|
||||
.success()
|
||||
);
|
||||
}
|
||||
let source_commit = git_value(&["rev-parse", "HEAD"]);
|
||||
assert!(
|
||||
std::process::Command::new("git")
|
||||
.arg("-C")
|
||||
.arg(workspace.path())
|
||||
.args(["reset", "--hard", &target_commit])
|
||||
.status()
|
||||
.unwrap()
|
||||
.success()
|
||||
);
|
||||
let api = test_api(workspace.path()).await;
|
||||
let workspace_id = api.config.workspace_id.clone();
|
||||
let backend = browser_ticket_backend(&api).unwrap();
|
||||
@@ -12193,12 +12462,13 @@ mod tests {
|
||||
merge_request_id: "MR-server-completion".into(),
|
||||
ticket_id: ticket.id.clone(),
|
||||
repository_id: TEST_REPOSITORY_ID.into(),
|
||||
target_ref_selector: target_ref.clone(),
|
||||
revision: merge_request::MergeRequestRevision {
|
||||
revision_id: "V1".into(),
|
||||
ordinal: 1,
|
||||
base_commit: "base".into(),
|
||||
head_commit: "head".into(),
|
||||
head_tree: "tree".into(),
|
||||
base_commit: target_commit.clone(),
|
||||
head_commit: source_commit.clone(),
|
||||
|
||||
diff_digest: "sha256:diff".into(),
|
||||
changed_paths: vec!["src/lib.rs".into()],
|
||||
summary: "approved revision".into(),
|
||||
@@ -12258,6 +12528,11 @@ mod tests {
|
||||
let request = || CompleteMergeRequestRequest {
|
||||
operation_id: "complete-operation".into(),
|
||||
expected_revision_id: "V1".into(),
|
||||
target_commit: target_commit.clone(),
|
||||
source_commit: source_commit.clone(),
|
||||
result_commit: source_commit.clone(),
|
||||
strategy: merge_request::MergeStrategy::FastForward,
|
||||
resolution: merge_request::MergeResolution::None,
|
||||
};
|
||||
let coder_error = scoped_complete_merge_request(
|
||||
State(api.clone()),
|
||||
@@ -12298,6 +12573,35 @@ mod tests {
|
||||
.workflow_state,
|
||||
TicketWorkflowState::Done
|
||||
);
|
||||
assert_eq!(
|
||||
api.repository_reader()
|
||||
.observe_merge_target(TEST_REPOSITORY_ID, Some(&target_ref))
|
||||
.unwrap()
|
||||
.commit,
|
||||
source_commit
|
||||
);
|
||||
let merged = mr_store.show_for_ticket(&ticket.id).unwrap().unwrap();
|
||||
assert_eq!(
|
||||
merged.merged_target_commit.as_deref(),
|
||||
Some(target_commit.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
merged.merged_result_commit.as_deref(),
|
||||
Some(source_commit.as_str())
|
||||
);
|
||||
assert_eq!(
|
||||
merged.merge_strategy,
|
||||
Some(merge_request::MergeStrategy::FastForward)
|
||||
);
|
||||
let Json(replayed) = scoped_complete_merge_request(
|
||||
State(api.clone()),
|
||||
worker_headers(&orchestrator),
|
||||
AxumPath((api.config.workspace_id.clone(), ticket.id.clone())),
|
||||
Json(request()),
|
||||
)
|
||||
.await
|
||||
.unwrap();
|
||||
assert!(replayed.replayed);
|
||||
let conn = rusqlite::Connection::open(&api.config.database_path).unwrap();
|
||||
let actor: String = conn
|
||||
.query_row(
|
||||
@@ -16755,8 +17059,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,
|
||||
|
||||
@@ -248,8 +248,8 @@ Deno.test("workspace Tickets surface provides Kanban and lifecycle controls", as
|
||||
ticketDetailLoad.includes("/repositories") &&
|
||||
ticketDetailPage.includes('mutate("state", "/state"') &&
|
||||
ticketDetailPage.includes('mutate("queue", "/queue"') &&
|
||||
ticketDetailPage.includes("/merge-request/merge") &&
|
||||
ticketDetailPage.includes("explicit_confirmation: true") &&
|
||||
!ticketDetailPage.includes("/merge-request/merge") &&
|
||||
ticketDetailPage.includes("merged_result_commit") &&
|
||||
!ticketDetailPage.includes('mutate("review", "/review"') &&
|
||||
ticketDetailPage.includes('mutate("close", "/close"') &&
|
||||
ticketDetailPage.includes("ticketWorkerLaunchHref") &&
|
||||
|
||||
@@ -20,8 +20,18 @@
|
||||
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;
|
||||
merged_revision_id?: string | null;
|
||||
merged_target_commit?: string | null;
|
||||
merged_result_commit?: string | null;
|
||||
merge_strategy?: "fast_forward" | "merge" | null;
|
||||
merge_resolution?: "none" | "clean" | "conflicts_resolved" | null;
|
||||
merged_by_runtime_id?: string | null;
|
||||
merged_by_worker_id?: string | null;
|
||||
merged_at?: string | null;
|
||||
};
|
||||
|
||||
@@ -55,7 +65,6 @@
|
||||
let transitionReason = $state("");
|
||||
let threadRole = $state("comment");
|
||||
let threadBody = $state("");
|
||||
let confirmMerge = $state(false);
|
||||
let resolution = $state("");
|
||||
let busy = $state<string | null>(null);
|
||||
let errorMessage = $state<string | null>(null);
|
||||
@@ -145,29 +154,6 @@
|
||||
) threadBody = "";
|
||||
}
|
||||
|
||||
async function mergeConfirmedRevision() {
|
||||
if (!mergeRequest || !confirmMerge || busy) return;
|
||||
busy = "merge";
|
||||
errorMessage = null;
|
||||
try {
|
||||
mergeRequest = await workspaceApiJsonWithBody<MergeRequestDetail>(
|
||||
`${ticketPath}/merge-request/merge`,
|
||||
{
|
||||
method: "POST",
|
||||
body: JSON.stringify({
|
||||
expected_revision_id: mergeRequest.current_revision.revision_id,
|
||||
explicit_confirmation: true,
|
||||
}),
|
||||
},
|
||||
);
|
||||
confirmMerge = false;
|
||||
} catch (error) {
|
||||
errorMessage = error instanceof Error ? error.message : String(error);
|
||||
} finally {
|
||||
busy = null;
|
||||
}
|
||||
}
|
||||
|
||||
async function closeTicket(event: SubmitEvent) {
|
||||
event.preventDefault();
|
||||
if (!resolution.trim()) return;
|
||||
@@ -368,17 +354,28 @@
|
||||
<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.merged_result_commit}
|
||||
<p>
|
||||
Final merge · {mergeRequest.merge_strategy} / {mergeRequest.merge_resolution}
|
||||
</p>
|
||||
<p>
|
||||
Target before <code>{mergeRequest.merged_target_commit}</code> · result
|
||||
<code>{mergeRequest.merged_result_commit}</code>
|
||||
</p>
|
||||
<p>
|
||||
Revision <code>{mergeRequest.merged_revision_id}</code> · completed by
|
||||
<code>{mergeRequest.merged_by_runtime_id}/{mergeRequest.merged_by_worker_id}</code>
|
||||
</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>
|
||||
<button class="workspace-primary-button" type="button" disabled={!confirmMerge || busy !== null} onclick={mergeConfirmedRevision}>Confirm merge</button>
|
||||
{/if}
|
||||
{:else}
|
||||
<p class="workspace-empty-copy">The assigned Coder has not opened a Merge Request.</p>
|
||||
{/if}
|
||||
|
||||
Reference in New Issue
Block a user