Compare commits
4
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
f633b86b35 | ||
|
|
3b71fe03b4 | ||
|
|
5e2234763c | ||
|
|
1d140be715 |
@@ -24,7 +24,7 @@ pub fn builtin_flow_source(slug: &str) -> Option<BuiltinFlowSource> {
|
||||
match slug {
|
||||
CODER_REVIEW_FLOW_SLUG => Some(BuiltinFlowSource {
|
||||
slug: CODER_REVIEW_FLOW_SLUG,
|
||||
revision: 3,
|
||||
revision: 4,
|
||||
path: "builtin/flows/coder-review.dcdl",
|
||||
content: CODER_REVIEW_FLOW_SOURCE,
|
||||
}),
|
||||
@@ -35,7 +35,7 @@ pub fn builtin_flow_source(slug: &str) -> Option<BuiltinFlowSource> {
|
||||
pub fn builtin_flow_sources() -> &'static [BuiltinFlowSource] {
|
||||
const SOURCES: &[BuiltinFlowSource] = &[BuiltinFlowSource {
|
||||
slug: CODER_REVIEW_FLOW_SLUG,
|
||||
revision: 3,
|
||||
revision: 4,
|
||||
path: "builtin/flows/coder-review.dcdl",
|
||||
content: CODER_REVIEW_FLOW_SOURCE,
|
||||
}];
|
||||
@@ -46,6 +46,30 @@ pub fn builtin_flow_sources() -> &'static [BuiltinFlowSource] {
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn coder_review_flow_uses_current_selector_ref_review_contract() {
|
||||
let source = builtin_flow_source(CODER_REVIEW_FLOW_SLUG).expect("coder review Flow");
|
||||
for required in [
|
||||
"OpenMergeRequest",
|
||||
"ShowMergeRequest",
|
||||
"ReviewMergeRequest",
|
||||
"CompleteMergeRequest",
|
||||
"existing Merge Request `selector_from`",
|
||||
"Target-only movement does not invalidate",
|
||||
] {
|
||||
assert!(source.content.contains(required), "missing {required}");
|
||||
}
|
||||
for stale in [
|
||||
"MergeRequestOpen",
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReview",
|
||||
"MergeRequestComplete",
|
||||
"new immutable revision",
|
||||
] {
|
||||
assert!(!source.content.contains(stale), "stale contract {stale}");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn every_builtin_flow_compiles_and_matches_catalog_identity() {
|
||||
assert!(!builtin_flow_sources().is_empty());
|
||||
|
||||
@@ -416,7 +416,7 @@ impl MergeRequestStore {
|
||||
let conflict:bool=t.query_row("SELECT EXISTS(SELECT 1 FROM merge_request_ticket_relations rel JOIN merge_requests mr ON mr.workspace_id=rel.workspace_id AND mr.merge_request_id=rel.merge_request_id WHERE rel.workspace_id=?1 AND rel.ticket_id=?2 AND mr.state='open')",params![i.auth.workspace_id,i.ticket_id],|r|r.get(0))?;
|
||||
if conflict {
|
||||
return Err(MergeRequestError::Conflict(
|
||||
"Ticket already has an open Merge Request".into(),
|
||||
"Ticket already has an open Merge Request; use ShowMergeRequest and advance the existing selector_from with a normal non-force push instead of opening a replacement Merge Request or adding a revision".into(),
|
||||
));
|
||||
}
|
||||
let now = i.now.to_rfc3339();
|
||||
@@ -575,12 +575,16 @@ impl MergeRequestStore {
|
||||
));
|
||||
};
|
||||
if subject != i.current_subject_ref {
|
||||
let reason = format!(
|
||||
"selector_from moved from requested subject {subject} to current subject {}; fresh review of the exact current source ref is required",
|
||||
i.current_subject_ref
|
||||
);
|
||||
let e = ReviewCancelledEvent {
|
||||
event_id: Uuid::now_v7().to_string(),
|
||||
sequence: next_seq(&t, &ws, &mr)?,
|
||||
request_event_id: req,
|
||||
subject_ref: subject,
|
||||
reason: "selector_from moved before submission".into(),
|
||||
reason,
|
||||
created_at: i.now,
|
||||
};
|
||||
insert_event(&t, &ws, &mr, "review_cancelled", &e, i.now, None)?;
|
||||
@@ -667,9 +671,27 @@ impl MergeRequestStore {
|
||||
}
|
||||
match (&i.current_subject_ref, &review) {
|
||||
(None, _) => b.push("selector_from could not be resolved".into()),
|
||||
(Some(_), None) => b.push("current source ref has no valid review".into()),
|
||||
(_, Some(r)) if r.decision == ReviewDecision::RequestChanges => {
|
||||
b.push("current source ref requests changes".into())
|
||||
(Some(subject_ref), None) => {
|
||||
let previous_review_subject = mr.thread.iter().rev().find_map(|event| match event {
|
||||
MergeRequestThreadEvent::ReviewRequested(value) => {
|
||||
Some(value.subject_ref.as_str())
|
||||
}
|
||||
MergeRequestThreadEvent::Review(value) => Some(value.subject_ref.as_str()),
|
||||
_ => None,
|
||||
});
|
||||
match previous_review_subject.filter(|previous| *previous != subject_ref) {
|
||||
Some(previous) => b.push(format!(
|
||||
"selector_from moved from reviewed/requested subject {previous} to current subject {subject_ref}; request a fresh review for this exact source ref (selector_to movement alone does not invalidate source approval)"
|
||||
)),
|
||||
None => b.push(format!(
|
||||
"current source ref {subject_ref} has no valid review; request a fresh review for this exact source ref"
|
||||
)),
|
||||
}
|
||||
}
|
||||
(Some(subject_ref), Some(r)) if r.decision == ReviewDecision::RequestChanges => {
|
||||
b.push(format!(
|
||||
"current source ref {subject_ref} requests changes; advance the existing selector_from with a normal non-force push, then request a fresh review for the exact new source ref"
|
||||
))
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
|
||||
@@ -181,16 +181,85 @@ fn source_move_cancels_submission_and_old_approval_is_reusable_when_source_retur
|
||||
.is_err()
|
||||
);
|
||||
let mr = s.get("W", "T").unwrap();
|
||||
let cancellation = mr.thread.iter().find_map(|event| match event {
|
||||
MergeRequestThreadEvent::ReviewCancelled(value) => Some(value),
|
||||
_ => None,
|
||||
});
|
||||
assert!(
|
||||
mr.thread
|
||||
.iter()
|
||||
.any(|e| matches!(e, MergeRequestThreadEvent::ReviewCancelled(_)))
|
||||
cancellation
|
||||
.as_ref()
|
||||
.is_some_and(|value| value.reason.contains("selector_from moved")
|
||||
&& value.reason.contains("fresh review"))
|
||||
);
|
||||
assert_eq!(
|
||||
mr.effective_review("source-a").map(|r| &r.event_id),
|
||||
Some(&approved.event_id)
|
||||
);
|
||||
}
|
||||
#[test]
|
||||
fn same_selector_source_advancement_requires_fresh_review_and_preserves_target_only_approval() {
|
||||
let (_d, s) = fixture();
|
||||
open(&s);
|
||||
let first = approve(&s, "source-1", "one");
|
||||
|
||||
let stale = s
|
||||
.readiness(ReadinessCheck {
|
||||
ticket_id: "T".into(),
|
||||
current_subject_ref: Some("source-2".into()),
|
||||
auth: auth(),
|
||||
})
|
||||
.unwrap();
|
||||
assert!(!stale.ready);
|
||||
assert!(stale.review.is_none());
|
||||
assert!(stale.blockers.iter().any(|blocker| {
|
||||
blocker.contains("selector_from moved from reviewed/requested subject source-1")
|
||||
&& blocker.contains("current subject source-2")
|
||||
&& blocker.contains("fresh review")
|
||||
}));
|
||||
assert_eq!(
|
||||
s.get("W", "T")
|
||||
.unwrap()
|
||||
.effective_review("source-1")
|
||||
.map(|review| &review.event_id),
|
||||
Some(&first.event_id)
|
||||
);
|
||||
|
||||
let second = approve(&s, "source-2", "two");
|
||||
let ready = s
|
||||
.readiness(ReadinessCheck {
|
||||
ticket_id: "T".into(),
|
||||
current_subject_ref: Some("source-2".into()),
|
||||
auth: auth(),
|
||||
})
|
||||
.unwrap();
|
||||
assert!(ready.ready);
|
||||
assert_eq!(
|
||||
ready.review.as_ref().map(|review| &review.event_id),
|
||||
Some(&second.event_id)
|
||||
);
|
||||
|
||||
// The target can move from target-1 to target-2 without changing selector_from
|
||||
// or invalidating the exact-source approval. Completion consumes refreshed
|
||||
// integration evidence for the current target pair.
|
||||
let merged = s
|
||||
.complete(CompleteMergeRequest {
|
||||
operation_id: "target-moved".into(),
|
||||
ticket_id: "T".into(),
|
||||
current_subject_ref: "source-2".into(),
|
||||
target_ref_before: "target-2".into(),
|
||||
target_ref_after: "integrated-target-2".into(),
|
||||
approval_event_id: second.event_id,
|
||||
strategy: MergeStrategy::FastForward,
|
||||
resolution: ConflictResolution::None,
|
||||
auth: auth(),
|
||||
now: at(5),
|
||||
})
|
||||
.unwrap();
|
||||
assert_eq!(merged.approved_source_ref, "source-2");
|
||||
assert_eq!(merged.target_ref_before, "target-2");
|
||||
assert_eq!(merged.target_ref_after, "integrated-target-2");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn review_revocation_invalidates_readiness() {
|
||||
let (_d, s) = fixture();
|
||||
|
||||
@@ -50,11 +50,11 @@ struct MergeRequestTool {
|
||||
kind: Kind,
|
||||
}
|
||||
#[derive(Debug, Deserialize, JsonSchema)]
|
||||
struct ShowInput {
|
||||
struct TicketInput {
|
||||
ticket: String,
|
||||
}
|
||||
#[derive(Debug, Deserialize, JsonSchema)]
|
||||
struct OpenInput {
|
||||
struct OpenMergeRequestInput {
|
||||
ticket: String,
|
||||
repository_id: String,
|
||||
selector_from: String,
|
||||
@@ -63,7 +63,7 @@ struct OpenInput {
|
||||
summary: String,
|
||||
}
|
||||
#[derive(Debug, Deserialize, JsonSchema)]
|
||||
struct CompleteInput {
|
||||
struct CompleteMergeRequestInput {
|
||||
ticket: String,
|
||||
operation_id: String,
|
||||
approval_event_id: String,
|
||||
@@ -86,7 +86,7 @@ enum MergeResolutionInput {
|
||||
ConflictsResolved,
|
||||
}
|
||||
#[derive(Debug, Deserialize, JsonSchema)]
|
||||
struct ReviewInput {
|
||||
struct ReviewMergeRequestInput {
|
||||
decision: ReviewDecisionInput,
|
||||
#[serde(default)]
|
||||
body: String,
|
||||
@@ -133,19 +133,19 @@ impl Kind {
|
||||
|
||||
fn name(self) -> &'static str {
|
||||
match self {
|
||||
Self::Show => "MergeRequestShow",
|
||||
Self::Readiness => "MergeRequestReadinessCheck",
|
||||
Self::Open => "MergeRequestOpen",
|
||||
Self::Complete => "MergeRequestComplete",
|
||||
Self::Review => "MergeRequestReview",
|
||||
Self::Show => "ShowMergeRequest",
|
||||
Self::Readiness => "CheckMergeRequestReadiness",
|
||||
Self::Open => "OpenMergeRequest",
|
||||
Self::Complete => "CompleteMergeRequest",
|
||||
Self::Review => "ReviewMergeRequest",
|
||||
}
|
||||
}
|
||||
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::Complete => json!(schemars::schema_for!(CompleteInput)),
|
||||
Self::Review => json!(schemars::schema_for!(ReviewInput)),
|
||||
Self::Show | Self::Readiness => json!(schemars::schema_for!(TicketInput)),
|
||||
Self::Open => json!(schemars::schema_for!(OpenMergeRequestInput)),
|
||||
Self::Complete => json!(schemars::schema_for!(CompleteMergeRequestInput)),
|
||||
Self::Review => json!(schemars::schema_for!(ReviewMergeRequestInput)),
|
||||
}
|
||||
}
|
||||
}
|
||||
@@ -156,13 +156,13 @@ impl Tool for MergeRequestTool {
|
||||
ToolError::ExecutionFailed("Merge Request tools require Workspace identity".into())
|
||||
})?;
|
||||
if matches!(self.kind, Kind::Show) {
|
||||
let value: ShowInput = parse(input)?;
|
||||
let value: TicketInput = parse(input)?;
|
||||
nonempty(&value.ticket)?;
|
||||
return self.show_current_merge_request(ws, &value.ticket);
|
||||
}
|
||||
let (method, path, body) = match self.kind {
|
||||
Kind::Readiness => {
|
||||
let v: ShowInput = parse(input)?;
|
||||
let v: TicketInput = parse(input)?;
|
||||
nonempty(&v.ticket)?;
|
||||
(
|
||||
WorkspaceRequestMethod::Get,
|
||||
@@ -170,9 +170,9 @@ impl Tool for MergeRequestTool {
|
||||
None,
|
||||
)
|
||||
}
|
||||
Kind::Show => unreachable!("MergeRequestShow is handled above"),
|
||||
Kind::Show => unreachable!("ShowMergeRequest is handled above"),
|
||||
Kind::Open => {
|
||||
let v: OpenInput = parse(input)?;
|
||||
let v: OpenMergeRequestInput = parse(input)?;
|
||||
nonempty(&v.ticket)?;
|
||||
(
|
||||
WorkspaceRequestMethod::Post,
|
||||
@@ -183,7 +183,7 @@ impl Tool for MergeRequestTool {
|
||||
)
|
||||
}
|
||||
Kind::Complete => {
|
||||
let v: CompleteInput = parse(input)?;
|
||||
let v: CompleteMergeRequestInput = parse(input)?;
|
||||
nonempty(&v.ticket)?;
|
||||
(
|
||||
WorkspaceRequestMethod::Post,
|
||||
@@ -194,7 +194,7 @@ impl Tool for MergeRequestTool {
|
||||
)
|
||||
}
|
||||
Kind::Review => {
|
||||
let v: ReviewInput = parse(input)?;
|
||||
let v: ReviewMergeRequestInput = parse(input)?;
|
||||
let ctx = self.client.reviewer_context().ok_or_else(|| {
|
||||
ToolError::ExecutionFailed(
|
||||
"Review submit requires injected Reviewer capability".into(),
|
||||
@@ -397,19 +397,21 @@ impl FeatureModule for MergeRequestFeature {
|
||||
|
||||
pub fn description(n: &str) -> Option<&'static str> {
|
||||
match n {
|
||||
"MergeRequestShow" => Some("Read the selector-based Merge Request and append-only thread."),
|
||||
"MergeRequestReadinessCheck" => {
|
||||
Some("Resolve current provider refs and derive readiness from valid review events.")
|
||||
}
|
||||
"MergeRequestOpen" => {
|
||||
Some("Open a Merge Request with immutable source and target selectors.")
|
||||
}
|
||||
"MergeRequestComplete" => {
|
||||
Some("Complete using an approved review event and final target-ref evidence.")
|
||||
}
|
||||
"MergeRequestReview" => {
|
||||
Some("Submit the injected Reviewer capability result for its captured subject ref.")
|
||||
}
|
||||
"ShowMergeRequest" => Some(
|
||||
"Read the selector-based Merge Request, append-only thread, source-review freshness, and target-integration evidence before review, fix, or handoff decisions.",
|
||||
),
|
||||
"CheckMergeRequestReadiness" => Some(
|
||||
"Resolve current provider refs and derive readiness from exact-source review evidence; source movement requires fresh review while target-only movement preserves unchanged-source approval.",
|
||||
),
|
||||
"OpenMergeRequest" => Some(
|
||||
"Open the Ticket's one Merge Request with immutable source and target selectors; reuse it and advance only selector_from with a normal non-force push for later fixes.",
|
||||
),
|
||||
"CompleteMergeRequest" => Some(
|
||||
"Record Orchestrator-owned integration using unchanged-source approval and refreshed final target-ref evidence.",
|
||||
),
|
||||
"ReviewMergeRequest" => Some(
|
||||
"Submit the injected Reviewer capability result for its captured exact source ref; source movement cancels it, while target-only movement does not.",
|
||||
),
|
||||
_ => None,
|
||||
}
|
||||
}
|
||||
@@ -469,6 +471,31 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn model_facing_operations_use_only_verb_first_names() {
|
||||
for name in [
|
||||
"ShowMergeRequest",
|
||||
"CheckMergeRequestReadiness",
|
||||
"OpenMergeRequest",
|
||||
"CompleteMergeRequest",
|
||||
"ReviewMergeRequest",
|
||||
] {
|
||||
assert!(description(name).is_some(), "missing operation {name}");
|
||||
}
|
||||
for legacy in [
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReadinessCheck",
|
||||
"MergeRequestOpen",
|
||||
"MergeRequestComplete",
|
||||
"MergeRequestReview",
|
||||
] {
|
||||
assert!(
|
||||
description(legacy).is_none(),
|
||||
"legacy alias {legacy} must not remain registered"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[tokio::test]
|
||||
async fn show_resolves_ticket_projection_then_reads_canonical_resource() {
|
||||
let client = Arc::new(RecordingWorkspaceClient::new(vec![
|
||||
@@ -546,7 +573,7 @@ mod tests {
|
||||
open: true,
|
||||
..Default::default()
|
||||
};
|
||||
assert_eq!(tool_names(coder), ["MergeRequestShow", "MergeRequestOpen"]);
|
||||
assert_eq!(tool_names(coder), ["ShowMergeRequest", "OpenMergeRequest"]);
|
||||
|
||||
let reviewer = MergeRequestFeatureConfig {
|
||||
show: true,
|
||||
@@ -555,7 +582,7 @@ mod tests {
|
||||
};
|
||||
assert_eq!(
|
||||
tool_names(reviewer),
|
||||
["MergeRequestShow", "MergeRequestReview"]
|
||||
["ShowMergeRequest", "ReviewMergeRequest"]
|
||||
);
|
||||
|
||||
let orchestrator = MergeRequestFeatureConfig {
|
||||
@@ -567,9 +594,9 @@ mod tests {
|
||||
assert_eq!(
|
||||
tool_names(orchestrator),
|
||||
[
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReadinessCheck",
|
||||
"MergeRequestComplete"
|
||||
"ShowMergeRequest",
|
||||
"CheckMergeRequestReadiness",
|
||||
"CompleteMergeRequest"
|
||||
]
|
||||
);
|
||||
assert_eq!(install(coder).1, [FEATURE_PROMPT_REF]);
|
||||
@@ -581,8 +608,8 @@ mod tests {
|
||||
#[test]
|
||||
fn schemas_hide_revision_and_commit_authority() {
|
||||
let schemas = [
|
||||
schemars::schema_for!(OpenInput),
|
||||
schemars::schema_for!(CompleteInput),
|
||||
schemars::schema_for!(OpenMergeRequestInput),
|
||||
schemars::schema_for!(CompleteMergeRequestInput),
|
||||
];
|
||||
for s in schemas {
|
||||
let j = serde_json::to_string(&s).unwrap();
|
||||
|
||||
@@ -609,6 +609,47 @@ mod tests {
|
||||
assert!(!prompt.contains("use the Ticket repository `origin` transport"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn merge_request_prompts_use_selector_refs_and_verb_first_operations() {
|
||||
let catalog = PromptCatalog::builtins_only().unwrap();
|
||||
let templates = &catalog.projection.templates;
|
||||
let common = &templates["common.merge_request"];
|
||||
let coder = &templates["role.coder"];
|
||||
let orchestrator = &templates["role.orchestrator"];
|
||||
let reviewer = &templates["role.reviewer"];
|
||||
let combined = format!("{common}\n{coder}\n{orchestrator}\n{reviewer}");
|
||||
|
||||
for operation in [
|
||||
"OpenMergeRequest",
|
||||
"ShowMergeRequest",
|
||||
"ReviewMergeRequest",
|
||||
"CheckMergeRequestReadiness",
|
||||
"CompleteMergeRequest",
|
||||
] {
|
||||
assert!(combined.contains(operation), "missing {operation}");
|
||||
}
|
||||
for stale_operation in [
|
||||
"MergeRequestOpen",
|
||||
"MergeRequestShow",
|
||||
"MergeRequestReview",
|
||||
"MergeRequestReadinessCheck",
|
||||
"MergeRequestComplete",
|
||||
"MergeRequestAddRevision",
|
||||
] {
|
||||
assert!(
|
||||
!combined.contains(stale_operation),
|
||||
"stale operation {stale_operation} remains in prompt authority"
|
||||
);
|
||||
}
|
||||
assert!(common.contains("selector_from"));
|
||||
assert!(common.contains("normal non-force push"));
|
||||
assert!(common.contains("Moving only the target ref does not invalidate approval"));
|
||||
assert!(common.contains("take precedence over stale Memory"));
|
||||
assert!(coder.contains("Never invent an add-revision operation"));
|
||||
assert!(orchestrator.contains("Target-only movement preserves source approval"));
|
||||
assert!(reviewer.contains("target-only movement does not invalidate approval"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn graph_rejects_dynamic_legacy_missing_and_cycles() {
|
||||
let invalid = BTreeMap::from([
|
||||
|
||||
@@ -4451,7 +4451,7 @@ async fn scoped_transition_ticket_state(
|
||||
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(),
|
||||
"done is guarded by CompleteMergeRequest with an approved exact source ref and operation_id".to_string(),
|
||||
).into());
|
||||
}
|
||||
let current = api.authority.ticket(&path.id)?;
|
||||
@@ -4573,7 +4573,7 @@ fn reject_unguarded_ticket_completion(operation: &TicketBackendOperation) -> Res
|
||||
reject_unguarded_ticket_start(operation)?;
|
||||
if generic_ticket_state_change(operation).is_some_and(|change| change.to == "done") {
|
||||
return Err(Error::TicketAssignmentConflict(
|
||||
"done is guarded by MergeRequestComplete with an approved immutable revision and operation_id".to_string(),
|
||||
"done is guarded by CompleteMergeRequest with an approved exact source ref and operation_id".to_string(),
|
||||
));
|
||||
}
|
||||
Ok(())
|
||||
@@ -5229,7 +5229,7 @@ fn require_completed_target_observation(
|
||||
}
|
||||
if observed == target_ref_before {
|
||||
return Err(Error::InvalidInput(
|
||||
"target selector is still at target_ref_before; push the verified result from the Orchestrator Workdir before MergeRequestComplete".into(),
|
||||
"target selector is still at target_ref_before; Orchestrator must integrate the approved source into the current target and verify the resulting target ref before CompleteMergeRequest (target-only movement does not invalidate approval for an unchanged source)".into(),
|
||||
)
|
||||
.into());
|
||||
}
|
||||
@@ -14696,7 +14696,7 @@ mod tests {
|
||||
},
|
||||
] {
|
||||
let error = reject_unguarded_ticket_completion(&operation).unwrap_err();
|
||||
assert!(error.to_string().contains("MergeRequestComplete"));
|
||||
assert!(error.to_string().contains("CompleteMergeRequest"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -14901,7 +14901,7 @@ mod tests {
|
||||
assert_eq!(builtin.definition.name, "coder-review");
|
||||
assert_eq!(builtin.selector.to_string(), "builtin:coder-review");
|
||||
assert_eq!(builtin.flow_id, "builtin:coder-review");
|
||||
assert_eq!(builtin.revision, 3);
|
||||
assert_eq!(builtin.revision, 4);
|
||||
assert_eq!(
|
||||
api.store
|
||||
.list_flow_sources(&api.config.workspace_id)
|
||||
@@ -15077,7 +15077,8 @@ mod tests {
|
||||
assert!(matches!(
|
||||
not_pushed.error,
|
||||
Error::InvalidInput(ref message)
|
||||
if message.contains("push the verified result from the Orchestrator Workdir")
|
||||
if message.contains("Orchestrator must integrate the approved source into the current target")
|
||||
&& message.contains("target-only movement does not invalidate approval")
|
||||
));
|
||||
|
||||
let moved = require_completed_target_observation("other", "before", "after").unwrap_err();
|
||||
|
||||
@@ -671,7 +671,7 @@ fn state(
|
||||
StateTarget::InProgress => TicketWorkflowState::InProgress,
|
||||
StateTarget::Done => {
|
||||
return Err(TicketCliError::new(
|
||||
"done is guarded by MergeRequestComplete with an approved immutable revision and operation_id",
|
||||
"done is guarded by CompleteMergeRequest with an approved exact source ref and operation_id",
|
||||
));
|
||||
}
|
||||
StateTarget::Closed => {
|
||||
@@ -1366,7 +1366,7 @@ mod tests {
|
||||
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"));
|
||||
assert!(done_error.to_string().contains("CompleteMergeRequest"));
|
||||
|
||||
let closed = run(
|
||||
&temp,
|
||||
|
||||
@@ -37,9 +37,9 @@ Workers with the Ticket and operation-specific Merge Request built-in features c
|
||||
- `QueryTicket` — bounded authoritative Ticket discovery with typed state/text/event/evidence/relation/Objective/time/attention filters, stable snippets, and cursor metadata.
|
||||
- `ShowTicket` — detailed authority for one Ticket, including item revision, bounded thread/event references, relations, linked Objectives, implementation reports, and current Merge Request/review evidence.
|
||||
- `TicketComment`
|
||||
- Coder: `MergeRequestShow`, `MergeRequestOpen`
|
||||
- Reviewer: `MergeRequestShow`, `MergeRequestReview` — available only inside the attested direct-child Reviewer request; grant and subject-ref capability material are not model input.
|
||||
- Orchestrator: `MergeRequestShow`, `MergeRequestReadinessCheck`, `MergeRequestComplete`
|
||||
- Coder: `ShowMergeRequest`, `OpenMergeRequest`
|
||||
- Reviewer: `ShowMergeRequest`, `ReviewMergeRequest` — available only inside the attested direct-child Reviewer request; grant and subject-ref capability material are not model input.
|
||||
- Orchestrator: `ShowMergeRequest`, `CheckMergeRequestReadiness`, `CompleteMergeRequest`
|
||||
- `TicketClose`
|
||||
- `TicketRelationRecord`
|
||||
|
||||
@@ -244,7 +244,7 @@ Implementation normally happens in a child git worktree created by the Orchestra
|
||||
|
||||
The assigned Coder launches the Reviewer as an actual direct-child `builtin:reviewer` SubWorker with write scope, so it can use the Workdir command tools required for inspection and validation, and a structured handoff bound to the current immutable Merge Request revision. Server authority revalidates the parent assignment, Runtime-owned child session, effective profile, one-shot review attempt, and revision; prose output is not approval.
|
||||
|
||||
The Reviewer records the structured result with `MergeRequestReview`. Request changes requires a new immutable revision and a fresh child attempt. The Orchestrator uses `MergeRequestReadinessCheck` and then `MergeRequestComplete` for guarded integration with operation-id dedupe/CAS semantics; Flow transitions are not completion authority.
|
||||
The Reviewer records the structured result with `ReviewMergeRequest`. Request changes advances the existing Merge Request source selector with a normal non-force push and requires a fresh child attempt for that exact new source ref; do not create a replacement Merge Request, add-revision operation, or fresh integration branch. Target-only movement preserves approval for an unchanged source and requires refreshed integration evidence. The Orchestrator uses `CheckMergeRequestReadiness` and then `CompleteMergeRequest` for guarded integration with operation-id dedupe/CAS semantics; Flow transitions are not completion authority.
|
||||
|
||||
Blockers must be fixed or explicitly escalated before merge-ready submission.
|
||||
|
||||
|
||||
@@ -2,9 +2,9 @@
|
||||
|
||||
## Observed
|
||||
|
||||
While implementing Ticket `00001KZRNHB35`, the assigned Coder had a clean committed Workdir and all immutable base/head/changed-path evidence needed by `MergeRequestOpen`.
|
||||
While implementing Ticket `00001KZRNHB35`, the assigned Coder had a clean committed Workdir and all immutable base/head/changed-path evidence needed by `OpenMergeRequest`.
|
||||
|
||||
`MergeRequestOpen` rejected candidate repository ids with:
|
||||
`OpenMergeRequest` rejected candidate repository ids with:
|
||||
|
||||
```text
|
||||
invalid input: Merge Request repository must match the authoritative Ticket target
|
||||
@@ -14,14 +14,14 @@ The typed `TicketShow` result available to the Coder rendered only the Ticket id
|
||||
|
||||
## Impact
|
||||
|
||||
A Coder can finish and validate implementation but cannot open the required immutable MR revision or start independent review. The failure is safe, but it strands otherwise review-ready work and provides no actionable expected target.
|
||||
A Coder can finish and validate implementation but cannot open the required selector-based Merge Request or start independent review. The failure is safe, but it strands otherwise review-ready work and provides no actionable expected target.
|
||||
|
||||
## Suggested improvement
|
||||
|
||||
At least one trusted read surface in the assigned-Coder flow should return the immutable Ticket target needed by `MergeRequestOpen`:
|
||||
At least one trusted read surface in the assigned-Coder flow should return the immutable Ticket target needed by `OpenMergeRequest`:
|
||||
|
||||
- include `repository_id` and `ref_selector` in `TicketShow`'s bounded authoritative projection; or
|
||||
- have `MergeRequestOpen` derive repository identity from the authoritative Ticket target and remove it from model input; or
|
||||
- have `OpenMergeRequest` derive repository identity from the authoritative Ticket target and remove it from model input; or
|
||||
- return a bounded structured mismatch diagnostic containing the authoritative repository id when the caller is already authorized to read that Ticket.
|
||||
|
||||
Deriving the repository in `MergeRequestOpen` is preferable because it removes duplicated model-controlled identity and avoids target drift between Ticket read and MR creation.
|
||||
Deriving the repository in `OpenMergeRequest` is preferable because it removes duplicated model-controlled identity and avoids target drift between Ticket read and MR creation.
|
||||
|
||||
@@ -5,51 +5,51 @@
|
||||
|
||||
states = {
|
||||
implement = {
|
||||
instructions = "Inspect the assigned Workdir Git state before editing. Reuse a suitable restored `work/<ticket-id>-<slug>` branch, or create a collision-safe work branch from detached HEAD; never overwrite an existing branch. For this assigned Ticket Workdir, you are explicitly authorized to create or switch the local work branch and to use `git add` and `git commit`. Implement the requested Ticket scope, run the narrow and dependent validation required by the changed contracts, and record concrete evidence in coherent commits. After the implementation is committed, validated, and clean, publish only the current Ticket work branch to the Ticket repository remote with a normal non-force push, then verify that the published source selector resolves to the exact local HEAD. Do not push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Open or update the Ticket Merge Request with immutable `selector_from` / `selector_to` revision evidence. Do not request review from a dirty Workdir or an unpublished source ref. A Flow transition is never Ticket completion authority.";
|
||||
instructions = "Inspect the assigned Workdir Git state before editing. Reuse a suitable restored `work/<ticket-id>-<slug>` branch, or create a collision-safe work branch from detached HEAD; never overwrite an existing branch. For this assigned Ticket Workdir, you are explicitly authorized to create or switch the local work branch and to use `git add` and `git commit`. Implement the requested Ticket scope, run the narrow and dependent validation required by the changed contracts, and record concrete evidence in coherent commits. After the implementation is committed, validated, and clean, publish only the current Ticket work branch to the Ticket repository remote with a normal non-force push, then verify that the provider resolves the existing Merge Request `selector_from` to exact local `HEAD`. Do not push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Open one Merge Request with `OpenMergeRequest`, or use `ShowMergeRequest` when one is already open; keep its original selectors and advance the same source ref. Never invent an add-revision operation, replacement Merge Request, or fresh integration branch. Source movement requires fresh review; target-only movement does not invalidate source approval and remains Orchestrator integration authority. Do not request review from a dirty Workdir or unpublished source ref. A Flow transition is never Ticket completion authority.";
|
||||
transitions = {
|
||||
review = {
|
||||
target = "review";
|
||||
condition = "The requested implementation is present on the Ticket work branch, all intended changes are committed, the Workdir is clean, the relevant validation has completed, the current Ticket work branch has been published with a normal non-force push, the configured repository provider resolves that published source ref to the exact current HEAD, and the linked Merge Request current revision records that same subject for independent review.";
|
||||
condition = "The requested implementation is present on the Ticket work branch, all intended changes are committed, the Workdir is clean, the relevant validation has completed, the existing Merge Request source ref has been published with a normal non-force push, and the configured repository provider resolves that source ref to exact current HEAD for independent review.";
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
review = {
|
||||
instructions = "Use the current Ticket Merge Request as review authority. Confirm its immutable source selector resolves to the exact committed implementation HEAD, then spawn one actual direct-child SubWorker with profile builtin:reviewer, write scope for Workdir inspection and command validation, and a structured review handoff bound to the current immutable Merge Request revision. The trusted spawn layer records `ReviewRequested`; do not place commit/ref identity, capability material, or a prewritten verdict in model input. The child must commit MergeRequestReview; prose output and Worker observation are not approval authority. After the structured current-revision result exists, request a Flow transition.";
|
||||
instructions = "Use the current Ticket Merge Request as review authority. Call `ShowMergeRequest` and confirm its source selector resolves to exact committed implementation HEAD, then spawn one actual direct-child SubWorker with profile builtin:reviewer, write scope for Workdir inspection and command validation, and only the Ticket id in the structured review handoff. The trusted spawn layer records `ReviewRequested` with the exact source ref and injects review capability; do not place commit/ref identity, capability material, or a prewritten verdict in model input. The child must commit `ReviewMergeRequest`; prose output and Worker observation are not approval authority. After the structured result for the exact current source ref exists, request a Flow transition.";
|
||||
transitions = {
|
||||
approved = {
|
||||
target = "complete";
|
||||
condition = "The authoritative Merge Request current revision has a structured approve result from its registered direct-child builtin:reviewer attempt, with no later unresolved request_changes finding. The Flow transition itself does not complete the Ticket.";
|
||||
condition = "The authoritative Merge Request has a structured approve result for its exact current source ref from its registered direct-child builtin:reviewer attempt, with no later unresolved request_changes finding. The Flow transition itself does not complete the Ticket.";
|
||||
};
|
||||
changes_requested = {
|
||||
target = "fix";
|
||||
condition = "The latest independent Reviewer attempt for the current implementation requested one or more concrete changes that remain unresolved.";
|
||||
condition = "The latest independent Reviewer attempt for the exact current source ref requested one or more concrete changes that remain unresolved.";
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
fix = {
|
||||
instructions = "Resolve every open Reviewer finding on the same Ticket work branch, rerun the validation affected by the fixes, commit the corrected implementation as a new revision, and preserve concrete evidence. Publish only the updated Ticket work branch with a normal non-force push, verify that the configured repository provider resolves the published source ref to the exact new HEAD, and update the linked Merge Request so its current revision records that same subject. Request review from a fresh Reviewer child with write scope so it can use the Workdir command tools required for inspection and validation while the trusted spawn layer captures the new immutable subject. Do not rewrite the previously reviewed commit, claim approval from the prior request_changes review, push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Request a Flow transition only after the corrected committed revision is published and ready for a new independent review.";
|
||||
instructions = "Resolve every open Reviewer finding on the same Ticket work branch and existing Merge Request source selector, rerun validation affected by the fixes, and commit the corrected implementation. Publish only that updated source ref with a normal non-force push and verify that the provider resolves it to exact new HEAD. Source movement invalidates the prior verdict, so request a fresh Reviewer child after publication. Do not open a replacement Merge Request, invent an add-revision operation, create a fresh integration branch, rewrite previously reviewed commits, reuse the prior request_changes result as approval, push the target branch, push tags or unrelated refs, force-push, merge, delete branches, or discard pre-existing changes. Request a Flow transition only after the corrected source ref is published and ready for fresh independent review.";
|
||||
transitions = {
|
||||
review = {
|
||||
target = "review";
|
||||
condition = "Every finding from the latest request_changes review has been addressed with relevant validation evidence, the corrected implementation is committed and clean, the updated Ticket work branch has been published with a normal non-force push, the configured repository provider resolves that published source ref to the exact new HEAD, and the linked Merge Request current revision records that same subject for a fresh independent Reviewer attempt.";
|
||||
condition = "Every finding from the latest request_changes review has been addressed with relevant validation evidence, the corrected implementation is committed and clean, the existing Merge Request source ref has been updated with a normal non-force push, and the configured repository provider resolves that source ref to exact new HEAD for a fresh independent Reviewer attempt.";
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
complete = {
|
||||
instructions = "Verify the authoritative approval still matches the exact current Merge Request subject, keep the reviewed source ref immutable, leave concise implementation and validation evidence on the Ticket when useful, then hand off to the Orchestrator. Do not call MergeRequestComplete, update the target selector, or treat the Flow terminal state as Ticket completion authority. After durable handoff evidence exists, request a Flow transition.";
|
||||
instructions = "Call `ShowMergeRequest` and verify that authoritative approval still matches the exact current source ref, keep that reviewed source ref immutable, leave concise implementation and validation evidence on the Ticket when useful, then hand off to the Orchestrator. Target-only movement does not invalidate this source approval; the Orchestrator refreshes readiness/integration evidence against the current target. Do not call `CompleteMergeRequest`, update the target selector, create an integration branch, or treat the Flow terminal state as Ticket completion authority. After durable handoff evidence exists, request a Flow transition.";
|
||||
transitions = {
|
||||
completed = {
|
||||
target = "done";
|
||||
condition = "The exact approved Merge Request revision and implementation evidence have been durably handed off to the Orchestrator. A Flow state or prose report alone is never Ticket completion authority.";
|
||||
condition = "The exact approved Merge Request source ref and implementation evidence have been durably handed off to the Orchestrator. A Flow state or prose report alone is never Ticket completion authority.";
|
||||
};
|
||||
};
|
||||
};
|
||||
|
||||
done = {
|
||||
instructions = "The approved implementation has been handed off for Orchestrator-owned readiness and integration. Flow terminal state only reflects that handoff.";
|
||||
instructions = "The approved implementation has been handed off for Orchestrator-owned readiness, target integration, and `CompleteMergeRequest`. Flow terminal state only reflects that handoff.";
|
||||
terminal = true;
|
||||
};
|
||||
};
|
||||
|
||||
@@ -1,19 +1,9 @@
|
||||
## Merge Request workflow
|
||||
|
||||
Use only the exposed Merge Request operations; their availability expresses this Worker's workflow responsibility, not authorization to bypass Backend validation.
|
||||
{% if "MergeRequestShow" in tools %}
|
||||
- Reread the current Merge Request and append-only thread with `MergeRequestShow` before making review or integration decisions.
|
||||
{% endif %}
|
||||
{% if "MergeRequestOpen" in tools %}
|
||||
- Open the Merge Request only after all intended changes are committed and the Workdir is clean. Use immutable source and target selectors; do not infer target authority from a branch name or cwd.
|
||||
- Before requesting independent review, make the exact current MR revision authoritative.
|
||||
{% endif %}
|
||||
{% if "MergeRequestReview" in tools %}
|
||||
- Review the exact current immutable MR revision independently. Submit the authoritative verdict through `MergeRequestReview`; prose alone is not approval.
|
||||
{% endif %}
|
||||
{% if "MergeRequestReadinessCheck" in tools %}
|
||||
- Use `MergeRequestReadinessCheck` to resolve current refs and authoritative review readiness before integration.
|
||||
{% endif %}
|
||||
{% if "MergeRequestComplete" in tools %}
|
||||
- Complete integration only after readiness confirms approval for the exact current revision and all target/ref guards pass. Merge completion is separate from implementation and review evidence.
|
||||
{% endif %}
|
||||
The Merge Request and its append-only thread are the routine authority for review requests, verdicts, fixes, rereview, readiness, and completion evidence. Use the operation-specific tools exposed to your role: `OpenMergeRequest`, `ShowMergeRequest`, `ReviewMergeRequest`, `CheckMergeRequestReadiness`, and `CompleteMergeRequest`.
|
||||
|
||||
An open Merge Request keeps one immutable `selector_from` and `selector_to`. Advance only the existing source selector with a normal non-force push; do not open a replacement Merge Request, invent an add-revision operation, or create a fresh integration branch for each fix or target movement. Before opening or requesting review, publish the exact source and verify that the provider resolves `selector_from` to local `HEAD`.
|
||||
|
||||
A review verdict is valid only for the exact provider-resolved source ref captured by `ReviewRequested`. Moving the source ref requires a fresh review of the new exact source. Moving only the target ref does not invalidate approval for an unchanged source; it requires refreshed readiness/integration evidence against the current target. Target integration and `CompleteMergeRequest` are Orchestrator authority, not Coder or Reviewer authority.
|
||||
|
||||
Current Ticket, Merge Request, provider refs, and thread evidence take precedence over stale Memory, old implementation reports, branch-name assumptions, or previous instructions that describe a revision-based workflow. Reread the Ticket and `ShowMergeRequest` before decisions. If source or target movement races with review or completion, stop and reread current authority rather than reusing stale evidence.
|
||||
|
||||
@@ -1,13 +1,9 @@
|
||||
You are the assigned Coder. Implement the requested scope in the provided Workdir and keep durable evidence on the Ticket and its Merge Request.
|
||||
|
||||
Use the Merge Request as the routine authority for review requests, verdicts, fixes, and rereview cycles. Do not add a Ticket comment for each review or fix iteration. Add a Ticket comment only when a blocker or decision requires Orchestrator attention, or once after approval to hand off the final implementation and validation evidence.
|
||||
Use the existing Merge Request as the routine authority for review requests, verdicts, fixes, and rereview cycles. `OpenMergeRequest` creates the one selector-based Merge Request; if one is already open, use `ShowMergeRequest`, keep its original selectors, and advance only that same source ref with a normal non-force push. Never invent an add-revision operation, replacement Merge Request, or fresh integration branch. Source movement requires fresh review of the exact new ref. Target-only movement does not invalidate source approval and is handled later by Orchestrator integration authority.
|
||||
|
||||
Do not add a Ticket comment for each review or fix iteration. Add a Ticket comment only when a blocker or decision requires Orchestrator attention, or once after approval to hand off the final implementation and validation evidence.
|
||||
|
||||
Treat the first committed user message as the bounded Ticket/action context and do not infer control-plane identity from prose.
|
||||
|
||||
Before opening a Merge Request, publish only the committed Ticket work branch with a normal non-force push and verify that the Ticket repository remote resolves it to the exact local `HEAD`; a local branch name or dirty Workdir is not immutable review evidence. Do not push the target branch, tags, or unrelated refs, and never force-push.
|
||||
|
||||
{% include "common.git" %}
|
||||
|
||||
Before review, open a Merge Request with immutable `selector_from` / `selector_to`. Spawn the Reviewer only as your actual direct-child `builtin:reviewer` SubWorker, delegate write scope so it can use the Workdir command tools required for inspection and validation, and pass only the Ticket id in the structured review handoff. The host resolves `selector_from`, captures the immutable `subject_ref`, appends `ReviewRequested`, and injects the review capability; commit/ref identity is not model input. Reviewer prose is not approval: the child must commit `MergeRequestReview` through its injected capability authority.
|
||||
|
||||
A request-changes result requires a freshly published immutable subject and a fresh Reviewer child request. Flow terminal state is not Ticket completion authority. After the exact current Merge Request subject has authoritative approval, keep that source ref immutable, leave concise implementation evidence on the Ticket when useful, and hand off integration to the Orchestrator. Do not update the target selector. Do not call `MergeRequestComplete`.
|
||||
|
||||
@@ -6,15 +6,15 @@ Keep durable orchestration behavior here and treat the first committed user mess
|
||||
|
||||
The assigned Coder owns its review/fix loop and launches Reviewer SubWorkers itself. Do not spawn, restore, assign, or route work to Backend/Runtime Reviewer Workers, and do not select a Reviewer profile through the generic WorkerSpawn path. If durable `Review` evidence for the current provider-resolved `selector_from` subject is missing, indeterminate, revoked, cancelled, or requests changes, keep the Ticket in progress and return the requirement to the same assigned Coder; never compensate by creating an independent Reviewer Worker.
|
||||
|
||||
Treat the current linked Merge Request as implementation-completion authority. A current provider-resolved source ref, commit/repository evidence, an effective approval for that exact subject, review freshness after the latest substantive Ticket item edit, and no unresolved request-changes are sufficient; do not require an `implementation_report`. Human summaries remain optional audit context. Recheck `ShowTicket` and `MergeRequestReadinessCheck` immediately before guarded integration. Require the Merge Request source selector to remain on the exact reviewed commit; any source movement requires a fresh Reviewer attempt.
|
||||
Treat the current linked Merge Request as implementation-completion authority. A current provider-resolved source ref, commit/repository evidence, an effective approval for that exact subject, review freshness after the latest substantive Ticket item edit, and no unresolved request-changes are sufficient; do not require an `implementation_report`. Human summaries remain optional audit context. Recheck `ShowTicket`, `ShowMergeRequest`, and `CheckMergeRequestReadiness` immediately before guarded integration. Require the Merge Request source selector to remain on the exact reviewed commit: source movement requires a fresh Reviewer attempt. Target-only movement preserves source approval and requires refreshed integration evidence against the current target; it does not require a replacement Merge Request, an add-revision operation, or a fresh integration branch.
|
||||
|
||||
Before integration, run `MergeRequestReadinessCheck` and reread the Ticket, current assignment, and exact approved subject. Treat the provider-resolved `selector_from` as the `merge_from` branch and `selector_to` as the `merge_to` branch. Obtain their exact approved source hash and `target_ref_before`, selected merge strategy, and approval event from authoritative Merge Request evidence.
|
||||
Before integration, run `CheckMergeRequestReadiness` and reread the Ticket, current assignment, and exact approved subject. Treat the provider-resolved `selector_from` as the `merge_from` branch and `selector_to` as the `merge_to` branch. Obtain their exact approved source hash and `target_ref_before`, selected merge strategy, and approval event from authoritative Merge Request evidence.
|
||||
|
||||
Perform integration through normal source-control operations in the bound Orchestrator Workdir. Treat its current checkout, branch attachment, and tracking state as execution state to inspect and adjust, not by themselves as evidence of a missing integration capability. Ensure the Workdir is clean, resolve the required branches through the configured repository when necessary, verify `merge_from` points exactly to the approved source hash and `merge_to` points exactly to `target_ref_before`, switch to `merge_to`, merge `merge_from` with the approved strategy, and validate the resulting revision and tree.
|
||||
|
||||
Push the resulting `merge_to` branch through its configured normal push path. Preserve repository consistency guards: never rewrite history, bypass branch or Worktree safety, update an unrelated ref, or integrate a source revision different from the reviewed subject. Treat an integration blocker as authoritative only when supported by a concrete source-control or provider failure; before proposing a new control-plane capability, verify that the required operation cannot be expressed through the existing bound Workdir and repository provider.
|
||||
|
||||
After the push, verify the repository provider resolves `merge_to` exactly to `target_ref_after`, then call `MergeRequestComplete` with the before/after evidence, authoritative approval event, and merge strategy. `MergeRequestComplete` records and verifies an already-applied repository integration; it does not update the branch itself.
|
||||
After the push, verify the repository provider resolves `merge_to` exactly to `target_ref_after`, then call `CompleteMergeRequest` with the before/after evidence, authoritative approval event, and merge strategy. `CompleteMergeRequest` records and verifies an already-applied repository integration; it does not update the branch itself.
|
||||
|
||||
If the repository push succeeds but completion recording fails, do not push again or invent a new result. Retry the same completion operation and evidence: while no completion event exists, the Server requires the target to remain at the exact `target_ref_after` before it records `MergeResult`, moves the Ticket to `done`, and closes the current assignment atomically. Once that exact operation is recorded, later target movement does not invalidate an idempotent replay of the recorded result. Before recording, any other observed target is a stale/conflicting completion and must fail closed.
|
||||
|
||||
|
||||
@@ -2,6 +2,6 @@ You are the Ticket Reviewer role running as an actual Runtime-owned direct child
|
||||
|
||||
Keep role behavior here and treat the first committed user message as bounded Ticket/Merge Request context only, never as a supplied verdict. Review the host-captured `ReviewRequested.subject_ref` against Ticket intent, binding decisions/invariants, acceptance criteria, and project design boundaries. Use the available Workdir inspection and command tools for focused validation, but do not intentionally modify implementation files, merge, close, update a repository ref, or take over implementation.
|
||||
|
||||
Your prose response is not review authority. Before finishing, call `MergeRequestReview` exactly once with `approve` or `request_changes`, a bounded evidence summary, and concrete structured findings. Capability authority and subject identity are injected by your child Workspace client and are not model inputs. The Server re-resolves `selector_from`; if it moved, submission records cancellation and fails rather than approving stale work.
|
||||
Your prose response is not review authority. Before finishing, call `ReviewMergeRequest` exactly once with `approve` or `request_changes`, a bounded evidence summary, and concrete structured findings. Capability authority and subject identity are injected by your child Workspace client and are not model inputs. The Server re-resolves `selector_from`; if it moved, submission records cancellation and fails rather than approving stale work. A verdict applies only to that captured source ref; target-only movement does not invalidate approval for an unchanged source, and target integration remains Orchestrator authority.
|
||||
|
||||
Review more than the diff: verify the implementation satisfies the Ticket intent and acceptance criteria, remains coherent with the codebase design, and does not introduce unnecessary compatibility.
|
||||
|
||||
@@ -6,7 +6,7 @@
|
||||
"dev": "deno run -A npm:vite@7.2.7 dev",
|
||||
"dev:backend": "cd ../.. && cargo run -p yoi-workspace-server --bin yoi-server -- serve --listen 127.0.0.1:8787",
|
||||
"check": "deno run -A npm:@sveltejs/kit@2.49.4 sync && deno run -A npm:svelte-check@4.3.4 --tsconfig ./tsconfig.json",
|
||||
"test": "deno test --allow-read=src,test --allow-env=LOG,VSCODE_TEXTMATE_DEBUG src/lib/workspace/auth/model.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-completion.test.ts src/lib/workspace/console/markdown.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/console/tasks.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-launch.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts",
|
||||
"test": "deno test --allow-read=src,test --allow-env=LOG,VSCODE_TEXTMATE_DEBUG src/lib/workspace/auth/model.test.ts src/lib/workspace/api/http.test.ts src/lib/workspace/header/breadcrumb-model.test.ts src/lib/workspace/console/chat-submit.test.ts src/lib/workspace/console/composer-command.test.ts src/lib/workspace/console/composer-completion.test.ts src/lib/workspace/console/markdown.test.ts test/console/ansi.test.ts src/lib/workspace/console/model.test.ts src/lib/workspace/console/tasks.test.ts src/lib/workspace/console/worker-console.ui.test.ts src/lib/workspace/settings/model.test.ts src/lib/workspace/sidebar/workers.test.ts src/lib/workspace/sidebar/workspace-switcher.test.ts src/lib/workspace/sidebar/worker-subscription.test.ts src/lib/workspace/sidebar/worker-launch.test.ts src/lib/workspace/tickets/merge-request-resources.test.ts src/lib/workspace/tickets/ticket-panel.test.ts test/merge-request-status.test.ts test/config-source/decodal-grammar.test.ts test/config-source/editor-state.test.ts test/config-source/fixed-schema-wrapper.test.ts test/config-source/toolchain.test.ts test/config-source/wasm-parity.test.ts",
|
||||
"build": "deno run -A npm:vite@7.2.7 build",
|
||||
"preview": "deno run -A npm:vite@7.2.7 preview"
|
||||
},
|
||||
|
||||
@@ -0,0 +1,76 @@
|
||||
import type {
|
||||
MergeRequestDetail,
|
||||
MergeRequestThreadEvent,
|
||||
} from "./api/merge-requests.ts";
|
||||
|
||||
function isCurrentSourceEvent(
|
||||
event: MergeRequestThreadEvent,
|
||||
source: string,
|
||||
): boolean {
|
||||
return event.subject_ref === source;
|
||||
}
|
||||
|
||||
function requestHasTerminalOutcome(
|
||||
thread: MergeRequestThreadEvent[],
|
||||
requestEventId: unknown,
|
||||
): boolean {
|
||||
return thread.some((event) =>
|
||||
(event.kind === "review" || event.kind === "review_cancelled") &&
|
||||
event.request_event_id === requestEventId
|
||||
);
|
||||
}
|
||||
|
||||
export function sourceReviewFreshness(
|
||||
mergeRequest: MergeRequestDetail,
|
||||
): string {
|
||||
const source = mergeRequest.source.ref;
|
||||
if (!source) return "Source review unavailable: selector_from is unresolved.";
|
||||
|
||||
const effectiveReview = [...mergeRequest.thread].reverse().find((event) => {
|
||||
if (event.kind !== "review" || !isCurrentSourceEvent(event, source)) {
|
||||
return false;
|
||||
}
|
||||
return !mergeRequest.thread.some(
|
||||
(candidate) =>
|
||||
candidate.kind === "review_revoked" &&
|
||||
candidate.review_event_id === event.event_id,
|
||||
);
|
||||
});
|
||||
if (effectiveReview) {
|
||||
return effectiveReview.decision === "approve"
|
||||
? `Current source approved at exact ref ${source}.`
|
||||
: `Current source requests changes at exact ref ${source}.`;
|
||||
}
|
||||
|
||||
const latestEvidence = [...mergeRequest.thread].reverse().find(
|
||||
(event) =>
|
||||
(event.kind === "review" || event.kind === "review_requested") &&
|
||||
typeof event.subject_ref === "string",
|
||||
);
|
||||
if (latestEvidence?.subject_ref && latestEvidence.subject_ref !== source) {
|
||||
return `Fresh source review required: selector_from moved from ${latestEvidence.subject_ref} to ${source}.`;
|
||||
}
|
||||
|
||||
const pendingRequest = [...mergeRequest.thread].reverse().find((event) =>
|
||||
event.kind === "review_requested" &&
|
||||
isCurrentSourceEvent(event, source) &&
|
||||
!requestHasTerminalOutcome(mergeRequest.thread, event.event_id)
|
||||
);
|
||||
if (pendingRequest) {
|
||||
return `Current source review pending for exact ref ${source}.`;
|
||||
}
|
||||
|
||||
return `Fresh source review required: no effective verdict exists for ${source}.`;
|
||||
}
|
||||
|
||||
export function targetIntegrationStatus(
|
||||
mergeRequest: MergeRequestDetail,
|
||||
): string {
|
||||
if (mergeRequest.state === "merged") {
|
||||
return "Target integration recorded by CompleteMergeRequest.";
|
||||
}
|
||||
if (!mergeRequest.target.ref) {
|
||||
return "Target integration unavailable: selector_to is unresolved.";
|
||||
}
|
||||
return `Target integration awaits Orchestrator action at ${mergeRequest.target.ref}. Target-only movement refreshes integration evidence; it does not invalidate approval for an unchanged source.`;
|
||||
}
|
||||
@@ -31,6 +31,15 @@ const detailLoader = await Deno.readTextFile(
|
||||
import.meta.url,
|
||||
),
|
||||
);
|
||||
const detailPage = await Deno.readTextFile(
|
||||
new URL(
|
||||
"../../../routes/w/[workspaceId]/merge-requests/[mergeRequestId]/+page.svelte",
|
||||
import.meta.url,
|
||||
),
|
||||
);
|
||||
const statusProjection = await Deno.readTextFile(
|
||||
new URL("../merge-request-status.ts", import.meta.url),
|
||||
);
|
||||
const sidebar = await Deno.readTextFile(
|
||||
new URL("../sidebar/WorkspaceSidebar.svelte", import.meta.url),
|
||||
);
|
||||
@@ -64,3 +73,34 @@ Deno.test("Workspace exposes Merge Request collection and detail pages", () => {
|
||||
"MR resources are absent from navigation",
|
||||
);
|
||||
});
|
||||
|
||||
Deno.test("Merge Request UI separates source review freshness from target integration", () => {
|
||||
assert(
|
||||
detailPage.includes("sourceReviewFreshness") &&
|
||||
detailPage.includes("targetIntegrationStatus"),
|
||||
"MR detail page does not render authority status projections",
|
||||
);
|
||||
for (const source of [`${detailPage}\n${statusProjection}`, ticketPage]) {
|
||||
assert(
|
||||
source.includes("Fresh source review required"),
|
||||
"missing source-review freshness diagnostic",
|
||||
);
|
||||
assert(
|
||||
source.includes("Target integration"),
|
||||
"missing target-integration status",
|
||||
);
|
||||
assert(
|
||||
source.includes("Target-only movement") &&
|
||||
source.includes("does not invalidate approval for an unchanged source"),
|
||||
"source review and target integration semantics are conflated",
|
||||
);
|
||||
}
|
||||
assert(
|
||||
statusProjection.includes("selector_from moved from"),
|
||||
"source ref mismatch is not explained",
|
||||
);
|
||||
assert(
|
||||
statusProjection.includes("CompleteMergeRequest"),
|
||||
"target integration authority is not named",
|
||||
);
|
||||
});
|
||||
|
||||
@@ -1,5 +1,9 @@
|
||||
<script lang="ts">
|
||||
import { mergeRequestPagePath } from "$lib/workspace/api/merge-requests";
|
||||
import {
|
||||
sourceReviewFreshness,
|
||||
targetIntegrationStatus,
|
||||
} from "$lib/workspace/merge-request-status";
|
||||
import type { PageData } from "./$types";
|
||||
|
||||
let { data }: { data: PageData } = $props();
|
||||
@@ -13,6 +17,7 @@
|
||||
const value = event[key];
|
||||
return typeof value === "string" && value.length > 0 ? value : null;
|
||||
}
|
||||
|
||||
</script>
|
||||
|
||||
<svelte:head><title>Merge Request · Yoi</title></svelte:head>
|
||||
@@ -53,6 +58,14 @@
|
||||
</dl>
|
||||
</section>
|
||||
|
||||
<section class="ticket-detail-section">
|
||||
<div class="ticket-section-heading"><h2>Review and integration status</h2></div>
|
||||
<dl class="ticket-facts">
|
||||
<div><dt>Source review freshness</dt><dd>{sourceReviewFreshness(mergeRequest)}</dd></div>
|
||||
<div><dt>Target integration</dt><dd>{targetIntegrationStatus(mergeRequest)}</dd></div>
|
||||
</dl>
|
||||
</section>
|
||||
|
||||
<section class="ticket-detail-section">
|
||||
<div class="ticket-section-heading">
|
||||
<h2>Thread</h2><span>{mergeRequest.thread.length}</span>
|
||||
|
||||
@@ -556,14 +556,22 @@
|
||||
<section class="ticket-control-card">
|
||||
<header><h2>Merge Request</h2></header>
|
||||
{#if mergeRequest}
|
||||
<p><strong>{mergeRequest.state}</strong> · review {mergeRequest.review_status}</p>
|
||||
<p><strong>{mergeRequest.state}</strong></p>
|
||||
<p>
|
||||
From <code>{mergeRequest.selector_from ?? "requires repair"}</code>
|
||||
to <code>{mergeRequest.selector_to}</code>
|
||||
</p>
|
||||
{#if mergeRequest.current_subject_ref}
|
||||
<p>Current source <code>{mergeRequest.current_subject_ref}</code></p>
|
||||
{#if mergeRequest.current_subject_ref && mergeRequest.review_subject_ref === mergeRequest.current_subject_ref}
|
||||
<p><strong>Source review:</strong> {mergeRequest.review_status} for exact ref <code>{mergeRequest.current_subject_ref}</code></p>
|
||||
{:else if mergeRequest.current_subject_ref && mergeRequest.review_subject_ref}
|
||||
<p><strong>Fresh source review required:</strong> selector_from moved from <code>{mergeRequest.review_subject_ref}</code> to <code>{mergeRequest.current_subject_ref}</code>.</p>
|
||||
{:else if mergeRequest.current_subject_ref}
|
||||
<p><strong>Fresh source review required:</strong> no effective verdict exists for <code>{mergeRequest.current_subject_ref}</code>.</p>
|
||||
{:else}
|
||||
<p><strong>Source review unavailable:</strong> selector_from is unresolved.</p>
|
||||
{/if}
|
||||
<p><strong>Target integration:</strong> {mergeRequest.state === "merged" ? "recorded" : `awaiting Orchestrator integration into ${mergeRequest.selector_to}`}.</p>
|
||||
<p class="workspace-empty-copy">Target-only movement refreshes integration evidence; it does not invalidate approval for an unchanged source.</p>
|
||||
<a
|
||||
class="workspace-secondary-button"
|
||||
href={mergeRequestPagePath(data.workspaceId, mergeRequest.merge_request_id)}
|
||||
|
||||
@@ -0,0 +1,142 @@
|
||||
/// <reference lib="deno.ns" />
|
||||
|
||||
import type {
|
||||
MergeRequestDetail,
|
||||
MergeRequestThreadEvent,
|
||||
} from "../src/lib/workspace/api/merge-requests.ts";
|
||||
import { sourceReviewFreshness } from "../src/lib/workspace/merge-request-status.ts";
|
||||
|
||||
function assertEquals(actual: unknown, expected: unknown): void {
|
||||
if (actual !== expected) {
|
||||
throw new Error(
|
||||
`expected ${JSON.stringify(expected)}, got ${JSON.stringify(actual)}`,
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
function event(
|
||||
kind: string,
|
||||
fields: Record<string, unknown>,
|
||||
): MergeRequestThreadEvent {
|
||||
return { kind, sequence: 1, at: "2026-09-01T00:00:00Z", ...fields };
|
||||
}
|
||||
|
||||
function detail(thread: MergeRequestThreadEvent[]): MergeRequestDetail {
|
||||
return {
|
||||
merge_request_id: "MR-1",
|
||||
workspace_id: "W",
|
||||
repository_id: "main",
|
||||
ticket_ids: ["T-1"],
|
||||
selector_from: "work/ticket",
|
||||
selector_to: "develop",
|
||||
state: "open",
|
||||
opened_by: {
|
||||
runtime_id: "runtime",
|
||||
worker_id: "worker",
|
||||
assignment_id: "assignment",
|
||||
},
|
||||
created_at: "2026-09-01T00:00:00Z",
|
||||
updated_at: "2026-09-01T00:00:00Z",
|
||||
source: {
|
||||
status: "known",
|
||||
ref: "source-2",
|
||||
observed_at: "2026-09-01T00:00:00Z",
|
||||
},
|
||||
target: {
|
||||
status: "known",
|
||||
ref: "target-2",
|
||||
observed_at: "2026-09-01T00:00:00Z",
|
||||
},
|
||||
linked_tickets: [{ ticket_id: "T-1", key: "T-1" }],
|
||||
thread,
|
||||
};
|
||||
}
|
||||
|
||||
Deno.test("revoked review requires a fresh review instead of appearing pending", () => {
|
||||
const mergeRequest = detail([
|
||||
event("review_requested", {
|
||||
event_id: "request-1",
|
||||
subject_ref: "source-2",
|
||||
}),
|
||||
event("review", {
|
||||
event_id: "review-1",
|
||||
request_event_id: "request-1",
|
||||
subject_ref: "source-2",
|
||||
decision: "approve",
|
||||
}),
|
||||
event("review_revoked", {
|
||||
event_id: "revoke-1",
|
||||
review_event_id: "review-1",
|
||||
}),
|
||||
]);
|
||||
|
||||
assertEquals(
|
||||
sourceReviewFreshness(mergeRequest),
|
||||
"Fresh source review required: no effective verdict exists for source-2.",
|
||||
);
|
||||
});
|
||||
|
||||
Deno.test("unresolved review request for the current source is pending", () => {
|
||||
const mergeRequest = detail([
|
||||
event("review_requested", {
|
||||
event_id: "request-2",
|
||||
subject_ref: "source-2",
|
||||
}),
|
||||
]);
|
||||
|
||||
assertEquals(
|
||||
sourceReviewFreshness(mergeRequest),
|
||||
"Current source review pending for exact ref source-2.",
|
||||
);
|
||||
});
|
||||
|
||||
Deno.test("completed or cancelled request is not projected as pending", () => {
|
||||
const approved = detail([
|
||||
event("review_requested", {
|
||||
event_id: "request-3",
|
||||
subject_ref: "source-2",
|
||||
}),
|
||||
event("review", {
|
||||
event_id: "review-3",
|
||||
request_event_id: "request-3",
|
||||
subject_ref: "source-2",
|
||||
decision: "approve",
|
||||
}),
|
||||
]);
|
||||
assertEquals(
|
||||
sourceReviewFreshness(approved),
|
||||
"Current source approved at exact ref source-2.",
|
||||
);
|
||||
|
||||
const cancelled = detail([
|
||||
event("review_requested", {
|
||||
event_id: "request-4",
|
||||
subject_ref: "source-2",
|
||||
}),
|
||||
event("review_cancelled", {
|
||||
event_id: "cancel-4",
|
||||
request_event_id: "request-4",
|
||||
subject_ref: "source-2",
|
||||
}),
|
||||
]);
|
||||
assertEquals(
|
||||
sourceReviewFreshness(cancelled),
|
||||
"Fresh source review required: no effective verdict exists for source-2.",
|
||||
);
|
||||
});
|
||||
|
||||
Deno.test("source movement explains the exact stale and current refs", () => {
|
||||
const mergeRequest = detail([
|
||||
event("review", {
|
||||
event_id: "review-old",
|
||||
request_event_id: "request-old",
|
||||
subject_ref: "source-1",
|
||||
decision: "approve",
|
||||
}),
|
||||
]);
|
||||
|
||||
assertEquals(
|
||||
sourceReviewFreshness(mergeRequest),
|
||||
"Fresh source review required: selector_from moved from source-1 to source-2.",
|
||||
);
|
||||
});
|
||||
Reference in New Issue
Block a user