From c44d072600dfa45656d8a97fc36a717895b332fd Mon Sep 17 00:00:00 2001 From: Justin <9146678+brickfrog@users.noreply.github.com> Date: Thu, 9 Jul 2026 15:22:28 -0500 Subject: [PATCH 1/2] fix: persist audit state for statusline PR tokens --- src/bin/choir/init.mbt | 1 + src/bin/choir/statusline.mbt | 37 +++++++++ src/poller/detect_test.mbt | 1 + src/poller/poller.mbt | 1 + src/poller/poller_test.mbt | 86 +++++++++++++++++++++ src/poller/pr_health_test.mbt | 1 + src/poller/state.mbt | 62 +++++++++++++++ src/poller/tl_decision_test.mbt | 1 + src/server/delivery_test_helpers_test.mbt | 8 +- src/server/handler_poll_delivery.mbt | 83 +++++++++++++++++++++ src/server/handler_test.mbt | 91 +++++++++++++++++++++++ src/server/recovery_test.mbt | 2 + src/server/server_test.mbt | 1 + src/tools/file_pr.mbt | 1 + src/tools/merge_pr_test.mbt | 1 + src/tools/status_bar_state_test.mbt | 1 + src/tools/wave_state_test.mbt | 1 + 17 files changed, 376 insertions(+), 3 deletions(-) diff --git a/src/bin/choir/init.mbt b/src/bin/choir/init.mbt index 06c574a9..5001d7cb 100644 --- a/src/bin/choir/init.mbt +++ b/src/bin/choir/init.mbt @@ -786,6 +786,7 @@ fn init_test_tracked_pr( mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/bin/choir/statusline.mbt b/src/bin/choir/statusline.mbt index ce68ffa7..720c5819 100644 --- a/src/bin/choir/statusline.mbt +++ b/src/bin/choir/statusline.mbt @@ -429,6 +429,7 @@ fn statusline_test_tracked_pr( mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, @@ -726,6 +727,7 @@ fn statusline_pr_token( Some(0), review_mode=tracked.review_mode, review_required~, + audit_receipt_state=tracked.audit_receipt_state, ) { return match merge_mode { @poller.MergeOperatorMode::Manual => "\u001b[32m#\{n}M\u001b[0m" @@ -743,6 +745,41 @@ fn statusline_pr_token( } } +///| +test "statusline_pr_summary gates merge-ready glyph on audit receipt state" { + let project_dir = "/tmp/choir-statusline-pr-audit-state-" + + @sys.get_pid().to_string() + + "-" + + @env.now().reinterpret_as_int64().to_string() + ignore(@sys.rm_rf(project_dir)) + assert_true(@sys.create_dir_all(project_dir + "/.choir/state")) + ignore( + @sys.write_file_sync( + project_dir + "/.choir/config.toml", + "automerge = false\n", + ), + ) + let pending = { + ..statusline_test_tracked_pr(84, "main.leaf-audit-pending"), + audit_receipt_state: @types.AuditReceiptState::Pending(head_sha="x"), + } + let valid = { + ..statusline_test_tracked_pr(85, "main.leaf-audit-valid"), + audit_receipt_state: @types.AuditReceiptState::Valid, + } + ignore( + @sys.write_file_sync( + project_dir + "/.choir/state/poller_state.json", + "[" + pending.to_json() + "," + valid.to_json() + "]", + ), + ) + let summary = statusline_pr_summary(project_dir) + ignore(@sys.rm_rf(project_dir)) + assert_true(summary.body.contains("#84")) + assert_false(summary.body.contains("#84M")) + assert_true(summary.body.contains("#85M")) +} + ///| fn statusline_pr_summary(project_dir : String) -> StatuslinePrSummary { let tracked_prs = @poller.read_state_file(project_dir) diff --git a/src/poller/detect_test.mbt b/src/poller/detect_test.mbt index ddc182a6..e3d88d11 100644 --- a/src/poller/detect_test.mbt +++ b/src/poller/detect_test.mbt @@ -26,6 +26,7 @@ fn detect_test_tracked( mergeable: Mergeable::Unknown, merge_state_status: MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/poller/poller.mbt b/src/poller/poller.mbt index 0f36f325..7ce87269 100644 --- a/src/poller/poller.mbt +++ b/src/poller/poller.mbt @@ -136,6 +136,7 @@ pub(all) struct TrackedPR { mut mergeable : Mergeable mut merge_state_status : MergeStateStatus mut merged_at_unix : Int64? + mut audit_receipt_state : @types.AuditReceiptState mut last_timeout_at : Int64 /// True once the poller has observed an issue-thread comment authored by a /// GitHub Copilot bot login (REST `/issues/{n}/comments`), distinct from review objects. diff --git a/src/poller/poller_test.mbt b/src/poller/poller_test.mbt index 94f77fda..4f4ea5c2 100644 --- a/src/poller/poller_test.mbt +++ b/src/poller/poller_test.mbt @@ -10,6 +10,92 @@ test "poller_backoff_delay_after_failure exponential ladder capped at 900s" { assert_eq(@poller.poller_backoff_delay_after_failure(50), 900L) } +///| +test "TrackedPR audit receipt state defaults for old JSON and round-trips" { + let old_json = + #|[ + #| { + #| "pr_number": 41, + #| "agent_id": "main.leaf", + #| "branch": "work/leaf", + #| "parent_branch": "main", + #| "pr_url": "https://example.test/pr/41", + #| "first_seen": 1, + #| "last_review_state": "approved", + #| "last_review_bodies": "", + #| "last_sha": "old-head", + #| "notified_parent": true, + #| "last_ci_rollup": "success", + #| "last_timeout_at": 0 + #| } + #|] + let old = parse_tracked_prs_json(old_json) + assert_eq(old.length(), 1) + assert_eq(old[0].audit_receipt_state, @types.AuditReceiptState::NotRequired) + + let pending_json = + #|[ + #| { + #| "pr_number": 42, + #| "agent_id": "main.leaf", + #| "branch": "work/leaf", + #| "parent_branch": "main", + #| "pr_url": "https://example.test/pr/42", + #| "first_seen": 1, + #| "last_review_state": "approved", + #| "last_review_bodies": "", + #| "last_sha": "head", + #| "notified_parent": true, + #| "last_ci_rollup": "success", + #| "last_timeout_at": 0, + #| "audit_receipt_state": {"tag":"Pending","head_sha":"abc"} + #| } + #|] + let pending = parse_tracked_prs_json(pending_json) + assert_eq( + pending[0].audit_receipt_state, + @types.AuditReceiptState::Pending(head_sha="abc"), + ) + let pending_round_trip = parse_tracked_prs_json( + "[" + pending[0].to_json() + "]", + ) + assert_eq( + pending_round_trip[0].audit_receipt_state, + @types.AuditReceiptState::Pending(head_sha="abc"), + ) + + let findings_json = + #|[ + #| { + #| "pr_number": 43, + #| "agent_id": "main.leaf", + #| "branch": "work/leaf", + #| "parent_branch": "main", + #| "pr_url": "https://example.test/pr/43", + #| "first_seen": 1, + #| "last_review_state": "approved", + #| "last_review_bodies": "", + #| "last_sha": "head", + #| "notified_parent": true, + #| "last_ci_rollup": "success", + #| "last_timeout_at": 0, + #| "audit_receipt_state": {"tag":"FindingsRemain","head_sha":"abc","count":2} + #| } + #|] + let findings = parse_tracked_prs_json(findings_json) + assert_eq( + findings[0].audit_receipt_state, + @types.AuditReceiptState::FindingsRemain(head_sha="abc", count=2), + ) + let findings_round_trip = parse_tracked_prs_json( + "[" + findings[0].to_json() + "]", + ) + assert_eq( + findings_round_trip[0].audit_receipt_state, + @types.AuditReceiptState::FindingsRemain(head_sha="abc", count=2), + ) +} + ///| test "PollerState merge receipts are bounded markers" { let ps = @poller.PollerState::new() diff --git a/src/poller/pr_health_test.mbt b/src/poller/pr_health_test.mbt index 54aa585a..398ee042 100644 --- a/src/poller/pr_health_test.mbt +++ b/src/poller/pr_health_test.mbt @@ -26,6 +26,7 @@ fn pr_health_sample( mergeable: Mergeable::Unknown, merge_state_status: MergeStateStatus::Unknown, merged_at_unix: merged_at, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/poller/state.mbt b/src/poller/state.mbt index ae31a88e..af31cefa 100644 --- a/src/poller/state.mbt +++ b/src/poller/state.mbt @@ -121,6 +121,57 @@ fn review_thread_refs_to_json(refs : Array[ReviewThreadRef]) -> Json { items.to_json() } +///| +fn audit_receipt_state_from_tag_fields( + tag : String, + fields : Map[String, Json], +) -> @types.AuditReceiptState? { + match tag { + "NotRequired" => Some(@types.AuditReceiptState::NotRequired) + "Valid" => Some(@types.AuditReceiptState::Valid) + "Pending" => + @types.json_field_string(fields, "head_sha").map(fn(head_sha) { + @types.AuditReceiptState::Pending(head_sha~) + }) + "FindingsRemain" => + match + ( + @types.json_field_string(fields, "head_sha"), + @types.json_field_int(fields, "count"), + ) { + (Some(head_sha), Some(count)) => + Some(@types.AuditReceiptState::FindingsRemain(head_sha~, count~)) + _ => None + } + _ => None + } +} + +///| +fn audit_receipt_state_from_json(json : Json) -> @types.AuditReceiptState? { + match json { + Object(fields) => + match @types.json_field_string(fields, "tag") { + Some(tag) => audit_receipt_state_from_tag_fields(tag, fields) + None => None + } + Array(items) => { + if items.length() != 2 { + return None + } + let tag = match items[0] { + String(value) => value + _ => return None + } + guard items[1] is Object(fields) else { return None } + audit_receipt_state_from_tag_fields(tag, fields) + } + String("NotRequired") => Some(@types.AuditReceiptState::NotRequired) + String("Valid") => Some(@types.AuditReceiptState::Valid) + _ => None + } +} + ///| fn parse_review_thread_refs(json : Json?) -> Array[ReviewThreadRef]? { match json { @@ -169,6 +220,7 @@ pub fn TrackedPR::to_json(self : TrackedPR) -> String { "mergeable": self.mergeable.to_wire_string().to_json(), "merge_state_status": self.merge_state_status.to_wire_string().to_json(), "merged_at_unix": tracked_pr_optional_int64_to_json(self.merged_at_unix), + "audit_receipt_state": self.audit_receipt_state.to_json(), "last_timeout_at": self.last_timeout_at.to_json(), "copilot_issue_comment_seen": self.copilot_issue_comment_seen.to_json(), "orphan_notified": self.orphan_notified.to_json(), @@ -275,6 +327,13 @@ fn parse_tracked_pr(fields : Map[String, Json]) -> TrackedPR? { None => MergeStateStatus::Unknown } let merged_at_unix = @types.json_field_int64(fields, "merged_at_unix") + let audit_receipt_state = match fields.get("audit_receipt_state") { + Some(value) => + audit_receipt_state_from_json(value).unwrap_or( + @types.AuditReceiptState::NotRequired, + ) + None => @types.AuditReceiptState::NotRequired + } let last_timeout_at = match @types.json_field_int64(fields, "last_timeout_at") { Some(value) => value @@ -355,6 +414,7 @@ fn parse_tracked_pr(fields : Map[String, Json]) -> TrackedPR? { mergeable, merge_state_status, merged_at_unix, + audit_receipt_state, last_timeout_at, copilot_issue_comment_seen, orphan_notified, @@ -434,6 +494,7 @@ pub fn PollerState::track( mergeable: Mergeable::Unknown, merge_state_status: MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -563,6 +624,7 @@ pub fn PollerState::track_recovered( mergeable: Mergeable::Unknown, merge_state_status: MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen, orphan_notified: false, diff --git a/src/poller/tl_decision_test.mbt b/src/poller/tl_decision_test.mbt index d31b7fb9..342eeba3 100644 --- a/src/poller/tl_decision_test.mbt +++ b/src/poller/tl_decision_test.mbt @@ -22,6 +22,7 @@ fn sample_tracked( mergeable: Mergeable::Unknown, merge_state_status: MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/server/delivery_test_helpers_test.mbt b/src/server/delivery_test_helpers_test.mbt index f373ac6d..1700ce0e 100644 --- a/src/server/delivery_test_helpers_test.mbt +++ b/src/server/delivery_test_helpers_test.mbt @@ -154,8 +154,9 @@ async fn ServerState::process_poller_tick_with_test( git_pull_capture? : async (@workspace.Command) -> (Int, String) = server_delivery_test_capture, hook_capture? : async (String, String, String?) -> (Int, String) = server_delivery_test_hook_capture, hook_path_exists? : (String) -> Bool = server_delivery_test_path_entry_exists, - audit_receipt_exists? : (String) -> Bool = server_delivery_test_path_entry_exists, - audit_receipt_read? : (String) -> String = server_delivery_test_read_file, + path_exists? : (String) -> Bool = @sys.path_entry_exists, + audit_receipt_exists? : (String) -> Bool = @sys.path_entry_exists, + audit_receipt_read? : (String) -> String = self.read_file, git_state_capture? : async ( String, async (@workspace.Command) -> (Int, String), @@ -195,6 +196,7 @@ async fn ServerState::process_poller_tick_with_test( kill_pgid~, git_capture=server_delivery_test_git_capture, bd_capture=server_delivery_test_capture, - path_exists=@sys.path_entry_exists, + path_exists~, + read_file=audit_receipt_read, ) } diff --git a/src/server/handler_poll_delivery.mbt b/src/server/handler_poll_delivery.mbt index 0b2b2b60..da02e159 100644 --- a/src/server/handler_poll_delivery.mbt +++ b/src/server/handler_poll_delivery.mbt @@ -623,6 +623,20 @@ fn poll_events_pr_merged_numbers( pr_numbers } +///| +fn poll_events_terminal_pr_numbers( + events : Array[@poller.PollEvent], +) -> Array[Int] { + let pr_numbers : Array[Int] = [] + for event in events { + match event { + PRMerged(pr) | PRClosedUnmerged(pr) => pr_numbers.push(pr.pr_number) + _ => () + } + } + pr_numbers +} + ///| fn poll_event_tracked_snapshot( event : @poller.PollEvent, @@ -937,6 +951,7 @@ fn synthetic_untracked_merged_pr( } else { Some(info.merged_at_unix) }, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -2292,6 +2307,55 @@ async fn ServerState::retry_auto_resolve_outdated_review_threads( events } +///| +async fn ServerState::refresh_tracked_pr_audit_receipt_states( + self : ServerState, + capture : async (@workspace.Command) -> (Int, String), + path_exists : (String) -> Bool, + read_file : (String) -> String, + skip_pr_numbers? : Array[Int] = [], +) -> Unit { + let tracked_prs : Array[@poller.TrackedPR] = [] + for tracked in self.poller.tracked.values() { + tracked_prs.push(tracked) + } + if tracked_prs.length() == 0 { + return + } + let default_branch = @tools.detect_default_branch_with_capture( + self.config.project_dir, + capture, + ) + for tracked in tracked_prs { + if skip_pr_numbers.contains(tracked.pr_number) { + continue + } + match self.poller.get(tracked.pr_number) { + Some(current) => { + let audit_gate_path = @tools.merge_pr_audit_gate_state_path( + self.config.project_dir, + ) + let audit_receipt_state = if !path_exists(audit_gate_path) { + current.audit_receipt_state + } else { + @tools.audit_receipt_advisory_state_for_tracked_pr( + self.config.project_dir, + current.parent_branch, + default_branch, + current.last_sha, + current.first_seen, + capture, + read_file, + path_exists, + ) + } + self.poller.restore({ ..current, audit_receipt_state, }) + } + None => () + } + } +} + ///| async fn deliver_pending_poll_events( state : ServerState, @@ -3793,6 +3857,7 @@ pub async fn ServerState::process_poller_tick_with( git_pull_runner? : async (String, async (@workspace.Command) -> (Int, String)) -> Bool = run_parent_git_pull_ff_only, sleep_fn? : async (Int) -> Unit = @async.sleep, kill_pgid? : (Int) -> Unit = default_kill_pgid_best_effort, + read_file? : (String) -> String = self.read_file, ) -> Unit { self.mutex_registry.expire(now) self.warn_on_disk_config_divergence() @@ -3856,6 +3921,12 @@ pub async fn ServerState::process_poller_tick_with( git_pull_runner~, sleep_fn~, ) + self.refresh_tracked_pr_audit_receipt_states( + capture, + path_exists, + read_file, + skip_pr_numbers=poll_events_terminal_pr_numbers(pending_events), + ) self.refresh_beads_status_cache_if_due(now, bd_capture, path_exists) tick_summary.changes = pending_events.length() emit_tick_summary_line( @@ -4094,6 +4165,12 @@ pub async fn ServerState::process_poller_tick( audit_receipt_read~, sleep_fn~, ) + self.refresh_tracked_pr_audit_receipt_states( + capture, + @exec.path_entry_exists, + self.read_file, + skip_pr_numbers=poll_events_terminal_pr_numbers(pending_events), + ) self.refresh_beads_status_cache_if_due( now, @exec.capture_command_stdout_with_stderr_on_error, @exec.path_entry_exists, ) @@ -4185,6 +4262,12 @@ pub async fn ServerState::refresh_pr_with( audit_receipt_read~, sleep_fn~, ) + self.refresh_tracked_pr_audit_receipt_states( + capture, + path_exists, + self.read_file, + skip_pr_numbers=poll_events_terminal_pr_numbers(pending_events), + ) if had_pending_events { self.request_goal_judge_tick() } diff --git a/src/server/handler_test.mbt b/src/server/handler_test.mbt index 815a046b..1ad350ba 100644 --- a/src/server/handler_test.mbt +++ b/src/server/handler_test.mbt @@ -1,3 +1,70 @@ +///| + +///| +async test "poller tick persists injected audit receipt state" { + let project_dir = "/tmp/choir-poller-audit-state-" + + @sys.get_pid().to_string() + + "-" + + @env.now().reinterpret_as_int64().to_string() + ignore(@sys.rm_rf(project_dir)) + assert_true(@sys.create_dir_all(project_dir + "/.choir/state/kv")) + let state = ServerState::new( + { ..@types.Config::default(), project_dir, terminal_session: "s" }, + make_hermetic_recovery_provider(), + ) + state.poller.track( + 44, "main.leaf", "work/leaf", "main", "https://example.test/pr/44", 1L, + ) + state.process_poller_tick_with_test( + 1000L, + 900L, + async fn(_pr_number, _skip_issue_comment_fetch) { + @async.sleep(0) + ( + true, + @poller.ReviewState::Approved, + "head", + @poller.CiRollupStatus::Success, + "", + true, + false, + @poller.Mergeable::Unknown, + @poller.MergeStateStatus::Unknown, + None, + ) + }, + None, + "", + runner=async fn(_cmds) { + @async.sleep(0) + [0] + }, + capture=async fn(_cmd) { + @async.sleep(0) + (1, "") + }, + automerge_capture=async fn(_cmd) { + @async.sleep(0) + (1, "") + }, + path_exists=fn(path) { path.has_suffix("/.choir/state/audit-gate.json") }, + audit_receipt_read=fn(path) { + if path.has_suffix("/.choir/state/audit-gate.json") { + "{\"enabled_at_unix\":1}\n" + } else { + "" + } + }, + ) + let persisted = @poller.read_state_file(project_dir) + assert_eq(persisted.length(), 1) + assert_eq( + persisted[0].audit_receipt_state, + @types.AuditReceiptState::Pending(head_sha="head"), + ) + ignore(@sys.rm_rf(project_dir)) +} + ///| fn[T] handler_test_capture( body : () -> T raise @types.ChoirError, @@ -795,6 +862,7 @@ test "poll_event_beads_note_and_state maps review and ci events" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -3538,6 +3606,7 @@ fn parent_poll_test_ready_tracked(pr_number : Int) -> @poller.TrackedPR { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, @@ -6309,6 +6378,7 @@ fn parent_poll_event_test_tracked( mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, @@ -6344,6 +6414,7 @@ test "parent_poll_event_message PR READY stresses not merged" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -6417,6 +6488,7 @@ test "parent_poll_event_message PR READY shows merge gate when snapshot allows" mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -6603,6 +6675,7 @@ test "parent_poll_event_message MergeReady is actionable in manual mode" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, @@ -6656,6 +6729,7 @@ test "parent_poll_event_message MergeReady is informational in automerge mode" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, @@ -6853,6 +6927,7 @@ test "parent_poll_event_message FixesPushed stresses not merged" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -6908,6 +6983,7 @@ test "parent_poll_event_message FixesPushed single_pass avoids re-review wording mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -6974,6 +7050,7 @@ test "parent_poll_event_message CIFailureCleared stresses re-check not merge" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -7064,6 +7141,7 @@ test "parent_poll_event_message CILatestHeadRollupSucceeded stresses not merge o mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -7263,6 +7341,7 @@ test "leaf_poll_review_injection_message CILatestHeadRollupSucceeded escalates w mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 100L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -7341,6 +7420,7 @@ test "leaf_poll_review_injection_message CILatestHeadRollupSucceeded suppresses mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 100L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -7408,6 +7488,7 @@ test "parent_poll_event_message suppresses non-blocking review comments" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -8673,6 +8754,7 @@ test "parent_poll_event_message REVIEW TIMEOUT uses attention-required decision" mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -8726,6 +8808,7 @@ test "parent_poll_event_message ReviewTimeout is benign when review is skipped" mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 999L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -8780,6 +8863,7 @@ test "parent_poll_event_message CI LATEST HEAD adds escalation when review timeo mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 999L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -8863,6 +8947,7 @@ test "parent_poll_event_message CI FAILED uses attention-required decision" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -8912,6 +8997,7 @@ test "parent_poll_event_message PRMerged is handled by canonical teardown delive mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -8956,6 +9042,7 @@ test "evidence_events_for_poll_snapshot emits normalized snapshot events" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, @@ -9019,6 +9106,7 @@ test "evidence_events_for_poll_snapshot includes merge observed on PRMerged" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -9067,6 +9155,7 @@ test "evidence_events_for_poll_snapshot emits NO MergeObserved for PRClosedUnmer mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -9138,6 +9227,7 @@ async test "deliver_orphaned_pr_notification_for skips finalized merged lifecycl mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -9203,6 +9293,7 @@ async test "deliver_poller_stall_notification_for skips finalized merged lifecyc mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/server/recovery_test.mbt b/src/server/recovery_test.mbt index f47899bc..8ddb6a0d 100644 --- a/src/server/recovery_test.mbt +++ b/src/server/recovery_test.mbt @@ -503,6 +503,7 @@ test "plan_recovery handles offline agent and persisted PR" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, @@ -604,6 +605,7 @@ test "plan_recovery finalizes merged tracked PR after restart" { mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/server/server_test.mbt b/src/server/server_test.mbt index 72f5ac30..3d88b219 100644 --- a/src/server/server_test.mbt +++ b/src/server/server_test.mbt @@ -1942,6 +1942,7 @@ async test "recover_state finalizes merged agent without startup parent notifica mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, diff --git a/src/tools/file_pr.mbt b/src/tools/file_pr.mbt index 19551413..c8079296 100644 --- a/src/tools/file_pr.mbt +++ b/src/tools/file_pr.mbt @@ -71,6 +71,7 @@ pub fn file_pr_plan( mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/tools/merge_pr_test.mbt b/src/tools/merge_pr_test.mbt index a86eb7bf..8ea97fd6 100644 --- a/src/tools/merge_pr_test.mbt +++ b/src/tools/merge_pr_test.mbt @@ -231,6 +231,7 @@ fn merge_pr_test_tracked( mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: false, orphan_notified: false, diff --git a/src/tools/status_bar_state_test.mbt b/src/tools/status_bar_state_test.mbt index bd7df42a..e6b133eb 100644 --- a/src/tools/status_bar_state_test.mbt +++ b/src/tools/status_bar_state_test.mbt @@ -138,6 +138,7 @@ fn status_bar_test_tracked_pr( mergeable: @poller.Mergeable::Unknown, merge_state_status: @poller.MergeStateStatus::Unknown, merged_at_unix: None, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: true, orphan_notified: false, diff --git a/src/tools/wave_state_test.mbt b/src/tools/wave_state_test.mbt index 5a8afec8..9e6202e9 100644 --- a/src/tools/wave_state_test.mbt +++ b/src/tools/wave_state_test.mbt @@ -253,6 +253,7 @@ fn tracked_pr_with( mergeable, merge_state_status, merged_at_unix, + audit_receipt_state: @types.AuditReceiptState::NotRequired, last_timeout_at: 0L, copilot_issue_comment_seen: copilot_comment, orphan_notified: false, From a12aedc53ec9bc29b96ca998a68152ab512598dd Mon Sep 17 00:00:00 2001 From: Justin <9146678+brickfrog@users.noreply.github.com> Date: Thu, 9 Jul 2026 15:45:39 -0500 Subject: [PATCH 2/2] fix: close audit state review findings --- src/poller/poller_test.mbt | 14 +++--- src/poller/state.mbt | 57 ++--------------------- src/server/delivery_test_helpers_test.mbt | 4 +- src/server/handler_poll_delivery.mbt | 36 +++++++------- src/server/handler_test.mbt | 2 - 5 files changed, 29 insertions(+), 84 deletions(-) diff --git a/src/poller/poller_test.mbt b/src/poller/poller_test.mbt index 4f4ea5c2..7980b0f6 100644 --- a/src/poller/poller_test.mbt +++ b/src/poller/poller_test.mbt @@ -29,7 +29,7 @@ test "TrackedPR audit receipt state defaults for old JSON and round-trips" { #| "last_timeout_at": 0 #| } #|] - let old = parse_tracked_prs_json(old_json) + let old = @poller.parse_tracked_prs_json(old_json) assert_eq(old.length(), 1) assert_eq(old[0].audit_receipt_state, @types.AuditReceiptState::NotRequired) @@ -48,15 +48,15 @@ test "TrackedPR audit receipt state defaults for old JSON and round-trips" { #| "notified_parent": true, #| "last_ci_rollup": "success", #| "last_timeout_at": 0, - #| "audit_receipt_state": {"tag":"Pending","head_sha":"abc"} + #| "audit_receipt_state": ["Pending",{"head_sha":"abc"}] #| } #|] - let pending = parse_tracked_prs_json(pending_json) + let pending = @poller.parse_tracked_prs_json(pending_json) assert_eq( pending[0].audit_receipt_state, @types.AuditReceiptState::Pending(head_sha="abc"), ) - let pending_round_trip = parse_tracked_prs_json( + let pending_round_trip = @poller.parse_tracked_prs_json( "[" + pending[0].to_json() + "]", ) assert_eq( @@ -79,15 +79,15 @@ test "TrackedPR audit receipt state defaults for old JSON and round-trips" { #| "notified_parent": true, #| "last_ci_rollup": "success", #| "last_timeout_at": 0, - #| "audit_receipt_state": {"tag":"FindingsRemain","head_sha":"abc","count":2} + #| "audit_receipt_state": ["FindingsRemain",{"head_sha":"abc","count":2}] #| } #|] - let findings = parse_tracked_prs_json(findings_json) + let findings = @poller.parse_tracked_prs_json(findings_json) assert_eq( findings[0].audit_receipt_state, @types.AuditReceiptState::FindingsRemain(head_sha="abc", count=2), ) - let findings_round_trip = parse_tracked_prs_json( + let findings_round_trip = @poller.parse_tracked_prs_json( "[" + findings[0].to_json() + "]", ) assert_eq( diff --git a/src/poller/state.mbt b/src/poller/state.mbt index af31cefa..91d11579 100644 --- a/src/poller/state.mbt +++ b/src/poller/state.mbt @@ -121,57 +121,6 @@ fn review_thread_refs_to_json(refs : Array[ReviewThreadRef]) -> Json { items.to_json() } -///| -fn audit_receipt_state_from_tag_fields( - tag : String, - fields : Map[String, Json], -) -> @types.AuditReceiptState? { - match tag { - "NotRequired" => Some(@types.AuditReceiptState::NotRequired) - "Valid" => Some(@types.AuditReceiptState::Valid) - "Pending" => - @types.json_field_string(fields, "head_sha").map(fn(head_sha) { - @types.AuditReceiptState::Pending(head_sha~) - }) - "FindingsRemain" => - match - ( - @types.json_field_string(fields, "head_sha"), - @types.json_field_int(fields, "count"), - ) { - (Some(head_sha), Some(count)) => - Some(@types.AuditReceiptState::FindingsRemain(head_sha~, count~)) - _ => None - } - _ => None - } -} - -///| -fn audit_receipt_state_from_json(json : Json) -> @types.AuditReceiptState? { - match json { - Object(fields) => - match @types.json_field_string(fields, "tag") { - Some(tag) => audit_receipt_state_from_tag_fields(tag, fields) - None => None - } - Array(items) => { - if items.length() != 2 { - return None - } - let tag = match items[0] { - String(value) => value - _ => return None - } - guard items[1] is Object(fields) else { return None } - audit_receipt_state_from_tag_fields(tag, fields) - } - String("NotRequired") => Some(@types.AuditReceiptState::NotRequired) - String("Valid") => Some(@types.AuditReceiptState::Valid) - _ => None - } -} - ///| fn parse_review_thread_refs(json : Json?) -> Array[ReviewThreadRef]? { match json { @@ -329,9 +278,9 @@ fn parse_tracked_pr(fields : Map[String, Json]) -> TrackedPR? { let merged_at_unix = @types.json_field_int64(fields, "merged_at_unix") let audit_receipt_state = match fields.get("audit_receipt_state") { Some(value) => - audit_receipt_state_from_json(value).unwrap_or( - @types.AuditReceiptState::NotRequired, - ) + @json.from_json(value) catch { + _ => @types.AuditReceiptState::NotRequired + } None => @types.AuditReceiptState::NotRequired } let last_timeout_at = match diff --git a/src/server/delivery_test_helpers_test.mbt b/src/server/delivery_test_helpers_test.mbt index 1700ce0e..640ca85b 100644 --- a/src/server/delivery_test_helpers_test.mbt +++ b/src/server/delivery_test_helpers_test.mbt @@ -155,8 +155,8 @@ async fn ServerState::process_poller_tick_with_test( hook_capture? : async (String, String, String?) -> (Int, String) = server_delivery_test_hook_capture, hook_path_exists? : (String) -> Bool = server_delivery_test_path_entry_exists, path_exists? : (String) -> Bool = @sys.path_entry_exists, - audit_receipt_exists? : (String) -> Bool = @sys.path_entry_exists, - audit_receipt_read? : (String) -> String = self.read_file, + audit_receipt_exists? : (String) -> Bool = server_delivery_test_path_entry_exists, + audit_receipt_read? : (String) -> String = server_delivery_test_read_file, git_state_capture? : async ( String, async (@workspace.Command) -> (Int, String), diff --git a/src/server/handler_poll_delivery.mbt b/src/server/handler_poll_delivery.mbt index da02e159..4198d5b4 100644 --- a/src/server/handler_poll_delivery.mbt +++ b/src/server/handler_poll_delivery.mbt @@ -2314,10 +2314,17 @@ async fn ServerState::refresh_tracked_pr_audit_receipt_states( path_exists : (String) -> Bool, read_file : (String) -> String, skip_pr_numbers? : Array[Int] = [], + only_pr_number? : Int? = None, ) -> Unit { let tracked_prs : Array[@poller.TrackedPR] = [] for tracked in self.poller.tracked.values() { - tracked_prs.push(tracked) + let matches_target = match only_pr_number { + Some(pr_number) => tracked.pr_number == pr_number + None => true + } + if !skip_pr_numbers.contains(tracked.pr_number) && matches_target { + tracked_prs.push(tracked) + } } if tracked_prs.length() == 0 { return @@ -2327,28 +2334,18 @@ async fn ServerState::refresh_tracked_pr_audit_receipt_states( capture, ) for tracked in tracked_prs { - if skip_pr_numbers.contains(tracked.pr_number) { - continue - } match self.poller.get(tracked.pr_number) { Some(current) => { - let audit_gate_path = @tools.merge_pr_audit_gate_state_path( + let audit_receipt_state = @tools.audit_receipt_advisory_state_for_tracked_pr( self.config.project_dir, + current.parent_branch, + default_branch, + current.last_sha, + current.first_seen, + capture, + read_file, + path_exists, ) - let audit_receipt_state = if !path_exists(audit_gate_path) { - current.audit_receipt_state - } else { - @tools.audit_receipt_advisory_state_for_tracked_pr( - self.config.project_dir, - current.parent_branch, - default_branch, - current.last_sha, - current.first_seen, - capture, - read_file, - path_exists, - ) - } self.poller.restore({ ..current, audit_receipt_state, }) } None => () @@ -4267,6 +4264,7 @@ pub async fn ServerState::refresh_pr_with( path_exists, self.read_file, skip_pr_numbers=poll_events_terminal_pr_numbers(pending_events), + only_pr_number=Some(pr_number), ) if had_pending_events { self.request_goal_judge_tick() diff --git a/src/server/handler_test.mbt b/src/server/handler_test.mbt index 1ad350ba..8ac1966c 100644 --- a/src/server/handler_test.mbt +++ b/src/server/handler_test.mbt @@ -1,5 +1,3 @@ -///| - ///| async test "poller tick persists injected audit receipt state" { let project_dir = "/tmp/choir-poller-audit-state-" +