Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 16 additions & 16 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ members = ["crates/*"]
resolver = "2"

[workspace.package]
version = "0.1.194"
version = "0.1.195"
edition = "2024"
rust-version = "1.88"
license = "Apache-2.0"
Expand Down
22 changes: 22 additions & 0 deletions crates/cli-sub-agent/src/pipeline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -385,6 +385,28 @@ pub(crate) fn run_pipeline_hook(
Ok(())
}

/// Fire an observational hook with auto-loaded config. Failures are logged and
/// silently discarded (observational events never block the caller).
pub(crate) fn fire_observational_hook(
event: HookEvent,
pairs: &[(&str, &str)],
project_root: &std::path::Path,
) {
let hooks_config = csa_hooks::load_hooks_config(
csa_session::get_session_root(project_root)
.ok()
.map(|r| r.join("hooks.toml"))
.as_deref(),
csa_hooks::global_hooks_path().as_deref(),
None,
);
let variables: std::collections::HashMap<String, String> = pairs
.iter()
.map(|(k, v)| (k.to_string(), v.to_string()))
.collect();
let _ = run_pipeline_hook(event, &hooks_config, &variables);
}

pub(crate) fn determine_project_root(cd: Option<&str>) -> Result<PathBuf> {
let path = if let Some(cd_path) = cd {
PathBuf::from(cd_path)
Expand Down
63 changes: 62 additions & 1 deletion crates/cli-sub-agent/src/review_cmd.rs
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,7 @@ pub(crate) async fn handle_review(args: ReviewArgs, current_depth: u32) -> Resul
} else {
result.execution.exit_code
};
let diff_fingerprint = compute_diff_fingerprint(&project_root, &scope);
persist_review_meta(
&project_root,
&ReviewSessionMeta {
Expand All @@ -357,10 +358,22 @@ pub(crate) async fn handle_review(args: ReviewArgs, current_depth: u32) -> Resul
fix_attempted: args.fix,
fix_rounds: 0,
timestamp: chrono::Utc::now(),
diff_fingerprint,
},
);

if !args.fix || verdict == CLEAN {
// Fire PostReview hook only for final results (no fix loop pending).
crate::pipeline::fire_observational_hook(
csa_hooks::HookEvent::PostReview,
&[
("session_id", result.meta_session_id.as_str()),
("decision", decision.as_str()),
("verdict", verdict),
("scope", &scope),
],
&project_root,
);
return Ok(effective_exit_code);
}

Expand All @@ -379,7 +392,8 @@ pub(crate) async fn handle_review(args: ReviewArgs, current_depth: u32) -> Resul
}

