feat: add merge request review authority

This commit is contained in:
2026-08-11 20:21:37 +09:00
parent 7f0d025312
commit 1814c35701
29 changed files with 3069 additions and 555 deletions
+14
View File
@@ -0,0 +1,14 @@
[package]
name = "merge-request"
version = "0.1.0"
edition.workspace = true
license.workspace = true
[dependencies]
rusqlite.workspace = true
serde = { workspace = true, features = ["derive"] }
sha2.workspace = true
thiserror.workspace = true
[dev-dependencies]
tempfile.workspace = true
File diff suppressed because it is too large Load Diff
+404
View File
@@ -0,0 +1,404 @@
use merge_request::*;
use rusqlite::{Connection, params};
use tempfile::TempDir;
fn setup() -> (TempDir, SqliteMergeRequestStore) {
let dir = tempfile::tempdir().unwrap();
let path = dir.path().join("server.db");
let conn = Connection::open(&path).unwrap();
conn.execute_batch(r#"
PRAGMA foreign_keys=ON;
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 typed_ticket_events(workspace_id TEXT NOT NULL,ticket_id TEXT NOT NULL,event_index INTEGER NOT NULL,kind TEXT NOT NULL,author TEXT,at TEXT,status TEXT,from_state TEXT,to_state TEXT,heading TEXT,body TEXT,PRIMARY KEY(workspace_id,ticket_id,event_index));
CREATE TABLE typed_ticket_event_attributes(workspace_id TEXT NOT NULL,ticket_id TEXT NOT NULL,event_index INTEGER NOT NULL,key TEXT NOT NULL,value TEXT NOT NULL,PRIMARY KEY(workspace_id,ticket_id,event_index,key));
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 ticket_current_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));
"#).unwrap();
for ws in ["ws-a", "ws-b"] {
conn.execute("INSERT INTO repositories VALUES(?1,'repo')", params![ws])
.unwrap();
conn.execute(
"INSERT INTO typed_tickets VALUES(?1,'T1','inprogress',1,'t0')",
params![ws],
)
.unwrap();
conn.execute(
"INSERT INTO ticket_worker_assignments VALUES(?1,'T1','A1','R1','W1')",
params![ws],
)
.unwrap();
conn.execute(
"INSERT INTO ticket_current_worker_assignments VALUES(?1,'T1','A1','R1','W1')",
params![ws],
)
.unwrap();
}
drop(conn);
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}"),
assignment_id: "A1".into(),
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"),
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) {
store
.register_reviewer_child_session(RegisterReviewerChildSession {
parent_runtime_id: "R1".into(),
parent_worker_id: "W1".into(),
child_session_id: child.into(),
now: "t".into(),
})
.unwrap();
store
.register_review_attempt(RegisterReviewAttempt {
attempt_id: id.into(),
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(),
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",
..
})
));
}
#[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);
}
}
#[test]
fn request_changes_new_revision_resets_and_exact_completion_replay_converges() {
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
);
store
.add_revision(AddRevision {
ticket_id: "T1".into(),
expected_current_revision_id: "V1".into(),
revision: revision("V2", 2, "h2"),
authenticated_runtime_id: "R1".into(),
authenticated_worker_id: "W1".into(),
now: "t2".into(),
})
.unwrap();
assert_eq!(
store.show_for_ticket("T1").unwrap().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(),
assignment_id: "A1".into(),
authenticated_runtime_id: "R1".into(),
authenticated_worker_id: "W1".into(),
now: "tc".into(),
};
let first = store.complete(input.clone()).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!(merged.state, MergeRequestState::Merged);
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)
)
.unwrap(),
"done"
);
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
);
}
#[test]
fn spoof_self_approval_replay_and_cross_workspace_are_rejected() {
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(),
assignment_id: "A1".into(),
authenticated_runtime_id: "R1".into(),
authenticated_worker_id: "W1".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(),
];
assert_eq!(
outcomes.iter().filter(|outcome| !outcome.replayed).count(),
1
);
assert_eq!(
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_assignment_takeover_are_fenced() {
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(),
ticket_id: "T1".into(),
expected_revision_id: "V1".into(),
assignment_id: "A1".into(),
authenticated_runtime_id: "R1".into(),
authenticated_worker_id: "W1".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();
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.expected_revision_id = "other".into();
assert!(matches!(
store.complete(input),
Err(MergeRequestError::OperationConflict)
));
}
+16 -149
View File
@@ -295,7 +295,6 @@ pub enum TicketEventKind {
Plan,
Decision,
ImplementationReport,
Review,
StateChanged,
IntakeSummary,
StatusChanged,
@@ -311,7 +310,6 @@ impl TicketEventKind {
Self::Plan => "plan",
Self::Decision => "decision",
Self::ImplementationReport => "implementation_report",
Self::Review => "review",
Self::StateChanged => "state_changed",
Self::IntakeSummary => "intake_summary",
Self::StatusChanged => "status_changed",
@@ -327,7 +325,6 @@ impl TicketEventKind {
Self::Plan => "Plan".to_string(),
Self::Decision => "Decision".to_string(),
Self::ImplementationReport => "Implementation report".to_string(),
Self::Review => "Review".to_string(),
Self::StateChanged => "State changed".to_string(),
Self::IntakeSummary => "Intake summary".to_string(),
Self::StatusChanged => "Status changed".to_string(),
@@ -345,7 +342,7 @@ impl From<&str> for TicketEventKind {
"plan" => Self::Plan,
"decision" => Self::Decision,
"implementation_report" => Self::ImplementationReport,
"review" => Self::Review,
"review" => Self::Comment,
"state_changed" => Self::StateChanged,
"intake_summary" => Self::IntakeSummary,
"status_changed" => Self::StatusChanged,
@@ -355,42 +352,6 @@ impl From<&str> for TicketEventKind {
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
#[serde(rename_all = "snake_case")]
pub enum TicketReviewResult {
Approve,
RequestChanges,
Other(String),
}
impl TicketReviewResult {
pub fn as_str(&self) -> &str {
match self {
Self::Approve => "approve",
Self::RequestChanges => "request_changes",
Self::Other(value) => value.as_str(),
}
}
fn heading(&self) -> String {
match self {
Self::Approve => "Review: approve".to_string(),
Self::RequestChanges => "Review: request changes".to_string(),
Self::Other(value) => format!("Review: {value}"),
}
}
}
impl From<&str> for TicketReviewResult {
fn from(value: &str) -> Self {
match value {
"approve" => Self::Approve,
"request_changes" => Self::RequestChanges,
other => Self::Other(other.to_string()),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct TicketReference {
pub kind: String,
@@ -461,31 +422,6 @@ impl TicketIntakeSummary {
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct TicketReview {
pub result: TicketReviewResult,
pub author: Option<String>,
pub body: MarkdownText,
}
impl TicketReview {
pub fn approve(body: impl Into<MarkdownText>) -> Self {
Self {
result: TicketReviewResult::Approve,
author: None,
body: body.into(),
}
}
pub fn request_changes(body: impl Into<MarkdownText>) -> Self {
Self {
result: TicketReviewResult::RequestChanges,
author: None,
body: body.into(),
}
}
}
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
pub struct NewTicket {
pub title: String,
@@ -1578,7 +1514,6 @@ pub trait TicketBackend {
change: TicketStateChange,
) -> Result<()>;
fn queue_ready(&self, id: TicketIdOrSlug, queued_by: &str) -> Result<()>;
fn review(&self, id: TicketIdOrSlug, review: TicketReview) -> Result<()>;
fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()>;
fn add_ticket_relation(
&self,
@@ -1656,10 +1591,6 @@ pub enum TicketBackendOperation {
id: TicketIdOrSlug,
queued_by: String,
},
Review {
id: TicketIdOrSlug,
review: TicketReview,
},
Close {
id: TicketIdOrSlug,
resolution: MarkdownText,
@@ -1763,10 +1694,6 @@ where
backend.queue_ready(id, &queued_by)?;
TicketBackendOperationResult::Unit
}
TicketBackendOperation::Review { id, review } => {
backend.review(id, review)?;
TicketBackendOperationResult::Unit
}
TicketBackendOperation::Close { id, resolution } => {
backend.close(id, resolution)?;
TicketBackendOperationResult::Unit
@@ -3201,34 +3128,6 @@ impl TicketBackend for SqliteTicketBackend {
})
}
fn review(&self, id: TicketIdOrSlug, review: TicketReview) -> Result<()> {
self.with_write(|conn| {
let ticket_id = self.resolve_ticket_id(conn, id)?;
let at = now_utc();
let mut attributes = BTreeMap::new();
attributes.insert("result".to_string(), review.result.as_str().to_string());
self.insert_event(
conn,
&ticket_id,
&TicketEvent {
kind: TicketEventKind::Review,
author: Some(review.author.unwrap_or_else(default_author)),
at: Some(at.clone()),
status: Some(review.result.as_str().to_string()),
from: None,
to: None,
reason: None,
state_field: None,
heading: Some(review.result.heading()),
body: review.body,
references: Vec::new(),
attributes,
},
)?;
self.touch_ticket(conn, &ticket_id, &at)
})
}
fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()> {
self.with_write(|conn| {
let ticket_id = self.resolve_ticket_id(conn, id)?;
@@ -3804,21 +3703,6 @@ impl TicketBackend for LocalTicketBackend {
)
}
fn review(&self, id: TicketIdOrSlug, review: TicketReview) -> Result<()> {
let _lock = self.acquire_lock()?;
let dir = self.find_ticket_dir(&id)?;
let author = review.author.unwrap_or_else(default_author);
self.append_thread_event(
&dir,
"review",
&review.result.heading(),
&author,
Some(review.result.as_str()),
&[],
&review.body,
)
}
fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> Result<()> {
let _lock = self.acquire_lock()?;
self.ensure_backend_dirs()?;
@@ -5337,7 +5221,8 @@ fn parse_thread(path: &Path) -> Result<Vec<TicketEvent>> {
.strip_prefix("<!-- ")
.and_then(|v| v.strip_suffix(" -->"))
{
let attrs = parse_event_comment(comment);
let mut attrs = parse_event_comment(comment);
let legacy_review = attrs.get("event").is_some_and(|value| value == "review");
let kind = attrs
.get("event")
.map(|value| TicketEventKind::from(value.as_str()))
@@ -5369,11 +5254,22 @@ fn parse_thread(path: &Path) -> Result<Vec<TicketEvent>> {
while body.ends_with('\n') {
body.pop();
}
if legacy_review {
heading = Some("Legacy review (non-authoritative)".to_string());
attrs.remove("status");
attrs.remove("result");
attrs.insert("event".to_string(), "comment".to_string());
attrs.insert("legacy_event_kind".to_string(), "review".to_string());
}
events.push(TicketEvent {
kind,
author: attrs.get("author").cloned(),
at: attrs.get("at").cloned(),
status: attrs.get("status").cloned(),
status: if legacy_review {
None
} else {
attrs.get("status").cloned()
},
from: attrs.get("from").cloned(),
to: attrs.get("to").cloned(),
reason: attrs.get("reason").cloned(),
@@ -6379,12 +6275,6 @@ state: planning
NewTicketEvent::new(TicketEventKind::Comment, "Imported into SQLite."),
)
.unwrap();
backend
.review(
TicketIdOrSlug::Id(created.id.clone()),
TicketReview::approve("Looks good."),
)
.unwrap();
backend
.close(
TicketIdOrSlug::Id(created.id.clone()),
@@ -6404,13 +6294,6 @@ state: planning
assert!(ticket.events.iter().any(|event| {
event.kind == TicketEventKind::Comment && event.body.0.contains("Imported into SQLite")
}));
assert!(
ticket
.events
.iter()
.any(|event| event.kind == TicketEventKind::Review
&& event.body.0.contains("Looks good"))
);
assert!(
ticket
.resolution
@@ -6524,7 +6407,7 @@ state: planning
}
#[test]
fn add_event_review_status_and_close_preserve_local_layout() {
fn add_event_status_and_close_preserve_local_layout() {
let tmp = TempDir::new().unwrap();
let backend = backend(&tmp);
let ticket = backend.create(NewTicket::new("Flow Ticket")).unwrap();
@@ -6534,12 +6417,6 @@ state: planning
NewTicketEvent::new(TicketEventKind::Plan, "Implementation plan."),
)
.unwrap();
backend
.review(
TicketIdOrSlug::Id(ticket.id.clone()),
TicketReview::approve("Looks good."),
)
.unwrap();
let mut summary = TicketIntakeSummary::new("Ready for queue.");
summary.author = Some("test".to_string());
let mut change = TicketStateChange::new(
@@ -6563,8 +6440,6 @@ state: planning
let closed_dir = tmp.path().join("tickets").join(&ticket.id);
assert!(closed_dir.join("resolution.md").exists());
let thread = fs::read_to_string(closed_dir.join("thread.md")).unwrap();
assert!(thread.contains("<!-- event: review"));
assert!(thread.contains("status: approve"));
assert!(thread.contains("<!-- event: close"));
let report = backend.doctor().unwrap();
assert!(report.is_ok(), "{:?}", report.diagnostics);
@@ -6592,14 +6467,6 @@ state: planning
));
assert_eq!(fs::read_to_string(&thread_path).unwrap(), original);
let mut review = TicketReview::approve("This must not append either.");
review.author = Some("bad-->author".into());
assert!(matches!(
backend.review(TicketIdOrSlug::Id(ticket.id.clone()), review),
Err(TicketError::Conflict(_))
));
assert_eq!(fs::read_to_string(&thread_path).unwrap(), original);
let invalid_kind = NewTicketEvent::new(
TicketEventKind::Other("bad\nevent".into()),
"Invalid event kind.",
+62 -5
View File
@@ -7,7 +7,7 @@ use crate::{Result, TicketError, sqlite_err};
const MIGRATION_TABLE: &str = "ticket_schema_migrations";
const MAX_SCHEMA_DIAGNOSTICS: usize = 32;
pub const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 2;
pub const LATEST_SQLITE_TICKET_SCHEMA_VERSION: i64 = 3;
#[derive(Clone, Copy)]
struct Migration {
@@ -27,6 +27,11 @@ const MIGRATIONS: &[Migration] = &[
name: "add_ticket_repository_target",
apply: add_ticket_repository_target,
},
Migration {
version: 3,
name: "convert_legacy_reviews_to_comments",
apply: retire_legacy_ticket_review_events,
},
];
#[derive(Clone, Copy)]
@@ -475,6 +480,33 @@ fn add_ticket_repository_target(connection: &Connection) -> Result<()> {
add_column_if_missing(connection, "typed_tickets", "ref_selector", "TEXT")
}
fn retire_legacy_ticket_review_events(connection: &Connection) -> Result<()> {
// Historical prose remains visible for audit, but it is explicitly converted to a
// non-authoritative comment. Approval authority now lives only in Merge Requests.
connection
.execute_batch(
r#"
INSERT OR REPLACE INTO typed_ticket_event_attributes
(workspace_id, ticket_id, event_index, key, value)
SELECT workspace_id, ticket_id, event_index, 'legacy_event_kind', 'review'
FROM typed_ticket_events WHERE kind = 'review';
UPDATE typed_ticket_events
SET kind = 'comment', status = NULL, heading = 'Legacy review (non-authoritative)'
WHERE kind = 'review';
DELETE FROM typed_ticket_event_attributes
WHERE key IN ('result', 'review_result', 'status')
AND EXISTS (
SELECT 1 FROM typed_ticket_events event
WHERE event.workspace_id = typed_ticket_event_attributes.workspace_id
AND event.ticket_id = typed_ticket_event_attributes.ticket_id
AND event.event_index = typed_ticket_event_attributes.event_index
AND event.heading = 'Legacy review (non-authoritative)'
);
"#,
)
.map_err(sqlite_err)
}
fn add_column_if_missing(
connection: &Connection,
table: &str,
@@ -809,10 +841,10 @@ mod tests {
verify_sqlite_ticket_schema(&connection).unwrap();
let versions = load_applied_migrations(&connection).unwrap();
assert_eq!(versions.len(), 2);
assert_eq!(versions.len(), 3);
assert_eq!(
versions.get(&LATEST_SQLITE_TICKET_SCHEMA_VERSION),
Some(&"add_ticket_repository_target".to_string())
Some(&"convert_legacy_reviews_to_comments".to_string())
);
}
@@ -957,7 +989,7 @@ mod tests {
.to_string()
.contains("unsupported Ticket schema migration version 99")
);
assert_eq!(load_applied_migrations(&connection).unwrap().len(), 3);
assert_eq!(load_applied_migrations(&connection).unwrap().len(), 4);
}
#[test]
@@ -1050,6 +1082,31 @@ mod tests {
assert!(!migration_table_exists);
}
#[test]
fn legacy_review_upgrade_preserves_prose_as_non_authoritative_comment() {
let connection = Connection::open_in_memory().unwrap();
migrate_sqlite_ticket_schema(&connection).unwrap();
connection.execute("INSERT INTO typed_tickets (workspace_id,ticket_id,slug,title,status,kind,priority,body,workflow_state,workflow_state_explicit) VALUES ('workspace-1','ticket-1','ticket-1','title','open','task','medium','body','inprogress',1)",[]).unwrap();
connection.execute("INSERT INTO typed_ticket_events (workspace_id,ticket_id,event_index,kind,author,at,status,heading,body) VALUES ('workspace-1','ticket-1',0,'review','reviewer','2026-08-11T00:00:00Z','approve','Review','legacy evidence')",[]).unwrap();
connection.execute("INSERT INTO typed_ticket_event_attributes (workspace_id,ticket_id,event_index,key,value) VALUES ('workspace-1','ticket-1',0,'result','approve')",[]).unwrap();
connection
.execute("DELETE FROM ticket_schema_migrations WHERE version=3", [])
.unwrap();
migrate_sqlite_ticket_schema(&connection).unwrap();
let (kind,status,heading,body):(String,Option<String>,Option<String>,Option<String>)=connection.query_row("SELECT kind,status,heading,body FROM typed_ticket_events WHERE workspace_id='workspace-1' AND ticket_id='ticket-1' AND event_index=0",[],|row|Ok((row.get(0)?,row.get(1)?,row.get(2)?,row.get(3)?))).unwrap();
assert_eq!(kind, "comment");
assert_eq!(status, None);
assert_eq!(
heading.as_deref(),
Some("Legacy review (non-authoritative)")
);
assert_eq!(body.as_deref(), Some("legacy evidence"));
let attributes:i64=connection.query_row("SELECT COUNT(*) FROM typed_ticket_event_attributes WHERE workspace_id='workspace-1' AND ticket_id='ticket-1'",[],|row|row.get(0)).unwrap();
assert_eq!(attributes, 1);
let legacy:String=connection.query_row("SELECT value FROM typed_ticket_event_attributes WHERE workspace_id='workspace-1' AND ticket_id='ticket-1' AND key='legacy_event_kind'",[],|row|row.get(0)).unwrap();
assert_eq!(legacy, "review");
}
#[test]
fn concurrent_migrators_converge_on_one_version_history() {
let directory = tempdir().unwrap();
@@ -1072,6 +1129,6 @@ mod tests {
let connection = Connection::open(database).unwrap();
verify_sqlite_ticket_schema(&connection).unwrap();
assert_eq!(load_applied_migrations(&connection).unwrap().len(), 2);
assert_eq!(load_applied_migrations(&connection).unwrap().len(), 3);
}
}
+5 -90
View File
@@ -17,8 +17,7 @@ use crate::{
Result as TicketResult, Ticket, TicketBackend, TicketBodyReplacement, TicketDoctorDiagnostic,
TicketDoctorReport, TicketDoctorSeverity, TicketError, TicketEventKind, TicketIdOrSlug,
TicketIntakeSummary, TicketListState, TicketRef, TicketRelation, TicketRelationKind,
TicketRelationView, TicketReview, TicketReviewResult, TicketStateChange, TicketSummary,
TicketWorkflowState, default_author,
TicketRelationView, TicketStateChange, TicketSummary, TicketWorkflowState, default_author,
};
const DEFAULT_LIST_LIMIT: usize = 50;
@@ -34,7 +33,7 @@ const MAX_BODY_MAX_BYTES: usize = 64 * 1024;
const DEFAULT_DIAGNOSTIC_LIMIT: usize = 100;
const MAX_DIAGNOSTIC_LIMIT: usize = 500;
pub const TICKET_BASE_TOOL_NAMES: [&str; 15] = [
pub const TICKET_BASE_TOOL_NAMES: [&str; 14] = [
"TicketCreate",
"TicketEditItem",
"TicketList",
@@ -43,7 +42,6 @@ pub const TICKET_BASE_TOOL_NAMES: [&str; 15] = [
"TicketPlan",
"TicketDecision",
"TicketImplementationReport",
"TicketReview",
"TicketIntakeReady",
"TicketQueue",
"TicketWorkflowState",
@@ -69,7 +67,7 @@ pub const TICKET_ORCHESTRATION_TOOL_NAMES: [&str; 4] = [
pub const TICKET_ORCHESTRATION_READ_ONLY_TOOL_NAMES: [&str; 2] =
["TicketRelationQuery", "TicketOrchestrationPlanQuery"];
pub const TICKET_TOOL_NAMES: [&str; 19] = [
pub const TICKET_TOOL_NAMES: [&str; 18] = [
"TicketCreate",
"TicketEditItem",
"TicketList",
@@ -78,7 +76,6 @@ pub const TICKET_TOOL_NAMES: [&str; 19] = [
"TicketPlan",
"TicketDecision",
"TicketImplementationReport",
"TicketReview",
"TicketIntakeReady",
"TicketQueue",
"TicketWorkflowState",
@@ -100,14 +97,13 @@ pub const TICKET_READ_ONLY_TOOL_NAMES: [&str; 6] = [
"TicketOrchestrationPlanQuery",
];
pub const TICKET_MUTATING_TOOL_NAMES: [&str; 13] = [
pub const TICKET_MUTATING_TOOL_NAMES: [&str; 12] = [
"TicketCreate",
"TicketEditItem",
"TicketComment",
"TicketPlan",
"TicketDecision",
"TicketImplementationReport",
"TicketReview",
"TicketIntakeReady",
"TicketQueue",
"TicketWorkflowState",
@@ -134,8 +130,6 @@ const PLAN_DESCRIPTION: &str = "Append a typed Ticket plan event. `body` is Mark
const DECISION_DESCRIPTION: &str = "Append a typed Ticket decision event. `body` is Markdown.";
const IMPLEMENTATION_REPORT_DESCRIPTION: &str =
"Append a typed Ticket implementation_report event. `body` is Markdown.";
const REVIEW_DESCRIPTION: &str = "Append a Ticket review event. `result` must be `approve` or \
`request_changes`; `body` is Markdown. Writes stay inside the configured Ticket backend root.";
const INTAKE_READY_DESCRIPTION: &str = "Mark an existing Ticket planning lane ready through the typed \
Ticket backend. The tool appends a bounded `intake_summary`, appends a typed `state_changed` event \
for `state`, and transitions state to `ready`.";
@@ -175,7 +169,6 @@ fn base_tool_description(name: &str) -> &'static str {
"TicketPlan" => PLAN_DESCRIPTION,
"TicketDecision" => DECISION_DESCRIPTION,
"TicketImplementationReport" => IMPLEMENTATION_REPORT_DESCRIPTION,
"TicketReview" => REVIEW_DESCRIPTION,
"TicketIntakeReady" => INTAKE_READY_DESCRIPTION,
"TicketQueue" => QUEUE_DESCRIPTION,
"TicketWorkflowState" => WORKFLOW_STATE_DESCRIPTION,
@@ -319,10 +312,6 @@ impl TicketBackend for TicketToolBackend {
self.backend.queue_ready(id, queued_by)
}
fn review(&self, id: TicketIdOrSlug, review: TicketReview) -> TicketResult<()> {
self.backend.review(id, review)
}
fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> TicketResult<()> {
self.backend.close(id, resolution)
}
@@ -554,23 +543,6 @@ struct TicketThreadEventParams {
body: String,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
#[serde(rename_all = "snake_case")]
enum TicketReviewResultParam {
Approve,
RequestChanges,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
struct TicketReviewParams {
/// Ticket id.
ticket: String,
/// Review result: `approve` or `request_changes`.
result: TicketReviewResultParam,
/// Markdown review body.
body: String,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
struct TicketIntakeReadyParams {
/// Ticket id.
@@ -839,11 +811,6 @@ struct TicketImplementationReportTool {
backend: TicketToolBackend,
}
#[derive(Clone)]
struct TicketReviewTool {
backend: TicketToolBackend,
}
#[derive(Clone)]
struct TicketIntakeReadyTool {
backend: TicketToolBackend,
@@ -1117,34 +1084,6 @@ impl_ticket_thread_event_tool!(
TicketEventKind::ImplementationReport
);
#[async_trait]
impl Tool for TicketReviewTool {
async fn execute(
&self,
input_json: &str,
_ctx: llm_engine::tool::ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
let params: TicketReviewParams = parse_input("TicketReview", input_json)?;
let result = match params.result {
TicketReviewResultParam::Approve => TicketReviewResult::Approve,
TicketReviewResultParam::RequestChanges => TicketReviewResult::RequestChanges,
};
let result_str = result.as_str().to_string();
let review = TicketReview {
result,
author: None,
body: MarkdownText::new(params.body),
};
self.backend
.review(TicketIdOrSlug::Query(params.ticket.clone()), review)
.map_err(|error| backend_error("TicketReview", error))?;
Ok(json_output(
format!("Appended {result_str} review to ticket {}", params.ticket),
json!({ "ticket": params.ticket, "review": result_str, "ok": true }),
))
}
}
#[async_trait]
impl Tool for TicketIntakeReadyTool {
async fn execute(
@@ -1731,7 +1670,6 @@ fn input_schema(name: &str) -> Value {
"TicketComment" | "TicketPlan" | "TicketDecision" | "TicketImplementationReport" => {
serde_json::to_value(schemars::schema_for!(TicketThreadEventParams))
}
"TicketReview" => serde_json::to_value(schemars::schema_for!(TicketReviewParams)),
"TicketIntakeReady" => serde_json::to_value(schemars::schema_for!(TicketIntakeReadyParams)),
"TicketQueue" => serde_json::to_value(schemars::schema_for!(TicketQueueParams)),
"TicketWorkflowState" => {
@@ -1777,7 +1715,6 @@ impl_from_backend!(TicketCommentTool);
impl_from_backend!(TicketPlanTool);
impl_from_backend!(TicketDecisionTool);
impl_from_backend!(TicketImplementationReportTool);
impl_from_backend!(TicketReviewTool);
impl_from_backend!(TicketIntakeReadyTool);
impl_from_backend!(TicketQueueTool);
impl_from_backend!(TicketWorkflowStateTool);
@@ -1804,7 +1741,6 @@ pub fn ticket_tools(backend: impl Into<TicketToolBackend>) -> Vec<ToolDefinition
"TicketImplementationReport",
backend.clone(),
),
tool_definition::<TicketReviewTool>("TicketReview", backend.clone()),
tool_definition::<TicketIntakeReadyTool>("TicketIntakeReady", backend.clone()),
tool_definition::<TicketQueueTool>("TicketQueue", backend.clone()),
tool_definition::<TicketWorkflowStateTool>("TicketWorkflowState", backend.clone()),
@@ -1880,7 +1816,6 @@ mod tests {
"TicketPlan",
"TicketDecision",
"TicketImplementationReport",
"TicketReview",
"TicketIntakeReady",
"TicketQueue",
"TicketWorkflowState",
@@ -2373,12 +2308,11 @@ mod tests {
}
#[tokio::test]
async fn ticket_tools_comment_review_state_and_close_are_doctor_clean() {
async fn ticket_tools_report_state_and_close_are_doctor_clean() {
let temp = TempDir::new().unwrap();
let backend = backend(&temp);
let created = backend.create(NewTicket::new("Flow Tool")).unwrap();
let report = tool_by_name(backend.clone(), "TicketImplementationReport");
let review = tool_by_name(backend.clone(), "TicketReview");
let close = tool_by_name(backend.clone(), "TicketClose");
let doctor = tool_by_name(backend.clone(), "TicketDoctor");
@@ -2393,18 +2327,6 @@ mod tests {
)
.await
.unwrap();
review
.execute(
&json!({
"ticket": created.id.clone(),
"result": "approve",
"body": "Looks good."
})
.to_string(),
Default::default(),
)
.await
.unwrap();
close
.execute(
&json!({ "ticket": created.id, "resolution": "Done via TicketClose.\n" })
@@ -2427,12 +2349,6 @@ mod tests {
.iter()
.any(|event| event.kind == TicketEventKind::ImplementationReport)
);
assert!(
closed
.events
.iter()
.any(|event| event.kind == TicketEventKind::Review)
);
assert!(
closed
.events
@@ -2852,7 +2768,6 @@ mod tests {
"TicketPlan",
"TicketDecision",
"TicketImplementationReport",
"TicketReview",
"TicketIntakeReady",
"TicketQueue",
"TicketRelationRecord",
+1 -7
View File
@@ -795,13 +795,7 @@ async fn ticket_review_action_does_not_silently_approve() {
.unwrap_err();
assert!(error.to_string().contains("current action is Queue"));
let ticket = backend.show(TicketIdOrSlug::Id(ticket_id)).unwrap();
assert!(
!ticket
.events
.iter()
.any(|event| event.kind == TicketEventKind::Review)
);
let _ticket = backend.show(TicketIdOrSlug::Id(ticket_id)).unwrap();
}
#[test]
+1
View File
@@ -9,6 +9,7 @@ pub mod manage_workdir;
pub mod manage_worker;
pub mod memory;
pub mod memory_extract;
pub mod merge_request;
pub mod objective;
pub mod session_explore;
pub mod task;
@@ -0,0 +1,299 @@
use crate::feature::ToolDefinition;
use crate::worker::{WorkspaceClient, WorkspaceRequest, WorkspaceRequestMethod};
use async_trait::async_trait;
use llm_engine::tool::{Tool, ToolError, ToolExecutionContext, ToolMeta, ToolOutput};
use schemars::JsonSchema;
use serde::Deserialize;
use serde_json::json;
use std::sync::Arc;
pub const MERGE_REQUEST_COMMON_TOOL_NAMES: &[&str] = &[
"MergeRequestShow",
"MergeRequestReadinessCheck",
"MergeRequestOpen",
"MergeRequestAddRevision",
"MergeRequestComplete",
];
pub const MERGE_REQUEST_REVIEW_TOOL_NAME: &str = "MergeRequestReviewSubmit";
#[derive(Clone, Copy)]
enum Kind {
Show,
Readiness,
Open,
AddRevision,
Complete,
Review,
}
#[derive(Clone)]
struct MergeRequestTool {
client: Arc<dyn WorkspaceClient>,
kind: Kind,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct ShowInput {
ticket: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct OpenInput {
ticket: String,
repository_id: String,
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
#[serde(default)]
summary: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct AddRevisionInput {
ticket: String,
expected_current_revision_id: String,
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
#[serde(default)]
summary: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct CompleteInput {
ticket: String,
operation_id: String,
expected_revision_id: String,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct ReviewInput {
decision: ReviewDecisionInput,
#[serde(default)]
body: String,
#[serde(default)]
findings: Vec<ReviewFindingInput>,
}
#[derive(Debug, Deserialize, JsonSchema)]
#[serde(rename_all = "snake_case")]
enum ReviewDecisionInput {
Approve,
RequestChanges,
}
#[derive(Debug, Deserialize, JsonSchema)]
struct ReviewFindingInput {
severity: String,
#[serde(default)]
code: Option<String>,
#[serde(default)]
path: Option<String>,
#[serde(default)]
line: Option<u64>,
body: String,
}
impl Kind {
fn name(self) -> &'static str {
match self {
Self::Show => "MergeRequestShow",
Self::Readiness => "MergeRequestReadinessCheck",
Self::Open => "MergeRequestOpen",
Self::AddRevision => "MergeRequestAddRevision",
Self::Complete => "MergeRequestComplete",
Self::Review => "MergeRequestReviewSubmit",
}
}
fn description(self) -> &'static str {
description(self.name()).unwrap_or("Merge Request operation.")
}
fn schema(self) -> serde_json::Value {
match self {
Self::Show | Self::Readiness => json!(schemars::schema_for!(ShowInput)),
Self::Open => json!(schemars::schema_for!(OpenInput)),
Self::AddRevision => json!(schemars::schema_for!(AddRevisionInput)),
Self::Complete => json!(schemars::schema_for!(CompleteInput)),
Self::Review => json!(schemars::schema_for!(ReviewInput)),
}
}
}
#[async_trait]
impl Tool for MergeRequestTool {
async fn execute(
&self,
input: &str,
_context: ToolExecutionContext,
) -> Result<ToolOutput, ToolError> {
let workspace_id = self.client.workspace_id().ok_or_else(|| {
ToolError::ExecutionFailed("Merge Request tools require Workspace identity".into())
})?;
let (method, path, body) = match self.kind {
Kind::Show => {
let v: ShowInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Get,
format!("/api/w/{workspace_id}/tickets/{}/merge-request", v.ticket),
None,
)
}
Kind::Readiness => {
let v: ShowInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Get,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/readiness",
v.ticket
),
None,
)
}
Kind::Open => {
let v: OpenInput = parse(input)?;
nonempty(&v.ticket)?;
(
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}),
),
)
}
Kind::AddRevision => {
let v: AddRevisionInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/revisions",
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}),
),
)
}
Kind::Complete => {
let v: CompleteInput = parse(input)?;
nonempty(&v.ticket)?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/complete",
v.ticket
),
Some(
json!({"operation_id":v.operation_id,"expected_revision_id":v.expected_revision_id}),
),
)
}
Kind::Review => {
let v: ReviewInput = parse(input)?;
let context = self.client.reviewer_attempt_context().ok_or_else(|| {
ToolError::ExecutionFailed(
"MergeRequestReviewSubmit is available only to an attested Reviewer child"
.into(),
)
})?;
(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{workspace_id}/tickets/{}/merge-request/reviews",
context.ticket_id
),
Some(
json!({"decision":match v.decision{ReviewDecisionInput::Approve=>"approve",ReviewDecisionInput::RequestChanges=>"request_changes"},"body":v.body,"findings":v.findings.into_iter().map(|f|json!({"severity":f.severity,"code":f.code,"path":f.path,"line":f.line,"body":f.body})).collect::<Vec<_>>() }),
),
)
}
};
let request = match body {
Some(body) => WorkspaceRequest::json(method, path, body.to_string()),
None => WorkspaceRequest::get(path),
};
let response = self
.client
.execute(request)
.map_err(|e| ToolError::ExecutionFailed(e.to_string()))?;
if !response.is_success() {
return Err(ToolError::ExecutionFailed(format!(
"Merge Request API returned HTTP {}: {}",
response.status, response.body
)));
}
Ok(ToolOutput {
summary: self.kind.name().to_string(),
content: Some(response.body),
attachments: Vec::new(),
})
}
}
fn parse<T: serde::de::DeserializeOwned>(value: &str) -> Result<T, ToolError> {
serde_json::from_str(value).map_err(|e| ToolError::InvalidArgument(e.to_string()))
}
fn nonempty(value: &str) -> Result<(), ToolError> {
if value.trim().is_empty() {
Err(ToolError::InvalidArgument(
"ticket must not be empty".into(),
))
} else {
Ok(())
}
}
fn definition(client: Arc<dyn WorkspaceClient>, kind: Kind) -> ToolDefinition {
Arc::new(move || {
let meta = ToolMeta::new(kind.name())
.description(kind.description())
.input_schema(kind.schema());
let tool: Arc<dyn Tool> = Arc::new(MergeRequestTool {
client: client.clone(),
kind,
});
(meta, tool)
})
}
pub fn common_tools(client: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
vec![
definition(client.clone(), Kind::Show),
definition(client.clone(), Kind::Readiness),
definition(client.clone(), Kind::Open),
definition(client.clone(), Kind::AddRevision),
definition(client, Kind::Complete),
]
}
pub fn reviewer_tools(client: Arc<dyn WorkspaceClient>) -> Vec<ToolDefinition> {
if client.reviewer_attempt_context().is_some() {
vec![
definition(client.clone(), Kind::Show),
definition(client, Kind::Review),
]
} else {
Vec::new()
}
}
pub fn description(name: &str) -> Option<&'static str> {
match name {
"MergeRequestShow" => Some(
"Read the authoritative Merge Request, immutable current revision, and structured review status.",
),
"MergeRequestReadinessCheck" => {
Some("Check derived merge readiness for the current immutable revision.")
}
"MergeRequestOpen" => {
Some("Open an immutable Merge Request revision for the current assigned Coder.")
}
"MergeRequestAddRevision" => {
Some("Append an immutable revision; prior approval cannot carry to the new revision.")
}
"MergeRequestComplete" => {
Some("CAS-complete an approved revision with operation-id replay and crash fencing.")
}
"MergeRequestReviewSubmit" => Some(
"Submit the attested direct-child Reviewer result bound to its immutable revision.",
),
_ => None,
}
}
+32 -25
View File
@@ -14,12 +14,13 @@ use ticket::{
NewTicketRelation, OrchestrationPlanKind, OrchestrationPlanRecord, Result as TicketResult,
Ticket, TicketBackend, TicketBackendOperation, TicketBackendOperationResult,
TicketDoctorReport, TicketError, TicketIdOrSlug, TicketIntakeSummary, TicketListQuery,
TicketRef, TicketRelation, TicketRelationKind, TicketRelationView, TicketReview,
TicketStateChange, TicketSummary,
TicketRef, TicketRelation, TicketRelationKind, TicketRelationView, TicketStateChange,
TicketSummary,
config::{DEFAULT_TICKET_BACKEND_RELATIVE_PATH, TicketConfig},
tool::{TICKET_TOOL_NAMES, TicketToolBackend, ticket_tool_description, ticket_tools},
};
use super::merge_request;
use crate::feature::{
FeatureDescriptor, FeatureDiagnostic, FeatureInstallContext, FeatureInstallError,
FeatureInstructionContribution, FeatureInstructionDeclaration, FeatureInstructionId,
@@ -100,7 +101,7 @@ impl TicketFeatureAccess {
pub const fn review() -> Self {
Self {
authoring: false,
thread: true,
thread: false,
intake: false,
orchestration_control: false,
}
@@ -141,7 +142,7 @@ const AUTHORING_TOOL_NAMES: &[&str] = &[
"TicketRelationRecord",
];
const THREAD_TOOL_NAMES: &[&str] = &["TicketComment", "TicketReview"];
const THREAD_TOOL_NAMES: &[&str] = &["TicketComment"];
const INTAKE_TOOL_NAMES: &[&str] = &["TicketIntakeReady"];
@@ -152,7 +153,6 @@ const WORKSPACE_AUTHORING_TOOL_NAMES: &[&str] = &[
"TicketList",
"TicketShow",
"TicketComment",
"TicketReview",
"TicketQueue",
"TicketClose",
"TicketDependencyCheck",
@@ -167,7 +167,6 @@ const ORCHESTRATION_CONTROL_TOOL_NAMES: &[&str] = &[
"TicketList",
"TicketShow",
"TicketComment",
"TicketReview",
"TicketWorkflowState",
"TicketClose",
"TicketDependencyCheck",
@@ -340,6 +339,22 @@ impl FeatureModule for TicketFeature {
ticket_tool_description(name, self.record_language.as_deref()),
));
}
if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend {
let names: Vec<&str> = if client.reviewer_attempt_context().is_some() {
vec![
"MergeRequestShow",
merge_request::MERGE_REQUEST_REVIEW_TOOL_NAME,
]
} else {
merge_request::MERGE_REQUEST_COMMON_TOOL_NAMES.to_vec()
};
for name in names {
descriptor = descriptor.with_tool(ToolDeclaration::new(
name,
merge_request::description(name).unwrap_or("Merge Request operation."),
));
}
}
descriptor
}
@@ -373,6 +388,17 @@ impl FeatureModule for TicketFeature {
}
tools.register(ToolContribution::new(name, definition))?;
}
if let TicketFeatureBackend::WorkspaceClient(client) = &self.backend {
let definitions = if client.reviewer_attempt_context().is_some() {
merge_request::reviewer_tools(client.clone())
} else {
merge_request::common_tools(client.clone())
};
for definition in definitions {
let (meta, _) = definition();
tools.register(ToolContribution::new(meta.name.clone(), definition))?;
}
}
Ok(())
}
}
@@ -611,14 +637,6 @@ impl WorkspaceHttpTicketBackend {
format!("{base}/{}/workflow/queue", Self::ticket_path(&id)),
None,
),
TicketBackendOperation::Review { id, review } => Self::request_unit(
client,
WorkspaceRequestMethod::Post,
format!("{base}/{}/workflow/review", Self::ticket_path(&id)),
Some(serde_json::to_value(review).map_err(|error| {
TicketError::Conflict(format!("serialize Ticket review: {error}"))
})?),
),
TicketBackendOperation::Close { id, resolution } => Self::request_unit(
client,
WorkspaceRequestMethod::Post,
@@ -844,15 +862,6 @@ impl TicketBackend for WorkspaceHttpTicketBackend {
}
}
fn review(&self, id: TicketIdOrSlug, review: TicketReview) -> TicketResult<()> {
match self.invoke(TicketBackendOperation::Review { id, review })? {
TicketBackendOperationResult::Unit => Ok(()),
other => Err(TicketError::Conflict(format!(
"unexpected ticket backend response: {other:?}"
))),
}
}
fn close(&self, id: TicketIdOrSlug, resolution: MarkdownText) -> TicketResult<()> {
match self.invoke(TicketBackendOperation::Close { id, resolution })? {
TicketBackendOperationResult::Unit => Ok(()),
@@ -1075,7 +1084,6 @@ mod tests {
.map(|tool| tool.name.as_str())
.collect::<Vec<_>>();
assert!(work_report_tools.contains(&"TicketComment"));
assert!(work_report_tools.contains(&"TicketReview"));
assert!(!work_report_tools.contains(&"TicketWorkflowState"));
let review = ticket_tools_feature_with_access(temp.path(), TicketFeatureAccess::review());
@@ -1085,7 +1093,6 @@ mod tests {
.iter()
.map(|tool| tool.name.as_str())
.collect::<Vec<_>>();
assert!(review_tools.contains(&"TicketReview"));
assert!(!review_tools.contains(&"TicketWorkflowState"));
}
+4
View File
@@ -259,6 +259,10 @@ pub(crate) struct InternalWorkerSessionHandle {
}
impl InternalWorkerSessionHandle {
pub(crate) fn session_id_string(&self) -> String {
self.session_id.to_string()
}
pub(crate) fn status(&self) -> InternalWorkerSessionStatus {
InternalWorkerSessionStatus::decode(self.status.load(std::sync::atomic::Ordering::Acquire))
}
+169 -2
View File
@@ -27,7 +27,10 @@ use crate::internal_worker::{
};
use crate::prompt::catalog::PromptCatalog;
use crate::spawn::registry::SpawnedWorkerRegistry;
use crate::worker::{Worker, WorkerFilesystemAuthority};
use crate::worker::{
ReviewerAttemptContext, ReviewerChildWorkspaceClient, Worker, WorkerFilesystemAuthority,
WorkspaceRequest, WorkspaceRequestMethod,
};
use protocol::Method;
#[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -55,6 +58,16 @@ struct SubWorkerSpawnInput {
/// spawner's explicit delegation authority; direct tool scope alone is not
/// sufficient. Omit `recursive` for normal workspace/worktree delegation; it defaults to true.
scope: Vec<ScopeRuleInput>,
/// Binds an actual read-only builtin Reviewer child to an immutable Merge Request revision.
/// Review attempt identity and capability material are generated by the trusted spawn layer.
#[serde(default)]
review: Option<ReviewerHandoffInput>,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
struct ReviewerHandoffInput {
ticket_id: String,
revision_id: String,
}
#[derive(Debug, Deserialize, schemars::JsonSchema)]
@@ -320,6 +333,32 @@ impl SubWorkerSpawnTool {
}
}
fn validate_reviewer_handoff(input: &SubWorkerSpawnInput) -> Result<(), ToolError> {
let Some(review) = &input.review else {
return Ok(());
};
if review.ticket_id.trim().is_empty() || review.revision_id.trim().is_empty() {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires non-empty ticket_id and revision_id".to_string(),
));
}
if input.profile.as_deref() != Some("builtin:reviewer") {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires the explicit effective profile builtin:reviewer".to_string(),
));
}
if input
.scope
.iter()
.any(|rule| matches!(rule.permission, PermissionInput::Write))
{
return Err(ToolError::InvalidArgument(
"Merge Request Reviewer SubWorkers must have read-only delegated scope".to_string(),
));
}
Ok(())
}
#[async_trait]
impl Tool for SubWorkerSpawnTool {
async fn execute(
@@ -340,6 +379,7 @@ impl Tool for SubWorkerSpawnTool {
input.name
)));
}
validate_reviewer_handoff(&input)?;
let name_reservation = self
.registry
.reserve_internal_name(input.name.clone())
@@ -378,6 +418,48 @@ impl Tool for SubWorkerSpawnTool {
.map_err(|error| {
ToolError::ExecutionFailed(format!("resolve child manifest: {error}"))
})?;
let reviewer_attempt = input.review.as_ref().map(|review| {
(
review.ticket_id.clone(),
review.revision_id.clone(),
uuid::Uuid::now_v7().to_string(),
format!(
"{}{}",
uuid::Uuid::now_v7().simple(),
uuid::Uuid::now_v7().simple()
),
)
});
let child_workspace_context =
if let Some((ticket_id, revision_id, _, capability_token)) = &reviewer_attempt {
let workspace_id =
self.workspace_context
.workspace_id()
.cloned()
.ok_or_else(|| {
ToolError::InvalidArgument(
"reviewer handoff requires Workspace identity".to_string(),
)
})?;
let parent_client = self.workspace_context.client_handle();
if !parent_client.is_available() {
return Err(ToolError::InvalidArgument(
"reviewer handoff requires Workspace API authority".to_string(),
));
}
let child_client: Arc<dyn crate::worker::WorkspaceClient> =
Arc::new(ReviewerChildWorkspaceClient::new(
parent_client.clone(),
ReviewerAttemptContext {
ticket_id: ticket_id.clone(),
revision_id: revision_id.clone(),
},
capability_token.clone(),
));
crate::worker::WorkerWorkspaceContext::with_client(Some(workspace_id), child_client)
} else {
self.workspace_context.clone()
};
let store = EphemeralSessionStore::default();
let filesystem_authority =
WorkerFilesystemAuthority::local(self.workspace_root.clone(), child_cwd.clone());
@@ -385,7 +467,7 @@ impl Tool for SubWorkerSpawnTool {
child_manifest,
store.clone(),
self.prompt_loader.clone(),
self.workspace_context.clone(),
child_workspace_context,
filesystem_authority,
self.internal_client_override
.as_ref()
@@ -465,6 +547,66 @@ impl Tool for SubWorkerSpawnTool {
}
};
if let Some((ticket_id, revision_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())
})?;
let child_session_id = session.session_id_string();
let child_registration = WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/internal/reviewer-child-sessions",
workspace_id.as_str()
),
serde_json::json!({"child_session_id": child_session_id}).to_string(),
);
let child_response = self
.workspace_context
.client()
.execute(child_registration)
.map_err(|error| {
ToolError::ExecutionFailed(format!(
"register Runtime-owned Reviewer child session: {error}"
))
})?;
if !child_response.is_success() {
let _ = session.stop().await;
return Err(ToolError::ExecutionFailed(format!(
"register Runtime-owned Reviewer child session failed with status {}: {}",
child_response.status, child_response.body
)));
}
let body = serde_json::json!({
"attempt_id": attempt_id,
"revision_id": revision_id,
"child_session_id": child_session_id,
"capability_token": capability_token,
});
let request = WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
format!(
"/api/w/{}/tickets/{}/merge-request/review-attempts",
workspace_id.as_str(),
ticket_id
),
body.to_string(),
);
let response = self
.workspace_context
.client()
.execute(request)
.map_err(|error| {
ToolError::ExecutionFailed(format!("register reviewer attempt: {error}"))
})?;
if !response.is_success() {
let _ = session.stop().await;
return Err(ToolError::ExecutionFailed(format!(
"register reviewer attempt failed with status {}: {}",
response.status, response.body
)));
}
}
let record = crate::spawn::registry::InternalSpawnedWorkerRecord::new(
input.name.clone(),
scope_allow,
@@ -899,6 +1041,31 @@ mod tests {
WorkspaceClient, WorkspaceClientError, WorkspaceRequest, WorkspaceResponse,
};
#[test]
fn reviewer_handoff_requires_explicit_builtin_profile_and_read_only_scope() {
let valid: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:reviewer",
"scope":[{"target":"/tmp/work","permission":"read"}],
"review":{"ticket_id":"T1","revision_id":"V1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&valid).is_ok());
let wrong_profile: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:coder",
"scope":[{"target":"/tmp/work","permission":"read"}],
"review":{"ticket_id":"T1","revision_id":"V1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&wrong_profile).is_err());
let writable: SubWorkerSpawnInput = serde_json::from_value(serde_json::json!({
"name":"reviewer","task":"review","profile":"builtin:reviewer",
"scope":[{"target":"/tmp/work","permission":"write"}],
"review":{"ticket_id":"T1","revision_id":"V1"}
}))
.unwrap();
assert!(validate_reviewer_handoff(&writable).is_err());
}
fn abs_rule(path: &Path, permission: Permission) -> ScopeRule {
ScopeRule {
target: path.to_path_buf(),
+118
View File
@@ -223,6 +223,94 @@ pub trait WorkspaceClient: std::fmt::Debug + Send + Sync {
fn is_available(&self) -> bool;
fn execute(&self, request: WorkspaceRequest)
-> Result<WorkspaceResponse, WorkspaceClientError>;
/// Trusted review-attempt context is injected by the Internal SubWorker spawn layer.
/// It is never accepted from a model-visible tool argument.
fn reviewer_attempt_context(&self) -> Option<&ReviewerAttemptContext> {
None
}
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ReviewerAttemptContext {
pub ticket_id: String,
pub revision_id: String,
}
#[derive(Debug)]
pub struct ReviewerChildWorkspaceClient {
inner: Arc<dyn WorkspaceClient>,
context: ReviewerAttemptContext,
capability_token: String,
}
impl ReviewerChildWorkspaceClient {
pub fn new(
inner: Arc<dyn WorkspaceClient>,
context: ReviewerAttemptContext,
capability_token: String,
) -> Self {
Self {
inner,
context,
capability_token,
}
}
}
impl WorkspaceClient for ReviewerChildWorkspaceClient {
fn workspace_id(&self) -> Option<&str> {
self.inner.workspace_id()
}
fn kind(&self) -> &str {
"runtime-reviewer-child"
}
fn is_available(&self) -> bool {
self.inner.is_available()
}
fn reviewer_attempt_context(&self) -> Option<&ReviewerAttemptContext> {
Some(&self.context)
}
fn execute(
&self,
mut request: WorkspaceRequest,
) -> Result<WorkspaceResponse, WorkspaceClientError> {
let expected_path = format!(
"/api/w/{}/tickets/{}/merge-request/reviews",
self.workspace_id().unwrap_or_default(),
self.context.ticket_id
);
if request.method == WorkspaceRequestMethod::Post && request.path == expected_path {
let body = request.body.take().ok_or_else(|| {
WorkspaceClientError::Request("review submission requires a JSON body".to_string())
})?;
let mut value: serde_json::Value = serde_json::from_str(&body)
.map_err(|error| WorkspaceClientError::Request(error.to_string()))?;
let object = value.as_object_mut().ok_or_else(|| {
WorkspaceClientError::Request(
"review submission body must be an object".to_string(),
)
})?;
object.insert(
"revision_id".to_string(),
serde_json::Value::String(self.context.revision_id.clone()),
);
object.insert(
"capability_token".to_string(),
serde_json::Value::String(self.capability_token.clone()),
);
request.body = Some(
serde_json::to_string(&value)
.map_err(|error| WorkspaceClientError::Request(error.to_string()))?,
);
} else if request.method != WorkspaceRequestMethod::Get {
return Err(WorkspaceClientError::Unavailable(
"Reviewer child Workspace authority is read-only except for its one attested Merge Request review submission".to_string(),
));
}
self.inner.execute(request)
}
}
/// HTTP forwarding client created by Runtime for one concrete Worker execution.
@@ -365,6 +453,36 @@ impl WorkspaceClient for MarkerWorkspaceClient {
}
}
#[cfg(test)]
mod reviewer_client_tests {
use super::*;
#[test]
fn reviewer_child_client_denies_non_review_workspace_mutations() {
let inner: Arc<dyn WorkspaceClient> = Arc::new(MarkerWorkspaceClient {
workspace_id: Some("ws".to_string()),
kind: "marker".to_string(),
available: true,
reason: "forwarded".to_string(),
});
let client = ReviewerChildWorkspaceClient::new(
inner,
ReviewerAttemptContext {
ticket_id: "T1".into(),
revision_id: "V1".into(),
},
"secret".into(),
);
let request = WorkspaceRequest::json(
WorkspaceRequestMethod::Post,
"/api/w/ws/tickets/T1/comments",
"{}".to_string(),
);
let error = client.execute(request).unwrap_err();
assert!(error.to_string().contains("read-only"));
}
}
pub fn unavailable_workspace_client(
workspace_id: Option<&WorkspaceId>,
reason: impl Into<String>,
+1
View File
@@ -32,6 +32,7 @@ sha2.workspace = true
thiserror.workspace = true
ticket.workspace = true
memory.workspace = true
merge-request.workspace = true
tokio = { workspace = true, features = ["fs", "macros", "net", "rt-multi-thread", "sync", "time"] }
tokio-tungstenite.workspace = true
worker.workspace = true
+10
View File
@@ -55,6 +55,8 @@ pub enum Error {
Sqlite(#[from] rusqlite::Error),
#[error("ticket error: {0}")]
Ticket(#[from] ticket::TicketError),
#[error("merge request error: {0}")]
MergeRequest(#[from] merge_request::MergeRequestError),
#[error("yaml error: {0}")]
Yaml(#[from] serde_yaml::Error),
#[error("invalid input: {0}")]
@@ -88,6 +90,14 @@ 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"
)]
BrowserReopenConfirmationRequired,
#[error("workspace id does not match this Workspace backend")]
WorkspaceIdMismatch,
#[error("Ticket assignment conflict: {0}")]
+482 -77
View File
@@ -24,8 +24,7 @@ use serde::{Deserialize, Serialize};
use sha2::{Digest, Sha256};
use ticket::{
MarkdownText, NewTicketEvent, TicketBackend, TicketBodyReplacement, TicketEventKind,
TicketIdOrSlug, TicketItemEdit, TicketReview, TicketReviewResult, TicketStateChange,
TicketTargetEdit, TicketWorkflowState,
TicketIdOrSlug, TicketItemEdit, TicketStateChange, TicketTargetEdit, TicketWorkflowState,
};
use ticket::{
SqliteTicketBackend, TicketBackendOperation, TicketBackendOperationResult,
@@ -861,8 +860,40 @@ pub fn build_router(api: WorkspaceApi) -> Router {
post(scoped_queue_ticket_record),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/workflow/review",
post(scoped_review_ticket_record),
"/api/w/{workspace_id}/tickets/{id}/merge-request",
get(scoped_show_merge_request).post(scoped_open_merge_request),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/merge-request/readiness",
get(scoped_merge_request_readiness),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/merge-request/revisions",
post(scoped_add_merge_request_revision),
)
.route(
"/api/w/{workspace_id}/internal/reviewer-child-sessions",
post(scoped_register_reviewer_child_session),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/merge-request/review-attempts",
post(scoped_register_merge_request_review_attempt),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/merge-request/reviews",
post(scoped_submit_merge_request_review),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/merge-request/complete",
post(scoped_complete_merge_request),
)
.route(
"/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",
@@ -902,10 +933,6 @@ pub fn build_router(api: WorkspaceApi) -> Router {
"/api/w/{workspace_id}/tickets/{id}/events",
post(scoped_append_ticket_event),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/reviews",
post(scoped_review_ticket),
)
.route(
"/api/w/{workspace_id}/tickets/{id}/queue",
post(scoped_queue_ticket),
@@ -2419,14 +2446,6 @@ struct BrowserAppendTicketEventRequest {
author: Option<String>,
}
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct BrowserReviewTicketRequest {
result: TicketReviewResult,
body: String,
author: Option<String>,
}
#[derive(Debug, Deserialize)]
#[serde(deny_unknown_fields)]
struct BrowserQueueTicketRequest {
@@ -2505,6 +2524,11 @@ async fn scoped_transition_ticket_state(
Json(request): Json<BrowserTransitionTicketStateRequest>,
) -> ApiResult<Json<TicketDetail>> {
validate_workspace_scope(&api, &path.workspace_id)?;
if request.state == TicketWorkflowState::Done {
return Err(Error::TicketAssignmentConflict(
"done is guarded by MergeRequestComplete with an approved immutable revision and operation_id".to_string(),
).into());
}
let current = api.authority.ticket(&path.id)?;
let mut change = TicketStateChange::new(
current.state,
@@ -2535,25 +2559,6 @@ async fn scoped_append_ticket_event(
browser_ticket_detail(&api, &path.id)
}
async fn scoped_review_ticket(
State(api): State<WorkspaceApi>,
AxumPath(path): AxumPath<ScopedRecordPath>,
Json(request): Json<BrowserReviewTicketRequest>,
) -> ApiResult<Json<TicketDetail>> {
validate_workspace_scope(&api, &path.workspace_id)?;
browser_ticket_backend(&api)?
.review(
TicketIdOrSlug::Id(path.id.clone()),
TicketReview {
result: request.result,
body: MarkdownText::new(request.body),
author: request.author,
},
)
.map_err(Error::from)?;
browser_ticket_detail(&api, &path.id)
}
async fn scoped_queue_ticket(
State(api): State<WorkspaceApi>,
AxumPath(path): AxumPath<ScopedRecordPath>,
@@ -2602,6 +2607,16 @@ async fn scoped_close_ticket(
browser_ticket_detail(&api, &path.id)
}
fn reject_unguarded_ticket_completion(operation: &TicketBackendOperation) -> Result<()> {
if matches!(operation, TicketBackendOperation::SetWorkflowState { change, .. } if change.to == "done")
{
return Err(Error::TicketAssignmentConflict(
"done is guarded by MergeRequestComplete with an approved immutable revision and operation_id".to_string(),
));
}
Ok(())
}
async fn execute_worker_ticket_rest_operation(
api: &WorkspaceApi,
workspace_id: &str,
@@ -2621,6 +2636,7 @@ async fn execute_worker_ticket_rest_operation(
let is_mutation = operation_kind != "read";
let target = ticket_mutation_target(&operation).cloned();
let source = authenticate_worker_mutation_source(api, workspace_id, &headers)?;
reject_unguarded_ticket_completion(&operation)?;
validate_ticket_repository_operation(api, &operation)?;
let before = target.as_ref().and_then(|id| backend.show(id.clone()).ok());
let previous_state = before
@@ -2969,23 +2985,393 @@ async fn scoped_queue_ticket_record(
ticket_rest_unit(result)
}
async fn scoped_review_ticket_record(
State(api): State<WorkspaceApi>,
AxumPath((workspace_id, id)): AxumPath<(String, String)>,
headers: HeaderMap,
Json(review): Json<TicketReview>,
) -> ApiResult<StatusCode> {
let result = execute_worker_ticket_rest_operation(
&api,
&workspace_id,
headers,
TicketBackendOperation::Review {
id: TicketIdOrSlug::Query(id),
review,
},
#[derive(Debug, serde::Deserialize)]
struct OpenMergeRequestRequest {
repository_id: String,
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
#[serde(default)]
summary: String,
}
#[derive(Debug, serde::Deserialize)]
struct AddMergeRequestRevisionRequest {
expected_current_revision_id: String,
revision_id: String,
base_commit: String,
head_commit: String,
head_tree: String,
diff_digest: String,
#[serde(default)]
changed_paths: Vec<String>,
#[serde(default)]
summary: String,
}
#[derive(Debug, serde::Deserialize)]
struct RegisterReviewerChildSessionRequest {
child_session_id: String,
}
#[derive(Debug, serde::Deserialize)]
struct RegisterMergeRequestReviewAttemptRequest {
attempt_id: String,
revision_id: String,
child_session_id: String,
capability_token: String,
}
#[derive(Debug, serde::Deserialize)]
struct SubmitMergeRequestReviewRequest {
revision_id: String,
capability_token: String,
decision: merge_request::ReviewDecision,
#[serde(default)]
body: String,
#[serde(default)]
findings: Vec<merge_request::ReviewFinding>,
}
#[derive(Debug, serde::Deserialize)]
struct CompleteMergeRequestRequest {
operation_id: String,
expected_revision_id: String,
}
#[derive(Debug, serde::Deserialize)]
struct RevisionTransitionRequest {
expected_revision_id: String,
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());
}
Ok(value.to_string())
}
fn require_workspace_access(workspace_id: &str, api: &WorkspaceApi) -> ApiResult<()> {
if workspace_id != api.workspace_id() {
return Err(Error::WorkspaceIdMismatch.into());
}
Ok(())
}
fn merge_request_store(
api: &WorkspaceApi,
workspace_id: &str,
) -> ApiResult<merge_request::SqliteMergeRequestStore> {
require_workspace_access(workspace_id, api)?;
merge_request::SqliteMergeRequestStore::open_verified(
api.config.database_path.clone(),
workspace_id,
)
.await?;
ticket_rest_unit(result)
.map_err(Error::from)
.map_err(Into::into)
}
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 value = store
.show_for_ticket(&ticket_id)?
.ok_or_else(|| Error::from(merge_request::MergeRequestError::NotFound(ticket_id)))?;
Ok(Json(value))
}
async fn scoped_merge_request_readiness(
State(api): State<WorkspaceApi>,
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)?,
))
}
async fn scoped_open_merge_request(
State(api): State<WorkspaceApi>,
headers: HeaderMap,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
Json(input): Json<OpenMergeRequestRequest>,
) -> ApiResult<Json<merge_request::MergeRequest>> {
let workspace_id = parse_workspace_id(&workspace_id)?;
require_workspace_access(&workspace_id, &api)?;
let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?;
let assignment = api
.store
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict("Ticket has no current assigned Coder".to_string())
})?;
if assignment.worker.runtime_id != source.runtime_id
|| assignment.worker.worker_id != source.worker_id
{
return Err(Error::TicketAssignmentConflict(
"authenticated Worker is not the current Ticket assignee".to_string(),
)
.into());
}
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,
diff_digest: input.diff_digest,
changed_paths: input.changed_paths,
summary: input.summary,
assignment_id: assignment.assignment_id.clone(),
created_at: now.clone(),
};
let mr = merge_request_store(&api, &workspace_id)?.open_merge_request(
merge_request::OpenMergeRequest {
merge_request_id: format!("mr_{}", Uuid::now_v7().simple()),
ticket_id,
repository_id: input.repository_id,
revision,
authenticated_runtime_id: source.runtime_id,
authenticated_worker_id: source.worker_id,
now,
},
)?;
Ok(Json(mr))
}
async fn scoped_add_merge_request_revision(
State(api): State<WorkspaceApi>,
headers: HeaderMap,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
Json(input): Json<AddMergeRequestRevisionRequest>,
) -> ApiResult<Json<merge_request::MergeRequest>> {
let workspace_id = parse_workspace_id(&workspace_id)?;
require_workspace_access(&workspace_id, &api)?;
let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?;
let assignment = api
.store
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict("Ticket has no current assigned Coder".into())
})?;
if assignment.worker.runtime_id != source.runtime_id
|| assignment.worker.worker_id != source.worker_id
{
return Err(Error::TicketAssignmentConflict(
"authenticated Worker is not the current Ticket assignee".into(),
)
.into());
}
let current = merge_request_store(&api, &workspace_id)?
.show_for_ticket(&ticket_id)?
.ok_or_else(|| {
Error::from(merge_request::MergeRequestError::NotFound(
ticket_id.clone(),
))
})?;
let now = Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true);
let mr =
merge_request_store(&api, &workspace_id)?.add_revision(merge_request::AddRevision {
ticket_id,
expected_current_revision_id: input.expected_current_revision_id,
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,
diff_digest: input.diff_digest,
changed_paths: input.changed_paths,
summary: input.summary,
assignment_id: assignment.assignment_id,
created_at: now.clone(),
},
authenticated_runtime_id: source.runtime_id,
authenticated_worker_id: source.worker_id,
now,
})?;
Ok(Json(mr))
}
async fn scoped_register_reviewer_child_session(
State(api): State<WorkspaceApi>,
headers: HeaderMap,
AxumPath(workspace_id): AxumPath<String>,
Json(input): Json<RegisterReviewerChildSessionRequest>,
) -> ApiResult<StatusCode> {
let workspace_id = parse_workspace_id(&workspace_id)?;
require_workspace_access(&workspace_id, &api)?;
let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?;
merge_request_store(&api, &workspace_id)?.register_reviewer_child_session(
merge_request::RegisterReviewerChildSession {
parent_runtime_id: source.runtime_id,
parent_worker_id: source.worker_id,
child_session_id: input.child_session_id,
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
},
)?;
Ok(StatusCode::NO_CONTENT)
}
async fn scoped_register_merge_request_review_attempt(
State(api): State<WorkspaceApi>,
headers: HeaderMap,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
Json(input): Json<RegisterMergeRequestReviewAttemptRequest>,
) -> ApiResult<StatusCode> {
let workspace_id = parse_workspace_id(&workspace_id)?;
require_workspace_access(&workspace_id, &api)?;
let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?;
let assignment = api
.store
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict("Ticket has no current assigned Coder".into())
})?;
if assignment.worker.runtime_id != source.runtime_id
|| assignment.worker.worker_id != source.worker_id
{
return Err(Error::TicketAssignmentConflict(
"authenticated Worker is not the current Ticket assignee".into(),
)
.into());
}
merge_request_store(&api, &workspace_id)?.register_review_attempt(
merge_request::RegisterReviewAttempt {
attempt_id: input.attempt_id,
ticket_id,
revision_id: input.revision_id,
parent_assignment_id: assignment.assignment_id,
parent_runtime_id: source.runtime_id,
parent_worker_id: source.worker_id,
child_session_id: input.child_session_id,
capability_token: input.capability_token,
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
},
)?;
Ok(StatusCode::NO_CONTENT)
}
async fn scoped_submit_merge_request_review(
State(api): State<WorkspaceApi>,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
Json(input): Json<SubmitMergeRequestReviewRequest>,
) -> ApiResult<Json<merge_request::MergeRequestReview>> {
let workspace_id = parse_workspace_id(&workspace_id)?;
let review =
merge_request_store(&api, &workspace_id)?.submit_review(merge_request::SubmitReview {
ticket_id,
revision_id: input.revision_id,
capability_token: input.capability_token,
decision: input.decision,
body: input.body,
findings: input.findings,
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
})?;
Ok(Json(review))
}
async fn scoped_complete_merge_request(
State(api): State<WorkspaceApi>,
headers: HeaderMap,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
Json(input): Json<CompleteMergeRequestRequest>,
) -> ApiResult<Json<merge_request::CompletionOutcome>> {
let workspace_id = parse_workspace_id(&workspace_id)?;
require_workspace_access(&workspace_id, &api)?;
let source = authenticate_worker_mutation_source(&api, &workspace_id, &headers)?;
let assignment = api
.store
.get_current_ticket_worker_assignment(&workspace_id, &ticket_id)?
.ok_or_else(|| {
Error::TicketAssignmentConflict("Ticket has no current assigned Coder".into())
})?;
if assignment.worker.runtime_id != source.runtime_id
|| assignment.worker.worker_id != source.worker_id
{
return Err(Error::TicketAssignmentConflict(
"authenticated Worker is not the current Ticket assignee".into(),
)
.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,
assignment_id: assignment.assignment_id,
authenticated_runtime_id: source.runtime_id,
authenticated_worker_id: source.worker_id,
now: Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
},
)?;
Ok(Json(outcome))
}
async fn scoped_reopen_merge_request(
State(api): State<WorkspaceApi>,
headers: HeaderMap,
AxumPath((workspace_id, ticket_id)): AxumPath<(String, String)>,
Json(input): Json<RevisionTransitionRequest>,
) -> 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)?;
let _actor = require_actor(&api, &headers).await?;
if !input.explicit_confirmation {
return Err(Error::BrowserReopenConfirmationRequired.into());
}
Ok(Json(merge_request_store(&api, &workspace_id)?.reopen(
&ticket_id,
&input.expected_revision_id,
&Utc::now().to_rfc3339_opts(SecondsFormat::Millis, true),
)?))
}
fn reject_non_browser_merge_auth(headers: &HeaderMap) -> Result<()> {
if headers.contains_key("authorization") {
return Err(Error::BrowserMergeConfirmationRequired);
}
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(
@@ -3157,7 +3543,6 @@ fn ticket_mutation_target(operation: &TicketBackendOperation) -> Option<&TicketI
| TicketBackendOperation::SetWorkflowState { id, .. }
| TicketBackendOperation::MarkIntakeReady { id, .. }
| TicketBackendOperation::QueueReady { id, .. }
| TicketBackendOperation::Review { id, .. }
| TicketBackendOperation::Close { id, .. }
| TicketBackendOperation::AddTicketRelation { id, .. }
| TicketBackendOperation::AddOrchestrationPlanRecord { id, .. } => Some(id),
@@ -3203,7 +3588,6 @@ fn bind_worker_ticket_operation_source(
change.author = Some(author);
}
TicketBackendOperation::QueueReady { queued_by, .. } => *queued_by = author,
TicketBackendOperation::Review { review, .. } => review.author = Some(author),
TicketBackendOperation::AddTicketRelation { relation, .. } => {
relation.author = Some(author)
}
@@ -3225,7 +3609,6 @@ fn ticket_mutation_operation_kind(operation: &TicketBackendOperation) -> &'stati
TicketBackendOperation::SetWorkflowState { .. } => "set_workflow_state",
TicketBackendOperation::MarkIntakeReady { .. } => "mark_intake_ready",
TicketBackendOperation::QueueReady { .. } => "queue_ready",
TicketBackendOperation::Review { .. } => "review",
TicketBackendOperation::Close { .. } => "close",
TicketBackendOperation::AddTicketRelation { .. } => "add_relation",
TicketBackendOperation::AddOrchestrationPlanRecord { .. } => "add_plan_record",
@@ -7299,10 +7682,7 @@ struct RuntimeConfigBundleAvailabilityQuery {
digest: String,
}
fn reject_workdir_for_embedded_runtime(
runtime_id: &str,
has_workdir: bool,
) -> std::result::Result<(), ApiError> {
fn reject_workdir_for_embedded_runtime(runtime_id: &str, has_workdir: bool) -> ApiResult<()> {
if runtime_id != EMBEDDED_WORKER_RUNTIME_ID || !has_workdir {
return Ok(());
}
@@ -7320,9 +7700,7 @@ fn reject_workdir_for_embedded_runtime(
))
}
fn reject_no_workdir_for_non_embedded_runtime(
runtime_id: &str,
) -> std::result::Result<(), ApiError> {
fn reject_no_workdir_for_non_embedded_runtime(runtime_id: &str) -> ApiResult<()> {
if runtime_id == EMBEDDED_WORKER_RUNTIME_ID {
return Ok(());
}
@@ -9974,6 +10352,12 @@ struct ApiErrorLog {
diagnostics: Vec<RuntimeDiagnostic>,
}
impl From<merge_request::MergeRequestError> for ApiError {
fn from(error: merge_request::MergeRequestError) -> Self {
Error::MergeRequest(error).into()
}
}
impl From<Error> for ApiError {
fn from(error: Error) -> Self {
let diagnostics = match &error {
@@ -10013,6 +10397,9 @@ impl ApiError {
impl IntoResponse for ApiError {
fn into_response(self) -> Response {
let status = match &self.error {
Error::BrowserMergeConfirmationRequired | Error::BrowserReopenConfirmationRequired => {
StatusCode::FORBIDDEN
}
Error::TicketAssignmentConflict(_) | Error::WorkdirAttachmentConflict(_) => {
StatusCode::CONFLICT
}
@@ -10020,7 +10407,14 @@ impl IntoResponse for ApiError {
Error::InvalidRuntimeIdentifier { .. } | Error::ReservedWorkerName(_) => {
StatusCode::BAD_REQUEST
}
Error::Ticket(ticket::TicketError::NotFound(_)) => StatusCode::NOT_FOUND,
Error::Ticket(ticket::TicketError::NotFound(_))
| Error::MergeRequest(merge_request::MergeRequestError::NotFound(_)) => {
StatusCode::NOT_FOUND
}
Error::MergeRequest(merge_request::MergeRequestError::Empty(_)) => {
StatusCode::BAD_REQUEST
}
Error::MergeRequest(_) => StatusCode::CONFLICT,
Error::Ticket(
ticket::TicketError::Ambiguous { .. }
| ticket::TicketError::Locked { .. }
@@ -10161,6 +10555,32 @@ mod tests {
ObjectiveTicketLinkRecord, SqliteWorkspaceStore, WorkspaceRecord,
};
#[test]
fn merge_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)
));
assert!(reject_non_browser_merge_auth(&HeaderMap::new()).is_ok());
}
#[test]
fn flow_or_generic_worker_state_change_is_not_ticket_completion_authority() {
let operation = TicketBackendOperation::SetWorkflowState {
id: TicketIdOrSlug::Query("T1".to_string()),
change: TicketStateChange::new(
"inprogress",
"done",
"flow reached terminal state",
"terminal flow state",
),
};
let error = reject_unguarded_ticket_completion(&operation).unwrap_err();
assert!(error.to_string().contains("MergeRequestComplete"));
}
#[test]
fn failed_api_log_is_structured_and_omits_query_values() {
let uri = "/api/w/workspace/tickets?access_token=secret"
@@ -12596,21 +13016,6 @@ mod tests {
.unwrap();
assert_eq!(queued.state, "queued");
assert_eq!(queued.queued_by.as_deref(), Some("browser-user"));
let Json(reviewed) = scoped_review_ticket(
State(api.clone()),
AxumPath(path()),
Json(BrowserReviewTicketRequest {
result: TicketReviewResult::Approve,
body: "API review".to_string(),
author: Some("reviewer".to_string()),
}),
)
.await
.unwrap();
assert!(reviewed.events.iter().any(|event| {
event.kind == "review" && event.body.as_deref() == Some("API review")
}));
let Json(closed) = scoped_close_ticket(
State(api),
AxumPath(path()),
+1
View File
@@ -765,6 +765,7 @@ impl SqliteWorkspaceStore {
configure_sqlite(&conn)?;
apply_migrations(&conn)?;
ticket::migrate_sqlite_ticket_schema(&conn)?;
merge_request::migrate(&conn).map_err(|error| Error::Store(error.to_string()))?;
validate_workspace_repository_references(&conn)?;
Ok(Self {
conn: Arc::new(Mutex::new(conn)),
+13 -125
View File
@@ -12,8 +12,8 @@ use ticket::config::{
use ticket::{
LocalTicketBackend, MarkdownText, NewTicket, NewTicketEvent, NewTicketRelation,
SqliteTicketBackend, TicketBackend, TicketDoctorSeverity, TicketEventKind, TicketIdOrSlug,
TicketIntakeSummary, TicketListQuery, TicketListState, TicketRelationKind, TicketReview,
TicketReviewResult, TicketSummary, TicketWorkflowState,
TicketIntakeSummary, TicketListQuery, TicketListState, TicketRelationKind, TicketSummary,
TicketWorkflowState,
};
const DEFAULT_LIST_LIMIT: usize = 50;
@@ -35,7 +35,6 @@ pub enum TicketCommand {
List(ListOptions),
Show { query: String },
Comment(CommentOptions),
Review(ReviewOptions),
State(StateOptions),
Close(CloseOptions),
Relation(RelationOptions),
@@ -67,13 +66,6 @@ pub struct CommentOptions {
pub body: BodySource,
}
#[derive(Debug, Clone, PartialEq, Eq)]
pub struct ReviewOptions {
pub query: String,
pub result: TicketReviewResult,
pub body: BodySource,
}
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
pub enum StateTarget {
Planning,
@@ -194,7 +186,6 @@ pub fn parse_ticket_args(args: &[String]) -> Result<TicketCli, TicketCliError> {
query: parse_one_positional("show", &args[1..])?,
},
"comment" => TicketCommand::Comment(parse_comment(&args[1..])?),
"review" => TicketCommand::Review(parse_review(&args[1..])?),
"state" => TicketCommand::State(parse_state(&args[1..])?),
"close" => TicketCommand::Close(parse_close(&args[1..])?),
"relation" => TicketCommand::Relation(parse_relation(&args[1..])?),
@@ -249,7 +240,6 @@ fn run_command(
TicketCommand::List(options) => list(backend.as_ref(), options),
TicketCommand::Show { query } => show(backend.as_ref(), query),
TicketCommand::Comment(options) => comment(backend.as_ref(), options),
TicketCommand::Review(options) => review(backend.as_ref(), options),
TicketCommand::State(options) => state(backend.as_ref(), options),
TicketCommand::Close(options) => close(backend.as_ref(), options),
TicketCommand::Relation(options) => relation(backend.as_ref(), options),
@@ -633,23 +623,6 @@ fn comment(
Ok(success(format!("appended\t{}\t{}\n", options.query, role)))
}
fn review(
backend: &dyn TicketBackend,
options: ReviewOptions,
) -> Result<TicketCliOutput, TicketCliError> {
let result = options.result.as_str().to_string();
let review = TicketReview {
result: options.result,
author: Some(default_author()),
body: MarkdownText::new(read_body_source(&options.body)?),
};
backend.review(TicketIdOrSlug::Query(options.query.clone()), review)?;
Ok(success(format!(
"reviewed\t{}\t{}\n",
options.query, result
)))
}
fn state(
backend: &dyn TicketBackend,
options: StateOptions,
@@ -660,7 +633,11 @@ fn state(
StateTarget::Ready => TicketWorkflowState::Ready,
StateTarget::Queued => TicketWorkflowState::Queued,
StateTarget::InProgress => TicketWorkflowState::InProgress,
StateTarget::Done => TicketWorkflowState::Done,
StateTarget::Done => {
return Err(TicketCliError::new(
"done is guarded by MergeRequestComplete with an approved immutable revision and operation_id",
));
}
StateTarget::Closed => {
return Err(TicketCliError::new(
"yoi ticket state <ticket> closed cannot write resolution.md; use `yoi ticket close <ticket> --resolution <text>` instead",
@@ -965,64 +942,6 @@ fn parse_comment(args: &[String]) -> Result<CommentOptions, TicketCliError> {
})
}
fn parse_review(args: &[String]) -> Result<ReviewOptions, TicketCliError> {
if args.is_empty() || args[0].starts_with('-') {
return Err(TicketCliError::new("review requires <id>"));
}
let query = args[0].clone();
let mut approve = false;
let mut request_changes = false;
let mut file = None;
let mut message = None;
let mut i = 1;
while i < args.len() {
match args[i].as_str() {
"--approve" => {
approve = true;
i += 1;
}
"--request-changes" => {
request_changes = true;
i += 1;
}
_ => match option_with_value(args, &mut i)? {
Some(("--file", value)) => file = Some(PathBuf::from(value)),
Some(("--message", value)) => message = Some(value),
Some((name, _)) => {
return Err(TicketCliError::new(format!(
"unknown review argument: {name}"
)));
}
None => {
return Err(TicketCliError::new(format!(
"unknown review argument: {}",
args[i]
)));
}
},
}
}
let result = match (approve, request_changes) {
(true, false) => TicketReviewResult::Approve,
(false, true) => TicketReviewResult::RequestChanges,
(false, false) => {
return Err(TicketCliError::new(
"review requires exactly one of --approve or --request-changes",
));
}
(true, true) => {
return Err(TicketCliError::new(
"review accepts exactly one of --approve or --request-changes",
));
}
};
Ok(ReviewOptions {
query,
result,
body: exactly_one_body("review", file, message)?,
})
}
fn parse_state(args: &[String]) -> Result<StateOptions, TicketCliError> {
if args.len() != 2 {
return Err(TicketCliError::new(
@@ -1244,7 +1163,7 @@ fn default_author() -> String {
}
fn help_text() -> &'static str {
"yoi ticket\n\nUsage:\n yoi ticket init\n yoi ticket import-local\n yoi ticket create --title <title>\n yoi ticket list [--state active|all|planning|ready|queued|inprogress|done|closed[,..]] [--limit <n>]\n yoi ticket show <id>\n yoi ticket comment <id> [--role comment|plan|decision|implementation_report] (--file <path>|--message <text>)\n yoi ticket review <id> (--approve|--request-changes) (--file <path>|--message <text>)\n yoi ticket state <id> <planning|ready|queued|inprogress|done|closed>\n yoi ticket close <id> (--resolution <text>|--file <path>)\n yoi ticket relation add --ticket <id> --kind <depends_on|blocks|related|supersedes|duplicate_of> --target <id> [--note <text>]\n yoi ticket relation list [--ticket <id>] [--kind <kind>]\n yoi ticket doctor\n\nOptions:\n -h, --help Print help\n\nBackend:\n Tickets are stored in the workspace SQLite DB under the Yoi data directory.\n `yoi ticket import-local` imports the legacy .yoi/tickets backend root configured in .yoi/workspace.toml.\n `yoi ticket init` writes explicit fixed role profiles and optional [ticket].language into .yoi/workspace.toml, but does not create .yoi/tickets.\n"
"yoi ticket\n\nUsage:\n yoi ticket init\n yoi ticket import-local\n yoi ticket create --title <title>\n yoi ticket list [--state active|all|planning|ready|queued|inprogress|done|closed[,..]] [--limit <n>]\n yoi ticket show <id>\n yoi ticket comment <id> [--role comment|plan|decision|implementation_report] (--file <path>|--message <text>)\n yoi ticket state <id> <planning|ready|queued|inprogress|closed>\n yoi ticket close <id> (--resolution <text>|--file <path>)\n yoi ticket relation add --ticket <id> --kind <depends_on|blocks|related|supersedes|duplicate_of> --target <id> [--note <text>]\n yoi ticket relation list [--ticket <id>] [--kind <kind>]\n yoi ticket doctor\n\nOptions:\n -h, --help Print help\n\nBackend:\n Tickets are stored in the workspace SQLite DB under the Yoi data directory.\n `yoi ticket import-local` imports the legacy .yoi/tickets backend root configured in .yoi/workspace.toml.\n `yoi ticket init` writes explicit fixed role profiles and optional [ticket].language into .yoi/workspace.toml, but does not create .yoi/tickets.\n"
}
#[cfg(test)]
@@ -1375,7 +1294,7 @@ mod tests {
}
#[test]
fn ticket_cli_create_list_show_comment_review_state_close_and_doctor() {
fn ticket_cli_create_list_show_comment_state_close_and_doctor() {
let temp = TempDir::new().unwrap();
let created = run(&temp, &["create", "--title", "CLI Created"]);
@@ -1416,22 +1335,6 @@ mod tests {
.contains(&format!("appended\t{}\timplementation_report", ticket_id))
);
let reviewed = run(
&temp,
&[
"review",
&ticket_id,
"--approve",
"--message",
"Looks good.",
],
);
assert!(
reviewed
.stdout
.contains(&format!("reviewed\t{}\tapprove", ticket_id))
);
let ready = run(&temp, &["state", &ticket_id, "ready"]);
assert_eq!(ready.stdout, format!("state\t{}\tready\n", ticket_id));
let ready_listed = run(&temp, &["list", "--state", "ready"]);
@@ -1450,10 +1353,10 @@ mod tests {
let inprogress_listed = run(&temp, &["list", "--state", "inprogress"]);
assert!(inprogress_listed.stdout.contains(&ticket_id));
let done = run(&temp, &["state", &ticket_id, "done"]);
assert_eq!(done.stdout, format!("state\t{}\tdone\n", ticket_id));
let done_listed = run(&temp, &["list", "--state", "done"]);
assert!(done_listed.stdout.contains(&ticket_id));
let done_error = parse_ticket_args(&args(&["state", &ticket_id, "done"]))
.and_then(|cli| run_in_workspace(cli, temp.path()))
.unwrap_err();
assert!(done_error.to_string().contains("MergeRequestComplete"));
let closed = run(
&temp,
@@ -1469,7 +1372,6 @@ mod tests {
assert!(final_show.stdout.contains("State: closed"));
assert!(final_show.stdout.contains("Done via yoi ticket."));
assert!(final_show.stdout.contains("implementation_report"));
assert!(final_show.stdout.contains("review"));
}
#[test]
@@ -1594,20 +1496,6 @@ mod tests {
assert!(err.to_string().contains("exactly one"));
}
#[test]
fn ticket_cli_rejects_ambiguous_review_result() {
let err = parse_ticket_args(&args(&[
"review",
"ticket",
"--approve",
"--request-changes",
"--message",
"body",
]))
.unwrap_err();
assert!(err.to_string().contains("exactly one"));
}
#[test]
fn ticket_cli_state_closed_requires_close_command() {
let temp = TempDir::new().unwrap();