diff --git a/src/tools/merge_pr.mbt b/src/tools/merge_pr.mbt index 44bcfa16..40869b72 100644 --- a/src/tools/merge_pr.mbt +++ b/src/tools/merge_pr.mbt @@ -685,6 +685,8 @@ async fn merge_pr_audit_receipt_validation_for_head( project_dir : String, head_sha : String, capture : async (@workspace.Command) -> (Int, String), + read_file : (String) -> String, + path_exists : (String) -> Bool, ) -> MergePrAuditReceiptValidation { let tree_sha = match audit_receipt_tree_sha_for_head(project_dir, head_sha, capture) { @@ -692,21 +694,180 @@ async fn merge_pr_audit_receipt_validation_for_head( None => return MergeAuditReceiptTreeUnavailable(file_pr_short_sha(head_sha)) } let receipt_path = file_pr_audit_receipt_path_for_head(tree_sha) - let (exit_code, receipt_body) = capture({ - program: "cat", - args: [receipt_path], - workdir: Some(project_dir), - }) + let receipt_path = project_relative_path(project_dir, receipt_path) + let receipt_body = if path_exists(receipt_path) { + Some(read_file(receipt_path)) + } else { + None + } merge_pr_audit_receipt_validation( - parent_branch, - default_branch, - tree_sha, - if exit_code == 0 { - Some(receipt_body) - } else { - None + parent_branch, default_branch, tree_sha, receipt_body, + ) +} + +///| +async test "merge_pr audit receipt validation reports missing receipt without reading it" { + let head = "abcdef1234567890abcdef1234567890abcdef12" + let tree = "9999999999999999999999999999999999999999" + let captured : Array[@workspace.Command] = [] + let mut read_calls = 0 + let got = merge_pr_audit_receipt_validation_for_head( + "main", + "main", + "/repo", + head, + async fn(cmd : @workspace.Command) -> (Int, String) { + @async.sleep(0) + captured.push(cmd) + if cmd.program == "git" { + (0, tree + "\n") + } else { + (1, "") + } + }, + fn(_) { + read_calls = read_calls + 1 + "" + }, + fn(_) { false }, + ) + match got { + MergeAuditReceiptMissing(short_sha) => + @debug.assert_eq(short_sha, file_pr_short_sha(tree)) + _ => fail("expected missing receipt") + } + @debug.assert_eq(captured.length(), 1) + @debug.assert_eq(captured[0].program, "git") + @debug.assert_eq(read_calls, 0) +} + +///| +async test "merge_pr audit receipt validation parses an injected receipt body" { + let head = "abcdef1234567890abcdef1234567890abcdef12" + let tree = "9999999999999999999999999999999999999999" + let receipt : @types.AuditReceipt = { + sha: head, + tree_sha: tree, + branch: "main", + audited_at: 1L, + agent_id: "main.tl", + findings_count: 0, + findings: Some(""), + } + let receipt_body = receipt.to_json().stringify() + "\n" + let receipt_paths : Array[String] = [] + let got = merge_pr_audit_receipt_validation_for_head( + "main", + "main", + "/repo", + head, + async fn(cmd : @workspace.Command) -> (Int, String) { + @async.sleep(0) + if cmd.program == "git" { + (0, tree + "\n") + } else { + (1, "") + } + }, + fn(path) { + receipt_paths.push(path) + receipt_body + }, + fn(path) { + receipt_paths.push(path) + true + }, + ) + match got { + MergeAuditReceiptValid => () + _ => fail("expected injected valid receipt") + } + @debug.assert_eq(receipt_paths, [ + project_relative_path("/repo", file_pr_audit_receipt_path_for_head(tree)), + project_relative_path("/repo", file_pr_audit_receipt_path_for_head(tree)), + ]) +} + +///| +async test "merge_pr audit receipt validation only captures git tree resolution" { + let head = "abcdef1234567890abcdef1234567890abcdef12" + let tree = "9999999999999999999999999999999999999999" + let programs : Array[String] = [] + let got = merge_pr_audit_receipt_validation_for_head( + "main", + "main", + "/repo", + head, + async fn(cmd : @workspace.Command) -> (Int, String) { + @async.sleep(0) + programs.push(cmd.program) + if cmd.program == "git" { + (0, tree + "\n") + } else { + (1, "") + } + }, + fn(_) { + "{\"sha\":\"" + + head + + "\",\"tree_sha\":\"" + + tree + + "\",\"branch\":\"main\",\"audited_at\":1,\"agent_id\":\"main.tl\",\"findings_count\":0,\"findings\":\"\"}" + }, + fn(_) { true }, + ) + match got { + MergeAuditReceiptValid => () + _ => fail("expected valid receipt") + } + @debug.assert_eq(programs, ["git"]) +} + +///| +async test "merge_pr audit receipt validation reads receipt under project directory" { + let project_dir = "/tmp/choir-merge-pr-receipt-project-" + + @sys.get_pid().to_string() + let head = "abcdef1234567890abcdef1234567890abcdef12" + let tree = "9999999999999999999999999999999999999999" + ignore(@sys.rm_rf(project_dir)) + assert_true(@sys.create_dir_all(project_dir + "/.choir/state/audit-receipts")) + let receipt_path = project_relative_path( + project_dir, + file_pr_audit_receipt_path_for_head(tree), + ) + let receipt : @types.AuditReceipt = { + sha: head, + tree_sha: tree, + branch: "main", + audited_at: 1L, + agent_id: "main.tl", + findings_count: 0, + findings: Some(""), + } + assert_true( + @sys.write_file_sync(receipt_path, receipt.to_json().stringify() + "\n"), + ) + let got = merge_pr_audit_receipt_validation_for_head( + "main", + "main", + project_dir, + head, + async fn(cmd : @workspace.Command) -> (Int, String) { + @async.sleep(0) + if cmd.program == "git" { + (0, tree + "\n") + } else { + (1, "") + } }, + @sys.read_file, + @sys.path_entry_exists, ) + match got { + MergeAuditReceiptValid => () + _ => fail("expected receipt under project directory to validate") + } + ignore(@sys.rm_rf(project_dir)) } ///| @@ -808,6 +969,8 @@ pub async fn merge_pr_require_audit_receipt( gate_state : MergeGateState, pr_first_seen_unix : Int64, capture : async (@workspace.Command) -> (Int, String), + read_file? : (String) -> String = @sys.read_file, + path_exists? : (String) -> Bool = @sys.path_entry_exists, append_file? : (String, String) -> Bool = fn(_, _) { false }, ) -> Result[Unit, @types.ChoirError] { if !file_pr_audit_receipt_required(parent_branch, default_branch) { @@ -827,7 +990,7 @@ pub async fn merge_pr_require_audit_receipt( ) } let validation = merge_pr_audit_receipt_validation_for_head( - parent_branch, default_branch, project_dir, head_sha, capture, + parent_branch, default_branch, project_dir, head_sha, capture, read_file, path_exists, ) match validation { MergeAuditReceiptValid => Ok(()) @@ -2380,6 +2543,8 @@ pub async fn interpret_merge_pr( gate_state, tracked.first_seen, capture, + read_file~, + path_exists~, append_file~, ) { Err(e) => return @types.Response::Err(e.message()) diff --git a/src/tools/merge_pr_test.mbt b/src/tools/merge_pr_test.mbt index c3f4b11c..6661dafd 100644 --- a/src/tools/merge_pr_test.mbt +++ b/src/tools/merge_pr_test.mbt @@ -639,9 +639,6 @@ async test "interpret_merge_pr missing audit receipt policy-blocks on master def if cmd.program == "git" && cmd.args == ["rev-parse", head + "^{tree}"] { return (0, head + "\n") } - if cmd.program == "cat" { - return (1, "") - } (1, "spawn failed") }, fn(_path, content) { @@ -755,9 +752,6 @@ async test "interpret_merge_pr blocks when default branch detection fails" { cmd.args == ["rev-parse", "--abbrev-ref", "HEAD"] { return (0, "feature/audit-gate\n") } - if cmd.program == "cat" { - return (1, "") - } (0, "") }, fn(_path, content) { @@ -819,7 +813,6 @@ async test "interpret_merge_pr audit receipt remains valid when commit changes b @poller.CiRollupStatus::Success, true, ) - let cat_paths : Array[String] = [] let mut runner_calls = 0 let resp = interpret_merge_pr( { @@ -868,20 +861,20 @@ async test "interpret_merge_pr audit receipt remains valid when commit changes b if cmd.program == "git" && cmd.args == ["rev-parse", head + "^{tree}"] { return (0, tree + "\n") } - if cmd.program == "cat" { - cat_paths.push(cmd.args[0]) - return ( - 0, - merge_pr_test_audit_receipt_for_tree(audited_commit, tree, 0), - ) - } (0, "") }, fn(_, _) { true }, - fn(path) { path.has_suffix("/.choir/state/audit-gate.json") }, + fn(path) { + path.has_suffix("/.choir/state/audit-gate.json") || + path.contains("/.choir/state/audit-receipts/") || + path.has_prefix(".choir/state/audit-receipts/") + }, read_file=fn(path) { if path.has_suffix("/.choir/state/audit-gate.json") { "{\"enabled_at_unix\":100}\n" + } else if path.contains("/.choir/state/audit-receipts/") || + path.has_prefix(".choir/state/audit-receipts/") { + merge_pr_test_audit_receipt_for_tree(audited_commit, tree, 0) } else { "" } @@ -893,7 +886,6 @@ async test "interpret_merge_pr audit receipt remains valid when commit changes b @types.Response::Err(msg) => fail("expected merge success: " + msg) } assert_eq(runner_calls, 1) - @debug.assert_eq(cat_paths, [file_pr_audit_receipt_path_for_head(tree)]) ignore(@sys.rm_rf(project_dir)) } @@ -1233,16 +1225,20 @@ async test "interpret_merge_pr main audit gate merges only matching audited head if cmd.program == "git" && cmd.args == ["rev-parse", head + "^{tree}"] { return (0, head + "\n") } - if cmd.program == "cat" { - return (0, merge_pr_test_audit_receipt(head)) - } (0, "") }, fn(_, _) { true }, - fn(path) { path.has_suffix("/.choir/state/audit-gate.json") }, + fn(path) { + path.has_suffix("/.choir/state/audit-gate.json") || + path.contains("/.choir/state/audit-receipts/") || + path.has_prefix(".choir/state/audit-receipts/") + }, read_file=fn(path) { if path.has_suffix("/.choir/state/audit-gate.json") { "{\"enabled_at_unix\":100}\n" + } else if path.contains("/.choir/state/audit-receipts/") || + path.has_prefix(".choir/state/audit-receipts/") { + merge_pr_test_audit_receipt(head) } else { "" } @@ -1329,9 +1325,6 @@ async test "interpret_merge_pr success fires pr_merged completion hooks" { if cmd.program == "git" && cmd.args == ["rev-parse", head + "^{tree}"] { return (0, head + "\n") } - if cmd.program == "cat" { - return (0, merge_pr_test_audit_receipt(head)) - } (0, "") }, fn(_, _) { true }, @@ -1339,7 +1332,8 @@ async test "interpret_merge_pr success fires pr_merged completion hooks" { path.has_suffix("/.choir/state/audit-gate.json") || path.has_suffix("/.choir/hooks/pr_merged.sh") || path.has_suffix("/.choir/hooks/wave_complete.sh") || - path.has_suffix("/.choir/state/audit-receipts/abcdef123456.json") + path.has_suffix("/.choir/state/audit-receipts/abcdef123456.json") || + path.has_prefix(".choir/state/audit-receipts/") }, hook_capture=async fn( path : String, @@ -1362,7 +1356,8 @@ async test "interpret_merge_pr success fires pr_merged completion hooks" { "{\"enabled_at_unix\":100}\n" } else if path.has_suffix( "/.choir/state/audit-receipts/abcdef123456.json", - ) { + ) || + path.has_prefix(".choir/state/audit-receipts/") { merge_pr_test_audit_receipt(head) } else { "" @@ -1462,9 +1457,6 @@ async test "interpret_merge_pr suppresses pr_merged hooks when poller persist fa if cmd.program == "git" && cmd.args == ["rev-parse", head + "^{tree}"] { return (0, head + "\n") } - if cmd.program == "cat" { - return (0, merge_pr_test_audit_receipt(head)) - } (0, "") }, fn(_, _) { true }, @@ -1472,7 +1464,8 @@ async test "interpret_merge_pr suppresses pr_merged hooks when poller persist fa path.has_suffix("/.choir/state/audit-gate.json") || path.has_suffix("/.choir/hooks/pr_merged.sh") || path.has_suffix("/.choir/hooks/wave_complete.sh") || - path.has_suffix("/.choir/state/audit-receipts/abcdef123456.json") + path.has_suffix("/.choir/state/audit-receipts/abcdef123456.json") || + path.has_prefix(".choir/state/audit-receipts/") }, hook_capture=async fn( path : String, @@ -1488,7 +1481,8 @@ async test "interpret_merge_pr suppresses pr_merged hooks when poller persist fa "{\"enabled_at_unix\":100}\n" } else if path.has_suffix( "/.choir/state/audit-receipts/abcdef123456.json", - ) { + ) || + path.has_prefix(".choir/state/audit-receipts/") { merge_pr_test_audit_receipt(head) } else { "" @@ -3648,12 +3642,6 @@ async test "interpret_merge_pr tracked feature merge skips audit gate file and r receipt_captures = receipt_captures + 1 return (1, "") } - if cmd.program == "cat" && - cmd.args.length() == 1 && - cmd.args[0].has_prefix(".choir/state/audit-receipts/") { - receipt_captures = receipt_captures + 1 - return (1, "") - } mock_preflight_capture_no_unresolved_threads(cmd) } let got = interpret_merge_pr( diff --git a/src/tools/pr_merged_completion_hooks.mbt b/src/tools/pr_merged_completion_hooks.mbt index 6be38eae..a8974610 100644 --- a/src/tools/pr_merged_completion_hooks.mbt +++ b/src/tools/pr_merged_completion_hooks.mbt @@ -212,7 +212,8 @@ pub async fn audit_receipt_advisory_state_for_tracked_pr( } match merge_pr_audit_receipt_validation_for_head( - parent_branch, default_branch, project_dir, trimmed_head, capture, + parent_branch, default_branch, project_dir, trimmed_head, capture, read_file, + path_exists, ) { MergeAuditReceiptValid => @types.AuditReceiptState::Valid MergeAuditReceiptFindingsRemain(sha, count) => diff --git a/src/tools/pr_merged_completion_hooks_test.mbt b/src/tools/pr_merged_completion_hooks_test.mbt index 64da8530..a65933f4 100644 --- a/src/tools/pr_merged_completion_hooks_test.mbt +++ b/src/tools/pr_merged_completion_hooks_test.mbt @@ -14,6 +14,27 @@ fn hooks_test_gate_path_exists(path : String) -> Bool { path.has_suffix("/.choir/state/audit-gate.json") } +///| +fn hooks_test_receipt_path_exists(path : String) -> Bool { + path.has_suffix("/.choir/state/audit-gate.json") || + path.contains("/.choir/state/audit-receipts/") || + path.has_prefix(".choir/state/audit-receipts/") +} + +///| +fn hooks_test_read_file_with_receipt( + enabled_at_unix : Int64, + receipt_body : String, +) -> (String) -> String { + fn(path : String) -> String { + if path.has_suffix("/.choir/state/audit-gate.json") { + "{\"enabled_at_unix\":" + enabled_at_unix.to_string() + "}\n" + } else { + receipt_body + } + } +} + ///| fn hooks_test_receipt_for_tree( tree_sha : String, @@ -99,11 +120,11 @@ async test "audit receipt advisory uses resolved default branch on success" { if cmd.program == "git" { (0, tree + "\n") } else { - (0, hooks_test_receipt_for_tree(tree, 0)) + (1, "") } }, - hooks_test_gate_state_reader(1L), - hooks_test_gate_path_exists, + hooks_test_read_file_with_receipt(1L, hooks_test_receipt_for_tree(tree, 0)), + hooks_test_receipt_path_exists, default_branch_result=Some(default_branch), ) assert_eq(state, @types.AuditReceiptState::Valid) @@ -170,11 +191,11 @@ async test "audit_receipt_advisory_state_for_tracked_pr maps gate and receipt st if cmd.program == "git" { (0, tree + "\n") } else { - (0, hooks_test_receipt_for_tree(tree, 0)) + (1, "") } }, - hooks_test_gate_state_reader(1L), - hooks_test_gate_path_exists, + hooks_test_read_file_with_receipt(1L, hooks_test_receipt_for_tree(tree, 0)), + hooks_test_receipt_path_exists, ) assert_eq(valid, @types.AuditReceiptState::Valid) @@ -189,11 +210,11 @@ async test "audit_receipt_advisory_state_for_tracked_pr maps gate and receipt st if cmd.program == "git" { (0, tree + "\n") } else { - (0, hooks_test_receipt_for_tree(tree, 3)) + (1, "") } }, - hooks_test_gate_state_reader(1L), - hooks_test_gate_path_exists, + hooks_test_read_file_with_receipt(1L, hooks_test_receipt_for_tree(tree, 3)), + hooks_test_receipt_path_exists, ) assert_eq( findings, @@ -236,11 +257,11 @@ async test "audit_receipt_advisory_state_for_tracked_pr maps gate and receipt st if cmd.program == "git" { (0, tree + "\n") } else { - (0, "{not json") + (1, "") } }, - hooks_test_gate_state_reader(1L), - hooks_test_gate_path_exists, + hooks_test_read_file_with_receipt(1L, "{not json"), + hooks_test_receipt_path_exists, ) assert_eq( invalid, @@ -258,11 +279,14 @@ async test "audit_receipt_advisory_state_for_tracked_pr maps gate and receipt st if cmd.program == "git" { (0, tree + "\n") } else { - (0, hooks_test_receipt_for_tree(other_tree, 0)) + (1, "") } }, - hooks_test_gate_state_reader(1L), - hooks_test_gate_path_exists, + hooks_test_read_file_with_receipt( + 1L, + hooks_test_receipt_for_tree(other_tree, 0), + ), + hooks_test_receipt_path_exists, ) assert_eq(stale, @types.AuditReceiptState::Pending(head_sha=tree))