// --- Fix loop: resume the review session to apply fixes, then re-gate ---
return fix::run_fix_loop(fix::FixLoopContext {
let scope_for_hook = scope.clone();
let fix_exit_code = fix::run_fix_loop(fix::FixLoopContext {
tool,
config: config.as_ref(),
global_config: &global_config,
Expand All @@ -402,6 +416,21 @@ pub(crate) async fn handle_review(args: ReviewArgs, current_depth: u32) -> Resul
initial_session_id: result.meta_session_id.clone(),
})
.await;

// Fire PostReview hook after fix loop completes (final result).
let fix_passed = matches!(&fix_exit_code, Ok(0));
crate::pipeline::fire_observational_hook(
csa_hooks::HookEvent::PostReview,
&[
("session_id", result.meta_session_id.as_str()),
("decision", if fix_passed { "pass" } else { "fail" }),
("verdict", if fix_passed { CLEAN } else { verdict }),
("scope", &scope_for_hook),
],
&project_root,
);

return fix_exit_code;
}

if args.fix {
Expand Down Expand Up @@ -669,6 +698,38 @@ async fn execute_review(
Ok(execution)
}

/// Compute a SHA-256 content hash of the diff being reviewed.
///
/// The fingerprint enables diff-level deduplication: if two review
/// invocations produce the same diff content (e.g., revert-then-revert),
/// the second can reuse the first review's result.
fn compute_diff_fingerprint(project_root: &Path, scope: &str) -> Option<String> {
use sha2::{Digest, Sha256};

let diff_args: Vec<&str> = if scope == "uncommitted" {
vec!["diff", "HEAD"]
} else if let Some(range) = scope.strip_prefix("range:") {
vec!["diff", range]
} else if let Some(base) = scope.strip_prefix("base:") {
vec!["diff", base]
} else {
return None;
};
Comment on lines +709 to +717
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The construction of diff_args can be simplified to avoid repetition and improve conciseness.

Suggested change
let diff_args: Vec<&str> = if scope == "uncommitted" {
vec!["diff", "HEAD"]
} else if let Some(range) = scope.strip_prefix("range:") {
vec!["diff", range]
} else if let Some(base) = scope.strip_prefix("base:") {
vec!["diff", base]
} else {
return None;
};
let diff_param = if scope == "uncommitted" {
"HEAD"
} else if let Some(range) = scope.strip_prefix("range:") {
range
} else if let Some(base) = scope.strip_prefix("base:") {
base
} else {
return None;
};
let diff_args = ["diff", diff_param];


let output = std::process::Command::new("git")
.args(&diff_args)
.current_dir(project_root)
.output()
.ok()?;

if !output.status.success() || output.stdout.is_empty() {
return None;
}

let digest = Sha256::digest(&output.stdout);
Some(format!("sha256:{digest:x}"))
}

#[cfg(test)]
#[path = "review_cmd_tests.rs"]
mod tests;
2 changes: 2 additions & 0 deletions crates/cli-sub-agent/src/review_cmd_fix.rs
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,7 @@ pub(crate) async fn run_fix_loop(ctx: FixLoopContext<'_>) -> Result<i32> {
fix_attempted: true,
fix_rounds: u32::from(round),
timestamp: chrono::Utc::now(),
diff_fingerprint: None,
},
);
return Ok(0);
Expand All @@ -197,6 +198,7 @@ pub(crate) async fn run_fix_loop(ctx: FixLoopContext<'_>) -> Result<i32> {
fix_attempted: true,
fix_rounds: u32::from(ctx.max_rounds),
timestamp: chrono::Utc::now(),
diff_fingerprint: None,
},
);
error!(
Expand Down
70 changes: 70 additions & 0 deletions crates/csa-hooks/src/directive.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
//! CSA directive parsing from hook/step output.
//!
//! Directives are HTML-comment-style markers embedded in stdout:
//! `<!-- CSA:NEXT_STEP step_id=<id> -->` — instruct weave to jump to a step.

/// A parsed CSA directive from hook or step output.
#[derive(Debug, Clone, PartialEq, Eq)]
pub enum CsaDirective {
/// Jump to the named weave step.
NextStep { step_id: String },
}
Comment on lines +8 to +11
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The CsaDirective enum is defined but not used anywhere in the crate. This will result in a dead_code warning. If it's intended for future use, consider adding #[allow(dead_code)] to suppress the warning. Otherwise, it should be removed.


/// Parse all `CSA:NEXT_STEP` directives from text output.
///
/// Returns the **last** `NEXT_STEP` directive found (later directives
/// override earlier ones, matching the "last writer wins" convention).
pub fn parse_next_step(output: &str) -> Option<String> {
let mut last_step_id = None;

for line in output.lines() {
let trimmed = line.trim();
// Match: <!-- CSA:NEXT_STEP step_id=<value> -->
if let Some(rest) = trimmed.strip_prefix("<!-- CSA:NEXT_STEP")
&& let Some(rest) = rest.strip_suffix("-->")
{
let rest = rest.trim();
if let Some(value) = rest.strip_prefix("step_id=") {
let id = value.trim().trim_matches('"').trim();
if !id.is_empty() {
last_step_id = Some(id.to_string());
}
}
}
}

last_step_id
}

#[cfg(test)]
mod tests {
use super::*;

#[test]
fn parse_next_step_basic() {
let output = "some output\n<!-- CSA:NEXT_STEP step_id=merge -->\nmore output";
assert_eq!(parse_next_step(output), Some("merge".to_string()));
}

#[test]
fn parse_next_step_quoted() {
let output = "<!-- CSA:NEXT_STEP step_id=\"push_and_review\" -->";
assert_eq!(parse_next_step(output), Some("push_and_review".to_string()));
}

#[test]
fn parse_next_step_last_wins() {
let output = "<!-- CSA:NEXT_STEP step_id=first -->\n<!-- CSA:NEXT_STEP step_id=second -->";
assert_eq!(parse_next_step(output), Some("second".to_string()));
}

#[test]
fn parse_next_step_none() {
assert_eq!(parse_next_step("no directives here"), None);
}

#[test]
fn parse_next_step_empty_id() {
assert_eq!(parse_next_step("<!-- CSA:NEXT_STEP step_id= -->"), None);
}
}
Loading
Loading