From 2d9f74aa1c1dced7264716a77b95d89214227f07 Mon Sep 17 00:00:00 2001 From: Justin <9146678+brickfrog@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:16:12 -0500 Subject: [PATCH 1/3] fix: read audit receipts without cat --- src/tools/merge_pr.mbt | 143 ++++++++++++++++-- src/tools/merge_pr_test.mbt | 59 +++----- src/tools/pr_merged_completion_hooks.mbt | 3 +- src/tools/pr_merged_completion_hooks_test.mbt | 53 +++++-- 4 files changed, 193 insertions(+), 65 deletions(-) diff --git a/src/tools/merge_pr.mbt b/src/tools/merge_pr.mbt index 44bcfa16..28f03727 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,132 @@ 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_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) => + assert_eq(short_sha, file_pr_short_sha(tree)) + _ => fail("expected missing receipt") + } + assert_eq(captured.length(), 1) + assert_eq(captured[0].program, "git") + 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") + } + assert_eq(receipt_paths, [ + file_pr_audit_receipt_path_for_head(tree), + 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") + } + assert_eq(programs, ["git"]) } ///| @@ -808,6 +921,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 +942,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 +2495,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..f689ef6c 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,18 @@ 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.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.has_prefix(".choir/state/audit-receipts/") { + merge_pr_test_audit_receipt_for_tree(audited_commit, tree, 0) } else { "" } @@ -893,7 +884,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 +1223,21 @@ 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.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.has_suffix( + "/.choir/state/audit-receipts/abcdef123456.json", + ) || + path.has_prefix(".choir/state/audit-receipts/") { + merge_pr_test_audit_receipt(head) } else { "" } @@ -1329,9 +1324,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 +1331,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 +1355,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 +1456,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 +1463,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 +1480,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 +3641,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..41022cb2 100644 --- a/src/tools/pr_merged_completion_hooks_test.mbt +++ b/src/tools/pr_merged_completion_hooks_test.mbt @@ -14,6 +14,26 @@ 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.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 +119,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 +190,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 +209,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 +256,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 +278,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)) From 4568bb4536bb3069624a5731ebec8e4667ad6f1c Mon Sep 17 00:00:00 2001 From: Justin <9146678+brickfrog@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:34:33 -0500 Subject: [PATCH 2/3] fix: anchor audit receipt reads to project root --- src/tools/merge_pr.mbt | 56 +++++++++++++++++-- src/tools/merge_pr_test.mbt | 9 +-- src/tools/pr_merged_completion_hooks_test.mbt | 1 + 3 files changed, 58 insertions(+), 8 deletions(-) diff --git a/src/tools/merge_pr.mbt b/src/tools/merge_pr.mbt index 28f03727..cfa2deea 100644 --- a/src/tools/merge_pr.mbt +++ b/src/tools/merge_pr.mbt @@ -694,6 +694,7 @@ 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 receipt_path = project_relative_path(project_dir, receipt_path) let receipt_body = if path_exists(receipt_path) { Some(read_file(receipt_path)) } else { @@ -781,9 +782,9 @@ async test "merge_pr audit receipt validation parses an injected receipt body" { MergeAuditReceiptValid => () _ => fail("expected injected valid receipt") } - assert_eq(receipt_paths, [ - file_pr_audit_receipt_path_for_head(tree), - file_pr_audit_receipt_path_for_head(tree), + @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)), ]) } @@ -819,7 +820,54 @@ async test "merge_pr audit receipt validation only captures git tree resolution" MergeAuditReceiptValid => () _ => fail("expected valid receipt") } - assert_eq(programs, ["git"]) + @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)) } ///| diff --git a/src/tools/merge_pr_test.mbt b/src/tools/merge_pr_test.mbt index f689ef6c..6661dafd 100644 --- a/src/tools/merge_pr_test.mbt +++ b/src/tools/merge_pr_test.mbt @@ -866,12 +866,14 @@ async test "interpret_merge_pr audit receipt remains valid when commit changes b fn(_, _) { true }, 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.has_prefix(".choir/state/audit-receipts/") { + } 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 { "" @@ -1228,14 +1230,13 @@ async test "interpret_merge_pr main audit gate merges only matching audited head fn(_, _) { true }, 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.has_suffix( - "/.choir/state/audit-receipts/abcdef123456.json", - ) || + } else if path.contains("/.choir/state/audit-receipts/") || path.has_prefix(".choir/state/audit-receipts/") { merge_pr_test_audit_receipt(head) } else { diff --git a/src/tools/pr_merged_completion_hooks_test.mbt b/src/tools/pr_merged_completion_hooks_test.mbt index 41022cb2..a65933f4 100644 --- a/src/tools/pr_merged_completion_hooks_test.mbt +++ b/src/tools/pr_merged_completion_hooks_test.mbt @@ -17,6 +17,7 @@ fn hooks_test_gate_path_exists(path : String) -> Bool { ///| 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/") } From 975afa7d641c8428b497f15db1c0bb69cc4090aa Mon Sep 17 00:00:00 2001 From: Justin <9146678+brickfrog@users.noreply.github.com> Date: Thu, 9 Jul 2026 18:41:10 -0500 Subject: [PATCH 3/3] fix: use debug assertions in receipt tests --- src/tools/merge_pr.mbt | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/tools/merge_pr.mbt b/src/tools/merge_pr.mbt index cfa2deea..40869b72 100644 --- a/src/tools/merge_pr.mbt +++ b/src/tools/merge_pr.mbt @@ -733,12 +733,12 @@ async test "merge_pr audit receipt validation reports missing receipt without re ) match got { MergeAuditReceiptMissing(short_sha) => - assert_eq(short_sha, file_pr_short_sha(tree)) + @debug.assert_eq(short_sha, file_pr_short_sha(tree)) _ => fail("expected missing receipt") } - assert_eq(captured.length(), 1) - assert_eq(captured[0].program, "git") - assert_eq(read_calls, 0) + @debug.assert_eq(captured.length(), 1) + @debug.assert_eq(captured[0].program, "git") + @debug.assert_eq(read_calls, 0) } ///|