mirror of
https://github.com/instructkr/claw-code.git
synced 2026-04-07 00:24:50 +08:00
Close the clawability backlog with deterministic CLI output and lane lineage
Finish the remaining roadmap work by making direct CLI JSON output deterministic across the non-interactive surface, restoring the degraded-startup MCP test as a real workspace test, and adding branch-lock plus commit-lineage primitives so downstream lane consumers can distinguish superseded worktree commits from canonical lineage. Constraint: Keep the user-facing config namespace centered on .claw while preserving legacy fallback discovery for compatibility Constraint: Verification needed to stay clean-room and reproducible from the checked-in workspace alone Rejected: Leave the output-format contract implied by ad-hoc smoke runs only | too easy for direct CLI regressions to slip back into prose-only output Rejected: Keep commit provenance as free-form detail text | downstream consumers need structured branch/worktree/supersession metadata Confidence: medium Scope-risk: moderate Directive: Extend the JSON contract through the same direct CLI entrypoints instead of adding one-off serializers on parallel code paths Tested: python .github/scripts/check_doc_source_of_truth.py Tested: cd rust && cargo fmt --all --check Tested: cd rust && cargo test --workspace Tested: cd rust && cargo clippy -p commands -p tools -p rusty-claude-cli --all-targets --no-deps -- -D warnings Not-tested: full cargo clippy --workspace --all-targets -- -D warnings still reports unrelated pre-existing runtime lint debt outside this change set
This commit is contained in:
144
rust/crates/runtime/src/branch_lock.rs
Normal file
144
rust/crates/runtime/src/branch_lock.rs
Normal file
@@ -0,0 +1,144 @@
|
||||
use serde::{Deserialize, Serialize};
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct BranchLockIntent {
|
||||
#[serde(rename = "laneId")]
|
||||
pub lane_id: String,
|
||||
pub branch: String,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub worktree: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub modules: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct BranchLockCollision {
|
||||
pub branch: String,
|
||||
pub module: String,
|
||||
#[serde(rename = "laneIds")]
|
||||
pub lane_ids: Vec<String>,
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn detect_branch_lock_collisions(intents: &[BranchLockIntent]) -> Vec<BranchLockCollision> {
|
||||
let mut collisions = Vec::new();
|
||||
|
||||
for (index, left) in intents.iter().enumerate() {
|
||||
for right in &intents[index + 1..] {
|
||||
if left.branch != right.branch {
|
||||
continue;
|
||||
}
|
||||
for module in overlapping_modules(&left.modules, &right.modules) {
|
||||
collisions.push(BranchLockCollision {
|
||||
branch: left.branch.clone(),
|
||||
module,
|
||||
lane_ids: vec![left.lane_id.clone(), right.lane_id.clone()],
|
||||
});
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
collisions.sort_by(|a, b| {
|
||||
a.branch
|
||||
.cmp(&b.branch)
|
||||
.then(a.module.cmp(&b.module))
|
||||
.then(a.lane_ids.cmp(&b.lane_ids))
|
||||
});
|
||||
collisions.dedup();
|
||||
collisions
|
||||
}
|
||||
|
||||
fn overlapping_modules(left: &[String], right: &[String]) -> Vec<String> {
|
||||
let mut overlaps = Vec::new();
|
||||
for left_module in left {
|
||||
for right_module in right {
|
||||
if modules_overlap(left_module, right_module) {
|
||||
overlaps.push(shared_scope(left_module, right_module));
|
||||
}
|
||||
}
|
||||
}
|
||||
overlaps.sort();
|
||||
overlaps.dedup();
|
||||
overlaps
|
||||
}
|
||||
|
||||
fn modules_overlap(left: &str, right: &str) -> bool {
|
||||
left == right
|
||||
|| left.starts_with(&format!("{right}/"))
|
||||
|| right.starts_with(&format!("{left}/"))
|
||||
}
|
||||
|
||||
fn shared_scope(left: &str, right: &str) -> String {
|
||||
if left.starts_with(&format!("{right}/")) || left == right {
|
||||
right.to_string()
|
||||
} else {
|
||||
left.to_string()
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::{detect_branch_lock_collisions, BranchLockIntent};
|
||||
|
||||
#[test]
|
||||
fn detects_same_branch_same_module_collisions() {
|
||||
let collisions = detect_branch_lock_collisions(&[
|
||||
BranchLockIntent {
|
||||
lane_id: "lane-a".to_string(),
|
||||
branch: "feature/lock".to_string(),
|
||||
worktree: Some("wt-a".to_string()),
|
||||
modules: vec!["runtime/mcp".to_string()],
|
||||
},
|
||||
BranchLockIntent {
|
||||
lane_id: "lane-b".to_string(),
|
||||
branch: "feature/lock".to_string(),
|
||||
worktree: Some("wt-b".to_string()),
|
||||
modules: vec!["runtime/mcp".to_string()],
|
||||
},
|
||||
]);
|
||||
|
||||
assert_eq!(collisions.len(), 1);
|
||||
assert_eq!(collisions[0].branch, "feature/lock");
|
||||
assert_eq!(collisions[0].module, "runtime/mcp");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn detects_nested_module_scope_collisions() {
|
||||
let collisions = detect_branch_lock_collisions(&[
|
||||
BranchLockIntent {
|
||||
lane_id: "lane-a".to_string(),
|
||||
branch: "feature/lock".to_string(),
|
||||
worktree: None,
|
||||
modules: vec!["runtime".to_string()],
|
||||
},
|
||||
BranchLockIntent {
|
||||
lane_id: "lane-b".to_string(),
|
||||
branch: "feature/lock".to_string(),
|
||||
worktree: None,
|
||||
modules: vec!["runtime/mcp".to_string()],
|
||||
},
|
||||
]);
|
||||
|
||||
assert_eq!(collisions[0].module, "runtime");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ignores_different_branches() {
|
||||
let collisions = detect_branch_lock_collisions(&[
|
||||
BranchLockIntent {
|
||||
lane_id: "lane-a".to_string(),
|
||||
branch: "feature/a".to_string(),
|
||||
worktree: None,
|
||||
modules: vec!["runtime/mcp".to_string()],
|
||||
},
|
||||
BranchLockIntent {
|
||||
lane_id: "lane-b".to_string(),
|
||||
branch: "feature/b".to_string(),
|
||||
worktree: None,
|
||||
modules: vec!["runtime/mcp".to_string()],
|
||||
},
|
||||
]);
|
||||
|
||||
assert!(collisions.is_empty());
|
||||
}
|
||||
}
|
||||
@@ -76,6 +76,20 @@ pub struct LaneEventBlocker {
|
||||
pub detail: String,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct LaneCommitProvenance {
|
||||
pub commit: String,
|
||||
pub branch: String,
|
||||
#[serde(skip_serializing_if = "Option::is_none")]
|
||||
pub worktree: Option<String>,
|
||||
#[serde(rename = "canonicalCommit", skip_serializing_if = "Option::is_none")]
|
||||
pub canonical_commit: Option<String>,
|
||||
#[serde(rename = "supersededBy", skip_serializing_if = "Option::is_none")]
|
||||
pub superseded_by: Option<String>,
|
||||
#[serde(default, skip_serializing_if = "Vec::is_empty")]
|
||||
pub lineage: Vec<String>,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)]
|
||||
pub struct LaneEvent {
|
||||
pub event: LaneEventName,
|
||||
@@ -122,6 +136,36 @@ impl LaneEvent {
|
||||
.with_optional_detail(detail)
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn commit_created(
|
||||
emitted_at: impl Into<String>,
|
||||
detail: Option<String>,
|
||||
provenance: LaneCommitProvenance,
|
||||
) -> Self {
|
||||
Self::new(
|
||||
LaneEventName::CommitCreated,
|
||||
LaneEventStatus::Completed,
|
||||
emitted_at,
|
||||
)
|
||||
.with_optional_detail(detail)
|
||||
.with_data(serde_json::to_value(provenance).expect("commit provenance should serialize"))
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn superseded(
|
||||
emitted_at: impl Into<String>,
|
||||
detail: Option<String>,
|
||||
provenance: LaneCommitProvenance,
|
||||
) -> Self {
|
||||
Self::new(
|
||||
LaneEventName::Superseded,
|
||||
LaneEventStatus::Superseded,
|
||||
emitted_at,
|
||||
)
|
||||
.with_optional_detail(detail)
|
||||
.with_data(serde_json::to_value(provenance).expect("commit provenance should serialize"))
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn blocked(emitted_at: impl Into<String>, blocker: &LaneEventBlocker) -> Self {
|
||||
Self::new(LaneEventName::Blocked, LaneEventStatus::Blocked, emitted_at)
|
||||
@@ -161,11 +205,54 @@ impl LaneEvent {
|
||||
}
|
||||
}
|
||||
|
||||
#[must_use]
|
||||
pub fn dedupe_superseded_commit_events(events: &[LaneEvent]) -> Vec<LaneEvent> {
|
||||
let mut keep = vec![true; events.len()];
|
||||
let mut latest_by_key = std::collections::BTreeMap::<String, usize>::new();
|
||||
|
||||
for (index, event) in events.iter().enumerate() {
|
||||
if event.event != LaneEventName::CommitCreated {
|
||||
continue;
|
||||
}
|
||||
let Some(data) = event.data.as_ref() else {
|
||||
continue;
|
||||
};
|
||||
let key = data
|
||||
.get("canonicalCommit")
|
||||
.or_else(|| data.get("commit"))
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.map(str::to_string);
|
||||
let superseded = data
|
||||
.get("supersededBy")
|
||||
.and_then(serde_json::Value::as_str)
|
||||
.is_some();
|
||||
if superseded {
|
||||
keep[index] = false;
|
||||
continue;
|
||||
}
|
||||
if let Some(key) = key {
|
||||
if let Some(previous) = latest_by_key.insert(key, index) {
|
||||
keep[previous] = false;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
events
|
||||
.iter()
|
||||
.cloned()
|
||||
.zip(keep)
|
||||
.filter_map(|(event, retain)| retain.then_some(event))
|
||||
.collect()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use serde_json::json;
|
||||
|
||||
use super::{LaneEvent, LaneEventBlocker, LaneEventName, LaneEventStatus, LaneFailureClass};
|
||||
use super::{
|
||||
dedupe_superseded_commit_events, LaneCommitProvenance, LaneEvent, LaneEventBlocker,
|
||||
LaneEventName, LaneEventStatus, LaneFailureClass,
|
||||
};
|
||||
|
||||
#[test]
|
||||
fn canonical_lane_event_names_serialize_to_expected_wire_values() {
|
||||
@@ -240,4 +327,56 @@ mod tests {
|
||||
assert_eq!(failed.status, LaneEventStatus::Failed);
|
||||
assert_eq!(failed.detail.as_deref(), Some("broken server"));
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn commit_events_can_carry_worktree_and_supersession_metadata() {
|
||||
let event = LaneEvent::commit_created(
|
||||
"2026-04-04T00:00:00Z",
|
||||
Some("commit created".to_string()),
|
||||
LaneCommitProvenance {
|
||||
commit: "abc123".to_string(),
|
||||
branch: "feature/provenance".to_string(),
|
||||
worktree: Some("wt-a".to_string()),
|
||||
canonical_commit: Some("abc123".to_string()),
|
||||
superseded_by: None,
|
||||
lineage: vec!["abc123".to_string()],
|
||||
},
|
||||
);
|
||||
let event_json = serde_json::to_value(&event).expect("lane event should serialize");
|
||||
assert_eq!(event_json["event"], "lane.commit.created");
|
||||
assert_eq!(event_json["data"]["branch"], "feature/provenance");
|
||||
assert_eq!(event_json["data"]["worktree"], "wt-a");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn dedupes_superseded_commit_events_by_canonical_commit() {
|
||||
let retained = dedupe_superseded_commit_events(&[
|
||||
LaneEvent::commit_created(
|
||||
"2026-04-04T00:00:00Z",
|
||||
Some("old".to_string()),
|
||||
LaneCommitProvenance {
|
||||
commit: "old123".to_string(),
|
||||
branch: "feature/provenance".to_string(),
|
||||
worktree: Some("wt-a".to_string()),
|
||||
canonical_commit: Some("canon123".to_string()),
|
||||
superseded_by: Some("new123".to_string()),
|
||||
lineage: vec!["old123".to_string(), "new123".to_string()],
|
||||
},
|
||||
),
|
||||
LaneEvent::commit_created(
|
||||
"2026-04-04T00:00:01Z",
|
||||
Some("new".to_string()),
|
||||
LaneCommitProvenance {
|
||||
commit: "new123".to_string(),
|
||||
branch: "feature/provenance".to_string(),
|
||||
worktree: Some("wt-b".to_string()),
|
||||
canonical_commit: Some("canon123".to_string()),
|
||||
superseded_by: None,
|
||||
lineage: vec!["old123".to_string(), "new123".to_string()],
|
||||
},
|
||||
),
|
||||
]);
|
||||
assert_eq!(retained.len(), 1);
|
||||
assert_eq!(retained[0].detail.as_deref(), Some("new"));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
mod bash;
|
||||
pub mod bash_validation;
|
||||
mod bootstrap;
|
||||
pub mod branch_lock;
|
||||
mod compact;
|
||||
mod config;
|
||||
mod conversation;
|
||||
@@ -46,6 +47,7 @@ pub mod worker_boot;
|
||||
|
||||
pub use bash::{execute_bash, BashCommandInput, BashCommandOutput};
|
||||
pub use bootstrap::{BootstrapPhase, BootstrapPlan};
|
||||
pub use branch_lock::{detect_branch_lock_collisions, BranchLockCollision, BranchLockIntent};
|
||||
pub use compact::{
|
||||
compact_session, estimate_session_tokens, format_compact_summary,
|
||||
get_compact_continuation_message, should_compact, CompactionConfig, CompactionResult,
|
||||
@@ -72,7 +74,8 @@ pub use hooks::{
|
||||
HookAbortSignal, HookEvent, HookProgressEvent, HookProgressReporter, HookRunResult, HookRunner,
|
||||
};
|
||||
pub use lane_events::{
|
||||
LaneEvent, LaneEventBlocker, LaneEventName, LaneEventStatus, LaneFailureClass,
|
||||
dedupe_superseded_commit_events, LaneCommitProvenance, LaneEvent, LaneEventBlocker,
|
||||
LaneEventName, LaneEventStatus, LaneFailureClass,
|
||||
};
|
||||
pub use mcp::{
|
||||
mcp_server_signature, mcp_tool_name, mcp_tool_prefix, normalize_name_for_mcp,
|
||||
|
||||
@@ -2652,8 +2652,37 @@ mod tests {
|
||||
});
|
||||
}
|
||||
|
||||
fn write_initialize_disconnect_script() -> PathBuf {
|
||||
let root = temp_dir();
|
||||
fs::create_dir_all(&root).expect("temp dir");
|
||||
let script_path = root.join("initialize-disconnect.py");
|
||||
let script = [
|
||||
"#!/usr/bin/env python3",
|
||||
"import sys",
|
||||
"header = b''",
|
||||
r"while not header.endswith(b'\r\n\r\n'):",
|
||||
" chunk = sys.stdin.buffer.read(1)",
|
||||
" if not chunk:",
|
||||
" raise SystemExit(1)",
|
||||
" header += chunk",
|
||||
"length = 0",
|
||||
r"for line in header.decode().split('\r\n'):",
|
||||
r" if line.lower().startswith('content-length:'):",
|
||||
r" length = int(line.split(':', 1)[1].strip())",
|
||||
"if length:",
|
||||
" sys.stdin.buffer.read(length)",
|
||||
"raise SystemExit(0)",
|
||||
"",
|
||||
]
|
||||
.join("\n");
|
||||
fs::write(&script_path, script).expect("write script");
|
||||
let mut permissions = fs::metadata(&script_path).expect("metadata").permissions();
|
||||
permissions.set_mode(0o755);
|
||||
fs::set_permissions(&script_path, permissions).expect("chmod");
|
||||
script_path
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[ignore = "flaky: intermittent timing issues in CI, see ROADMAP P2.15"]
|
||||
fn manager_discovery_report_keeps_healthy_servers_when_one_server_fails() {
|
||||
let runtime = Builder::new_current_thread()
|
||||
.enable_all()
|
||||
@@ -2663,6 +2692,7 @@ mod tests {
|
||||
let script_path = write_manager_mcp_server_script();
|
||||
let root = script_path.parent().expect("script parent");
|
||||
let alpha_log = root.join("alpha.log");
|
||||
let broken_script_path = write_initialize_disconnect_script();
|
||||
let servers = BTreeMap::from([
|
||||
(
|
||||
"alpha".to_string(),
|
||||
@@ -2673,8 +2703,8 @@ mod tests {
|
||||
ScopedMcpServerConfig {
|
||||
scope: ConfigSource::Local,
|
||||
config: McpServerConfig::Stdio(McpStdioServerConfig {
|
||||
command: "python3".to_string(),
|
||||
args: vec!["-c".to_string(), "import sys; sys.exit(0)".to_string()],
|
||||
command: broken_script_path.display().to_string(),
|
||||
args: Vec::new(),
|
||||
env: BTreeMap::new(),
|
||||
tool_call_timeout_ms: None,
|
||||
}),
|
||||
@@ -2737,6 +2767,7 @@ mod tests {
|
||||
|
||||
manager.shutdown().await.expect("shutdown");
|
||||
cleanup_script(&script_path);
|
||||
cleanup_script(&broken_script_path);
|
||||
});
|
||||
}
|
||||
|
||||
|
||||
Reference in New Issue
Block a user