From 1b0b24c7e0de8b6148e6e55967665b8bf2aa884b Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 26 Jul 2026 12:33:48 +0900 Subject: [PATCH 1/5] Fix autonomous merge review findings --- bin/agent-workflow-seam-doctor | 1 + bin/agent-workflow-seam-doctor-test.rb | 48 +++++++++++++++++ skills/pr-batch/SKILL.md | 5 +- .../pr-batch/bin/autonomous-merge-calibrate | 16 +++--- .../bin/autonomous-merge-calibrate-test.rb | 54 +++++++++++++++++++ .../bin/goal-completion-contract-test.rb | 18 +++++++ workflows/pr-processing.md | 1 + 7 files changed, 134 insertions(+), 9 deletions(-) diff --git a/bin/agent-workflow-seam-doctor b/bin/agent-workflow-seam-doctor index 70e809fa..97ee2e93 100755 --- a/bin/agent-workflow-seam-doctor +++ b/bin/agent-workflow-seam-doctor @@ -1666,6 +1666,7 @@ module AgentWorkflowSeamDoctor config.each do |key, value| next if REQUIRED_POLICY_KEYS.include?(key) + next if key == "autonomous_merge" next unless unresolved_policy_value?(value) issues << "unresolved policy value for key: #{key}" diff --git a/bin/agent-workflow-seam-doctor-test.rb b/bin/agent-workflow-seam-doctor-test.rb index 571e3c5c..c7abbc05 100755 --- a/bin/agent-workflow-seam-doctor-test.rb +++ b/bin/agent-workflow-seam-doctor-test.rb @@ -413,6 +413,54 @@ def test_optional_autonomous_merge_policy_uses_the_shared_closed_schema end end + def test_optional_autonomous_merge_policy_accepts_an_exact_empty_mapping_seed + with_repo do |root| + write_valid_binstub_contract(root) + write_policy(root, POLICY.merge("autonomous_merge" => {})) + write_skill(root, "No commands here.\n") + + policy_text = File.read(File.join(root, ".agents/agent-workflow.yml"), encoding: "UTF-8") + out, status = run_doctor(root) + + assert_includes policy_text, "autonomous_merge: {}\n" + assert status.success?, out + end + end + + def test_optional_autonomous_merge_policy_accepts_runtime_valid_empty_arrays + with_repo do |root| + write_valid_binstub_contract(root) + write_policy( + root, + POLICY.merge( + "autonomous_merge" => { + "human_review_paths" => [], + "policy_paths" => [], + "generated_paths" => [] + } + ) + ) + write_skill(root, "No commands here.\n") + + out, status = run_doctor(root) + + assert status.success?, out + end + end + + def test_empty_arrays_outside_autonomous_merge_remain_unresolved + with_repo do |root| + write_valid_binstub_contract(root) + write_policy(root, POLICY.merge("custom_runtime_paths" => [])) + write_skill(root, "No commands here.\n") + + out, status = run_doctor(root) + + refute status.success? + assert_includes out, "unresolved policy value for key: custom_runtime_paths" + end + end + def test_invalid_autonomous_merge_policy_is_reported_by_the_seam_doctor with_repo do |root| write_valid_binstub_contract(root) diff --git a/skills/pr-batch/SKILL.md b/skills/pr-batch/SKILL.md index c5b76aea..77412c4e 100644 --- a/skills/pr-batch/SKILL.md +++ b/skills/pr-batch/SKILL.md @@ -197,7 +197,10 @@ Ask only for missing data. If the user already supplied an exact value, use it. 10. **Lane split**: exact per-machine list, odd/even, labels, area, owner, or another explicit partition. 11. **Permissions**: confirm the current session can run without blocking worker approval prompts. 12. **Question handling**: labels or comments to use for blocking questions, plus where non-blocking decisions should be recorded. -13. **Completion states**: `merged`, `ready-gates-clean`, `ready-no-merge-authority`, `waiting-on-checks-or-review`, `external-gate-failing`, `blocked-user-input`, or `no-pr-evidence`. +13. **Completion states**: `merged`, `ready-gates-clean`, `ready-no-merge-authority`, + `ready-human-review-required`, `autonomous-merge-evidence-unknown`, + `waiting-on-checks-or-review`, `external-gate-failing`, `blocked-user-input`, + or `no-pr-evidence`. ## Canonical Readiness Vocabulary diff --git a/skills/pr-batch/bin/autonomous-merge-calibrate b/skills/pr-batch/bin/autonomous-merge-calibrate index 683474ef..35cdd5b8 100755 --- a/skills/pr-batch/bin/autonomous-merge-calibrate +++ b/skills/pr-batch/bin/autonomous-merge-calibrate @@ -79,13 +79,18 @@ rescue Date::Error, KeyError, TypeError abort "calibration PR merged_at must be an ISO 8601 timestamp" end -def validate_calibration_pr_identity!(pull_request) +def validate_calibration_pr_repository!(pull_request) abort "calibration PR must be a mapping" unless pull_request.is_a?(Hash) repository = pull_request["repository"] unless repository.is_a?(String) && repository.match?(%r{\A[^/\s]+/[^/\s]+\z}) abort "calibration PR repository must use OWNER/REPO form" end + repository +end + +def validate_calibration_pr_identity!(pull_request) + repository = validate_calibration_pr_repository!(pull_request) unless pull_request["number"].is_a?(Integer) && pull_request["number"].positive? abort "calibration PR number must be a positive integer" end @@ -128,13 +133,8 @@ window_covered = requested_window_covered?(scope, options) effective_scope_complete = scope["complete"] == true && window_covered repositories = options[:repositories].empty? ? scope.fetch("repositories") : options[:repositories] -unless options[:repositories].any? - abort "calibration PR must be a mapping" unless prs.all? { |pr| pr.is_a?(Hash) } - unless prs.all? { |pr| pr["repository"].is_a?(String) } - abort "calibration PR repository must use OWNER/REPO form" - end -end -prs = prs.select { |pr| pr.is_a?(Hash) && repositories.include?(pr["repository"]) } +prs.each { |pr| validate_calibration_pr_repository!(pr) } +prs = prs.select { |pr| repositories.include?(pr.fetch("repository")) } candidate_identities = {} merged_at_instants = {}.compare_by_identity prs.each do |pr| diff --git a/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb b/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb index 86302edb..013f09ef 100755 --- a/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb +++ b/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb @@ -387,6 +387,60 @@ def test_selected_pr_identities_must_be_unique assert_includes stderr, "calibration PR identities must be unique" end + def test_every_pr_repository_shape_is_validated_before_repository_filtering + selected = pr( + "example/one", 1, + files: 1, lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["app"] + ) + selected["semantic_inspection"] = "reviewed" + base_dataset = { + "contract" => "autonomous-merge-calibration-dataset", + "version" => 1, + "scope" => { + "complete" => true, + "window" => "pre-filter validation fixture", + "repositories" => ["example/one"], + "reviewed_heads_decision" => { + "disposition" => "enforced", + "rationale" => "Every dataset entry must be structurally valid before filtering." + } + } + } + malformed_repository = selected.merge("repository" => "not-owner-repo", "number" => 2) + cases = { + "--repo filtering" => [ + base_dataset.merge("prs" => [selected, "not-a-mapping"]), + ["--repo", "example/one"], + "calibration PR must be a mapping" + ], + "dataset-scope filtering" => [ + base_dataset.merge("prs" => [selected, malformed_repository]), + [], + "calibration PR repository must use OWNER/REPO form" + ] + } + + results = cases.transform_values do |dataset, arguments, expected_error| + stdout, stderr, status = run_calibrator(dataset, *arguments) + { + "stdout" => stdout, + "stderr" => stderr, + "status" => status, + "expected_error" => expected_error + } + end + failures = results.filter_map do |label, result| + next unless result.fetch("status").success? || !result.fetch("stdout").empty? || + !result.fetch("stderr").include?(result.fetch("expected_error")) + + "#{label}: status=#{result.fetch('status').exitstatus}, stdout=#{result.fetch('stdout').inspect}, " \ + "stderr=#{result.fetch('stderr').inspect}" + end + + assert_empty failures, failures.join("\n") + end + def test_filtered_malformed_metrics_do_not_contaminate_zero_and_null_boundaries selected = pr( "example/one", 1, diff --git a/skills/pr-batch/bin/goal-completion-contract-test.rb b/skills/pr-batch/bin/goal-completion-contract-test.rb index afc40b72..d29bb0da 100755 --- a/skills/pr-batch/bin/goal-completion-contract-test.rb +++ b/skills/pr-batch/bin/goal-completion-contract-test.rb @@ -154,6 +154,8 @@ merged ready-gates-clean ready-no-merge-authority + ready-human-review-required + autonomous-merge-evidence-unknown waiting-on-checks-or-review external-gate-failing blocked-user-input @@ -569,6 +571,22 @@ def test_workflow_defines_canonical_readiness_vocabulary assert_text_includes workflow_text, "UNKNOWN", "workflows/pr-processing.md" end + def test_completion_state_checklists_match_canonical_readiness_vocabulary + surfaces = { + "skills/pr-batch/SKILL.md Completion states" => @pr_batch_skill, + "workflows/pr-processing.md Completion states" => @workflow + } + mismatches = surfaces.filter_map do |label, text| + paragraph = text.match(/(?:\*\*)?Completion states(?:\*\*)?:.*?(?=\n\n)/m)&.[](0) + actual = paragraph&.scan(/`([^`]+)`/)&.flatten + next if actual == CANONICAL_READINESS_STATES + + "#{label}: expected #{CANONICAL_READINESS_STATES.inspect}, got #{actual.inspect}" + end + + assert_empty mismatches, mismatches.join("\n") + end + def test_planning_skills_link_to_canonical_readiness_vocabulary { "skills/spec/SKILL.md" => extract_markdown_section(@spec_skill, "## Canonical Readiness Vocabulary", end_heading: /^##\s+/), diff --git a/workflows/pr-processing.md b/workflows/pr-processing.md index 362fab2d..6256b7a7 100644 --- a/workflows/pr-processing.md +++ b/workflows/pr-processing.md @@ -580,6 +580,7 @@ The user should not need to write a long launch prompt. If the request is short, - Permissions: whether the current session can run without blocking worker approval prompts. - Question handling: labels or comments to use for blocking questions, plus where non-blocking decisions should be recorded. - Completion states: `merged`, `ready-gates-clean`, `ready-no-merge-authority`, + `ready-human-review-required`, `autonomous-merge-evidence-unknown`, `waiting-on-checks-or-review`, `external-gate-failing`, `blocked-user-input`, or `no-pr-evidence`. From 811d1c96a830d3e605baa0e7d5811d0491e6d08a Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 26 Jul 2026 13:04:24 +0900 Subject: [PATCH 2/5] Reject nested autonomous merge placeholders --- bin/agent-workflow-seam-doctor | 17 ++++++++++---- bin/agent-workflow-seam-doctor-test.rb | 32 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/bin/agent-workflow-seam-doctor b/bin/agent-workflow-seam-doctor index 97ee2e93..e037ff1b 100755 --- a/bin/agent-workflow-seam-doctor +++ b/bin/agent-workflow-seam-doctor @@ -1666,8 +1666,10 @@ module AgentWorkflowSeamDoctor config.each do |key, value| next if REQUIRED_POLICY_KEYS.include?(key) - next if key == "autonomous_merge" - next unless unresolved_policy_value?(value) + next unless unresolved_policy_value?( + value, + empty_collections_unresolved: key != "autonomous_merge" + ) issues << "unresolved policy value for key: #{key}" end @@ -1720,14 +1722,19 @@ module AgentWorkflowSeamDoctor YAML.safe_load(File.read(path, encoding: "UTF-8"), aliases: false) || {} end - def unresolved_policy_value?(value) + def unresolved_policy_value?(value, empty_collections_unresolved: true) case value when String unresolved_template_value?(value) when Array - value.empty? || value.any? { |entry| unresolved_policy_value?(entry) } + (empty_collections_unresolved && value.empty?) || value.any? do |entry| + unresolved_policy_value?(entry, empty_collections_unresolved:) + end when Hash - value.empty? || value.any? { |key, entry| unresolved_policy_value?(key.to_s) || unresolved_policy_value?(entry) } + (empty_collections_unresolved && value.empty?) || value.any? do |key, entry| + unresolved_policy_value?(key.to_s, empty_collections_unresolved:) || + unresolved_policy_value?(entry, empty_collections_unresolved:) + end else value.nil? end diff --git a/bin/agent-workflow-seam-doctor-test.rb b/bin/agent-workflow-seam-doctor-test.rb index c7abbc05..c44d34de 100755 --- a/bin/agent-workflow-seam-doctor-test.rb +++ b/bin/agent-workflow-seam-doctor-test.rb @@ -448,6 +448,38 @@ def test_optional_autonomous_merge_policy_accepts_runtime_valid_empty_arrays end end + def test_optional_autonomous_merge_policy_rejects_nested_unresolved_placeholders + variants = { + "direct list scalar" => { + "policy_paths" => [""] + }, + "nested mapping list scalar" => { + "safe_path_groups" => { + "documentation" => { + "include" => ["docs/**"], + "exclude" => [""] + } + } + } + } + results = variants.transform_values do |autonomous_merge| + with_repo do |root| + write_valid_binstub_contract(root) + write_policy(root, POLICY.merge("autonomous_merge" => autonomous_merge)) + write_skill(root, "No commands here.\n") + + run_doctor(root) + end + end + failures = results.filter_map do |label, (out, status)| + next unless status.success? || !out.include?("unresolved policy value for key: autonomous_merge") + + "#{label}: status=#{status.exitstatus}, output=#{out.inspect}" + end + + assert_empty failures, failures.join("\n") + end + def test_empty_arrays_outside_autonomous_merge_remain_unresolved with_repo do |root| write_valid_binstub_contract(root) From 206e6462def52d57ffa2f5516d90d84a5f4a41b2 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 26 Jul 2026 13:43:27 +0900 Subject: [PATCH 3/5] Tighten autonomous merge contract tests --- bin/agent-workflow-seam-doctor-test.rb | 18 ++++++----- .../bin/goal-completion-contract-test.rb | 32 ++++++++++++++++--- 2 files changed, 37 insertions(+), 13 deletions(-) diff --git a/bin/agent-workflow-seam-doctor-test.rb b/bin/agent-workflow-seam-doctor-test.rb index c44d34de..5a73e9e3 100755 --- a/bin/agent-workflow-seam-doctor-test.rb +++ b/bin/agent-workflow-seam-doctor-test.rb @@ -480,16 +480,18 @@ def test_optional_autonomous_merge_policy_rejects_nested_unresolved_placeholders assert_empty failures, failures.join("\n") end - def test_empty_arrays_outside_autonomous_merge_remain_unresolved - with_repo do |root| - write_valid_binstub_contract(root) - write_policy(root, POLICY.merge("custom_runtime_paths" => [])) - write_skill(root, "No commands here.\n") + def test_empty_collections_outside_autonomous_merge_remain_unresolved + [[], {}].each do |empty_collection| + with_repo do |root| + write_valid_binstub_contract(root) + write_policy(root, POLICY.merge("custom_runtime_paths" => empty_collection)) + write_skill(root, "No commands here.\n") - out, status = run_doctor(root) + out, status = run_doctor(root) - refute status.success? - assert_includes out, "unresolved policy value for key: custom_runtime_paths" + refute status.success?, empty_collection.class.name + assert_includes out, "unresolved policy value for key: custom_runtime_paths", empty_collection.class.name + end end end diff --git a/skills/pr-batch/bin/goal-completion-contract-test.rb b/skills/pr-batch/bin/goal-completion-contract-test.rb index d29bb0da..6b4ff5b3 100755 --- a/skills/pr-batch/bin/goal-completion-contract-test.rb +++ b/skills/pr-batch/bin/goal-completion-contract-test.rb @@ -199,6 +199,12 @@ def extract_markdown_section(text, heading, end_heading: /^###\s+/) text[body_start...body_end] end +def completion_state_checklist(text, heading:, end_heading:) + section = extract_markdown_section(text, heading, end_heading:) + paragraph = section.match(/(?:\*\*)?Completion states(?:\*\*)?:.*?(?=\n\n)/m)&.[](0) + paragraph&.scan(/`([^`]+)`/)&.flatten +end + def contract_line(text) text.lines.grep(/^Goal Mode Completion Contract:/).first&.chomp end @@ -573,12 +579,11 @@ def test_workflow_defines_canonical_readiness_vocabulary def test_completion_state_checklists_match_canonical_readiness_vocabulary surfaces = { - "skills/pr-batch/SKILL.md Completion states" => @pr_batch_skill, - "workflows/pr-processing.md Completion states" => @workflow + "skills/pr-batch/SKILL.md Required Interview" => [@pr_batch_skill, "## Required Interview", /^##\s+/], + "workflows/pr-processing.md Short Invocation" => [@workflow, "### Short Invocation", /^###\s+/] } - mismatches = surfaces.filter_map do |label, text| - paragraph = text.match(/(?:\*\*)?Completion states(?:\*\*)?:.*?(?=\n\n)/m)&.[](0) - actual = paragraph&.scan(/`([^`]+)`/)&.flatten + mismatches = surfaces.filter_map do |label, (text, heading, end_heading)| + actual = completion_state_checklist(text, heading:, end_heading:) next if actual == CANONICAL_READINESS_STATES "#{label}: expected #{CANONICAL_READINESS_STATES.inspect}, got #{actual.inspect}" @@ -587,6 +592,23 @@ def test_completion_state_checklists_match_canonical_readiness_vocabulary assert_empty mismatches, mismatches.join("\n") end + def test_completion_state_checklists_ignore_earlier_duplicate_paragraphs + decoy = "Completion states: #{CANONICAL_READINESS_STATES.map { |state| "`#{state}`" }.join(', ')}.\n\n" + surfaces = { + "skills/pr-batch/SKILL.md Required Interview" => [@pr_batch_skill, "## Required Interview", /^##\s+/], + "workflows/pr-processing.md Short Invocation" => [@workflow, "### Short Invocation", /^###\s+/] + } + false_positives = surfaces.filter_map do |label, (text, heading, end_heading)| + mutation = text.sub("`ready-human-review-required`, ", "") + raise "fixture mutation missed #{label}" if mutation == text + + actual = completion_state_checklist("#{decoy}#{mutation}", heading:, end_heading:) + label if actual == CANONICAL_READINESS_STATES + end + + assert_empty false_positives, "earlier duplicate paragraph masked drift in: #{false_positives.join(', ')}" + end + def test_planning_skills_link_to_canonical_readiness_vocabulary { "skills/spec/SKILL.md" => extract_markdown_section(@spec_skill, "## Canonical Readiness Vocabulary", end_heading: /^##\s+/), From 04ca1ee757b1acde5e8c54d201a3d398aee5c3d7 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 26 Jul 2026 14:23:30 +0900 Subject: [PATCH 4/5] Harden autonomous merge input validation --- bin/agent-workflow-seam-doctor | 3 +- bin/agent-workflow-seam-doctor-test.rb | 77 ++++++++++- .../pr-batch/bin/autonomous-merge-calibrate | 69 +++++----- .../bin/autonomous-merge-calibrate-test.rb | 121 +++++++++++++++++- 4 files changed, 227 insertions(+), 43 deletions(-) diff --git a/bin/agent-workflow-seam-doctor b/bin/agent-workflow-seam-doctor index e037ff1b..c454cb38 100755 --- a/bin/agent-workflow-seam-doctor +++ b/bin/agent-workflow-seam-doctor @@ -160,6 +160,7 @@ module AgentWorkflowSeamDoctor ) [^>\n]*> }ix + FULL_STRING_ANGLE_PLACEHOLDER = /\A<[^<>\n]*[^\s<>\n][^<>\n]*>\z/ CI_PARITY_KEYWORDS = /(?:CI\ parity\ (?:environment|command)|parity\ environment|runner\ image|reproduction\ guide)/ CI_PARITY_SEAM_PLACEHOLDER = /<(?=[^>\n]*#{CI_PARITY_KEYWORDS.source}\b(?!\s*(?:[[:alnum:]_-]+\s*)*:\s*[^>\s]))[^>\n]*>/i CI_PARITY_EXECUTABLE_PLACEHOLDER = /<[^>\n]*#{CI_PARITY_KEYWORDS.source}\b[^>\n]*>/i @@ -1768,7 +1769,7 @@ module AgentWorkflowSeamDoctor def unresolved_template_value?(value) stripped = value.strip - stripped.empty? || stripped.match?(SEAM_PLACEHOLDER) || stripped.match?(CI_PARITY_SEAM_PLACEHOLDER) + stripped.empty? || stripped.match?(FULL_STRING_ANGLE_PLACEHOLDER) end def shared_markdown_paths(root, shared_roots: []) diff --git a/bin/agent-workflow-seam-doctor-test.rb b/bin/agent-workflow-seam-doctor-test.rb index 5a73e9e3..1040141a 100755 --- a/bin/agent-workflow-seam-doctor-test.rb +++ b/bin/agent-workflow-seam-doctor-test.rb @@ -450,16 +450,63 @@ def test_optional_autonomous_merge_policy_accepts_runtime_valid_empty_arrays def test_optional_autonomous_merge_policy_rejects_nested_unresolved_placeholders variants = { - "direct list scalar" => { + "legacy direct list scalar" => { "policy_paths" => [""] }, - "nested mapping list scalar" => { + "legacy nested mapping list scalar" => { "safe_path_groups" => { "documentation" => { "include" => ["docs/**"], "exclude" => [""] } } + }, + "threshold relaxation rationale" => { + "thresholds" => { "max_changed_files" => 30 }, + "threshold_relaxation" => { + "rationale" => "" + } + }, + "policy path" => { + "policy_paths" => [""] + }, + "generated path" => { + "generated_paths" => [""] + }, + "human-review pattern" => { + "human_review_paths" => [ + { + "id" => "repo-owned-risk", + "pattern" => "", + "reason" => "hot-path" + } + ] + }, + "human-review other detail" => { + "human_review_paths" => [ + { + "id" => "repo-owned-risk", + "pattern" => "app/**", + "reason" => "other", + "detail" => "" + } + ] + }, + "safe-path include" => { + "safe_path_groups" => { + "documentation" => { + "include" => [""], + "exclude" => [] + } + } + }, + "safe-path exclude" => { + "safe_path_groups" => { + "documentation" => { + "include" => ["docs/**"], + "exclude" => [""] + } + } } } results = variants.transform_values do |autonomous_merge| @@ -480,6 +527,32 @@ def test_optional_autonomous_merge_policy_rejects_nested_unresolved_placeholders assert_empty failures, failures.join("\n") end + def test_optional_autonomous_merge_policy_allows_angle_bracket_text_inside_ordinary_prose + rationales = [ + "Document after calibration.", + "Document as historical context after calibration." + ] + rationales.each do |rationale| + with_repo do |root| + write_valid_binstub_contract(root) + write_policy( + root, + POLICY.merge( + "autonomous_merge" => { + "thresholds" => { "max_changed_files" => 30 }, + "threshold_relaxation" => { "rationale" => rationale } + } + ) + ) + write_skill(root, "No commands here.\n") + + out, status = run_doctor(root) + + assert status.success?, out + end + end + end + def test_empty_collections_outside_autonomous_merge_remain_unresolved [[], {}].each do |empty_collection| with_repo do |root| diff --git a/skills/pr-batch/bin/autonomous-merge-calibrate b/skills/pr-batch/bin/autonomous-merge-calibrate index 35cdd5b8..70bb2918 100755 --- a/skills/pr-batch/bin/autonomous-merge-calibrate +++ b/skills/pr-batch/bin/autonomous-merge-calibrate @@ -97,6 +97,30 @@ def validate_calibration_pr_identity!(pull_request) [repository, pull_request.fetch("number")] end +def validate_calibration_pr!(pull_request, scope_complete:) + identity = validate_calibration_pr_identity!(pull_request) + merged_at = parse_calibration_pr_merged_at!(pull_request) + PR_COUNT_FIELDS.each do |field| + value = pull_request[field] + unless value.nil? || (value.is_a?(Integer) && value >= 0) + abort "calibration PR #{field} must be a nonnegative integer or null" + end + end + if scope_complete + COMPLETE_SCOPE_REQUIRED_PR_COUNTS.each do |field| + unless pull_request[field].is_a?(Integer) + abort "calibration PR #{field} must be a nonnegative integer when dataset scope is complete" + end + end + end + categories = pull_request["path_categories"] + unless categories.is_a?(Array) && + categories.all? { |category| category.is_a?(String) && !category.strip.empty? } + abort "calibration PR path_categories must be a list of nonempty strings" + end + [identity, merged_at] +end + def requested_window_covered?(scope, options) return true unless options[:since] || options[:pr_count] @@ -129,21 +153,21 @@ end scope = dataset.fetch("scope") prs = dataset.fetch("prs") abort "calibration PRs must be a list" unless prs.is_a?(Array) +declared_scope_complete = scope["complete"] == true window_covered = requested_window_covered?(scope, options) -effective_scope_complete = scope["complete"] == true && window_covered +effective_scope_complete = declared_scope_complete && window_covered -repositories = options[:repositories].empty? ? scope.fetch("repositories") : options[:repositories] -prs.each { |pr| validate_calibration_pr_repository!(pr) } -prs = prs.select { |pr| repositories.include?(pr.fetch("repository")) } -candidate_identities = {} +raw_identities = {} merged_at_instants = {}.compare_by_identity prs.each do |pr| - identity = validate_calibration_pr_identity!(pr) - abort "calibration PR identities must be unique" if candidate_identities.key?(identity) + identity, merged_at = validate_calibration_pr!(pr, scope_complete: declared_scope_complete) + abort "calibration PR identities must be unique" if raw_identities.key?(identity) - candidate_identities[identity] = true - merged_at_instants[pr] = parse_calibration_pr_merged_at!(pr) + raw_identities[identity] = true + merged_at_instants[pr] = merged_at end +repositories = options[:repositories].empty? ? scope.fetch("repositories") : options[:repositories] +prs = prs.select { |pr| repositories.include?(pr.fetch("repository")) } if options[:since] prs = prs.select do |pr| merged_at_instants.fetch(pr).to_date >= options[:since] @@ -154,33 +178,6 @@ if options[:pr_count] repo_prs.sort_by { |pr| merged_at_instants.fetch(pr) }.reverse.first(options[:pr_count]) end end - -def validate_calibration_pr!(pull_request, scope_complete:) - validate_calibration_pr_identity!(pull_request) - parse_calibration_pr_merged_at!(pull_request) - PR_COUNT_FIELDS.each do |field| - value = pull_request[field] - unless value.nil? || (value.is_a?(Integer) && value >= 0) - abort "calibration PR #{field} must be a nonnegative integer or null" - end - end - if scope_complete - COMPLETE_SCOPE_REQUIRED_PR_COUNTS.each do |field| - unless pull_request[field].is_a?(Integer) - abort "calibration PR #{field} must be a nonnegative integer when dataset scope is complete" - end - end - end - categories = pull_request["path_categories"] - unless categories.is_a?(Array) && - categories.all? { |category| category.is_a?(String) && !category.strip.empty? } - abort "calibration PR path_categories must be a list of nonempty strings" - end -end - -prs.each do |pr| - validate_calibration_pr!(pr, scope_complete: effective_scope_complete) -end prs.sort_by! { |pr| [pr.fetch("repository"), pr.fetch("number")] } def threshold_triggered?(pull_request, maxima) diff --git a/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb b/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb index 013f09ef..27241b86 100755 --- a/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb +++ b/skills/pr-batch/bin/autonomous-merge-calibrate-test.rb @@ -441,7 +441,7 @@ def test_every_pr_repository_shape_is_validated_before_repository_filtering assert_empty failures, failures.join("\n") end - def test_filtered_malformed_metrics_do_not_contaminate_zero_and_null_boundaries + def test_valid_filtered_metrics_do_not_contaminate_zero_and_null_boundaries selected = pr( "example/one", 1, files: 0, lines: 0, commits: 0, reviewed_heads: nil, @@ -450,16 +450,19 @@ def test_filtered_malformed_metrics_do_not_contaminate_zero_and_null_boundaries selected["merged_at"] = "2026-07-03T00:00:00Z" excluded_by_repository = pr( "example/two", 2, - files: "30", lines: -1, commits: 1, reviewed_heads: -1 + files: 30, lines: 1_000, commits: 10, reviewed_heads: 4, + path_categories: ["db"] ) excluded_by_date = pr( "example/one", 3, - files: "30", lines: -1, commits: 1, reviewed_heads: -1 + files: 40, lines: 2_000, commits: 11, reviewed_heads: 5, + path_categories: ["docs"] ) excluded_by_date["merged_at"] = "2026-06-30T00:00:00Z" excluded_by_count = pr( "example/one", 4, - files: "30", lines: -1, commits: 1, reviewed_heads: -1 + files: 50, lines: 3_000, commits: 12, reviewed_heads: 6, + path_categories: ["app"] ) excluded_by_count["merged_at"] = "2026-07-02T00:00:00Z" dataset = calibration_dataset( @@ -484,6 +487,116 @@ def test_filtered_malformed_metrics_do_not_contaminate_zero_and_null_boundaries assert_equal 0, report.dig("coverage", "reviewed_heads_known") end + def test_malformed_records_fail_before_repository_date_and_count_filters + selected = pr( + "example/one", 1, + files: 1, lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["app"] + ) + selected["merged_at"] = "2026-07-03T00:00:00Z" + excluded_by_repository = pr( + "example/two", 2, + files: "30", lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["docs"] + ) + excluded_by_date = pr( + "example/one", 3, + files: 1, lines: 1, commits: 1, reviewed_heads: 1, + path_categories: [""] + ) + excluded_by_date["merged_at"] = "2026-06-30T00:00:00Z" + excluded_by_count = pr( + "example/one", 4, + files: 1, lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["docs"] + ) + excluded_by_count["automation_reviewed_heads"] = -1 + excluded_by_count["merged_at"] = "2026-07-02T00:00:00Z" + cases = { + "repository filter" => [excluded_by_repository, ["--repo", "example/one"], + "calibration PR changed_files must be a nonnegative integer or null"], + "date filter" => [excluded_by_date, ["--since", "2026-07-01"], + "calibration PR path_categories must be a list of nonempty strings"], + "PR-count filter" => [excluded_by_count, ["--pr-count", "1"], + "calibration PR automation_reviewed_heads must be a nonnegative integer or null"] + } + + results = cases.transform_values do |malformed, arguments, expected_error| + repositories = ["example/one", malformed.fetch("repository")].uniq + stdout, stderr, status = run_calibrator( + calibration_dataset([selected, malformed], repositories:), + *arguments + ) + [stdout, stderr, status, expected_error] + end + failures = results.filter_map do |label, (stdout, stderr, status, expected_error)| + next unless status.success? || !stdout.empty? || !stderr.include?(expected_error) + + "#{label}: status=#{status.exitstatus}, stdout=#{stdout.inspect}, stderr=#{stderr.inspect}" + end + + assert_empty failures, failures.join("\n") + end + + def test_hostile_cross_repo_record_cannot_emit_an_enforced_decision + selected = pr( + "example/one", 1, + files: 1, lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["app"] + ) + selected["semantic_inspection"] = "reviewed" + malformed = pr( + "example/two", 2, + files: "30", lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["docs"] + ) + dataset = { + "contract" => "autonomous-merge-calibration-dataset", + "version" => 1, + "scope" => { + "complete" => true, + "window" => "hostile cross-repository fixture", + "repositories" => ["example/one", "example/two"], + "reviewed_heads_decision" => { + "disposition" => "enforced", + "rationale" => "Excluded malformed evidence must never reach enforcement." + } + }, + "prs" => [selected, malformed] + } + + stdout, stderr, status = run_calibrator( + dataset, + "--repo", "example/one", + "--format", "decision" + ) + + refute status.success? + assert_empty stdout + assert_includes stderr, "calibration PR changed_files must be a nonnegative integer" + end + + def test_raw_dataset_identities_must_be_unique_before_repository_filtering + selected = pr( + "example/one", 1, + files: 1, lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["app"] + ) + duplicate = pr( + "example/two", 2, + files: 1, lines: 1, commits: 1, reviewed_heads: 1, + path_categories: ["docs"] + ) + stdout, stderr, status = run_calibrator( + calibration_dataset([selected, duplicate, duplicate.dup], repositories: ["example/one", "example/two"]), + "--repo", "example/one" + ) + + refute status.success? + assert_empty stdout + assert_includes stderr, "calibration PR identities must be unique" + end + def test_pr_count_parses_all_candidate_timestamps_and_sorts_by_actual_instant newer = pr( "example/one", 1, From 2d9275bf760f60aba5dae381562dfc35bd4814b1 Mon Sep 17 00:00:00 2001 From: Justin Gordon Date: Sun, 26 Jul 2026 15:01:38 +0900 Subject: [PATCH 5/5] Restore legacy seam placeholder checks --- bin/agent-workflow-seam-doctor | 29 +++++++++++------- bin/agent-workflow-seam-doctor-test.rb | 42 ++++++++++++++++++++++++++ 2 files changed, 60 insertions(+), 11 deletions(-) diff --git a/bin/agent-workflow-seam-doctor b/bin/agent-workflow-seam-doctor index c454cb38..0d40642a 100755 --- a/bin/agent-workflow-seam-doctor +++ b/bin/agent-workflow-seam-doctor @@ -1660,23 +1660,26 @@ module AgentWorkflowSeamDoctor REQUIRED_POLICY_KEYS.each do |key| if !config.key?(key) issues << "missing policy key: #{key}" - elsif unresolved_policy_value?(config[key]) + elsif unresolved_policy_value?(config[key], placeholder_mode: :legacy) issues << "unresolved policy value for key: #{key}" end end config.each do |key, value| next if REQUIRED_POLICY_KEYS.include?(key) + + autonomous_merge = key == "autonomous_merge" next unless unresolved_policy_value?( value, - empty_collections_unresolved: key != "autonomous_merge" + empty_collections_unresolved: !autonomous_merge, + placeholder_mode: autonomous_merge ? :full_string : :legacy ) issues << "unresolved policy value for key: #{key}" end if config.key?(REPO_PREFIX_POLICY_KEY) && - !unresolved_policy_value?(config[REPO_PREFIX_POLICY_KEY]) && + !unresolved_policy_value?(config[REPO_PREFIX_POLICY_KEY], placeholder_mode: :legacy) && (!config[REPO_PREFIX_POLICY_KEY].is_a?(String) || !config[REPO_PREFIX_POLICY_KEY].match?(REPO_PREFIX_PATTERN)) issues << "invalid policy value for key: #{REPO_PREFIX_POLICY_KEY} " \ @@ -1709,7 +1712,8 @@ module AgentWorkflowSeamDoctor if !intake_policy.key?(key) "missing policy key: #{qualified_key}" elsif !intake_policy[key].is_a?(String) || intake_policy[key].strip.empty? || - unresolved_policy_value?(intake_policy[key]) || intake_policy[key].strip == "n/a" + unresolved_policy_value?(intake_policy[key], placeholder_mode: :legacy) || + intake_policy[key].strip == "n/a" "invalid policy value for key: #{qualified_key}" end end @@ -1723,18 +1727,18 @@ module AgentWorkflowSeamDoctor YAML.safe_load(File.read(path, encoding: "UTF-8"), aliases: false) || {} end - def unresolved_policy_value?(value, empty_collections_unresolved: true) + def unresolved_policy_value?(value, placeholder_mode:, empty_collections_unresolved: true) case value when String - unresolved_template_value?(value) + unresolved_template_value?(value, placeholder_mode:) when Array (empty_collections_unresolved && value.empty?) || value.any? do |entry| - unresolved_policy_value?(entry, empty_collections_unresolved:) + unresolved_policy_value?(entry, empty_collections_unresolved:, placeholder_mode:) end when Hash (empty_collections_unresolved && value.empty?) || value.any? do |key, entry| - unresolved_policy_value?(key.to_s, empty_collections_unresolved:) || - unresolved_policy_value?(entry, empty_collections_unresolved:) + unresolved_policy_value?(key.to_s, empty_collections_unresolved:, placeholder_mode:) || + unresolved_policy_value?(entry, empty_collections_unresolved:, placeholder_mode:) end else value.nil? @@ -1767,9 +1771,12 @@ module AgentWorkflowSeamDoctor nil end - def unresolved_template_value?(value) + def unresolved_template_value?(value, placeholder_mode:) stripped = value.strip - stripped.empty? || stripped.match?(FULL_STRING_ANGLE_PLACEHOLDER) + return true if stripped.empty? + return stripped.match?(FULL_STRING_ANGLE_PLACEHOLDER) if placeholder_mode == :full_string + + stripped.match?(SEAM_PLACEHOLDER) || stripped.match?(CI_PARITY_SEAM_PLACEHOLDER) end def shared_markdown_paths(root, shared_roots: []) diff --git a/bin/agent-workflow-seam-doctor-test.rb b/bin/agent-workflow-seam-doctor-test.rb index 1040141a..fcf8a111 100755 --- a/bin/agent-workflow-seam-doctor-test.rb +++ b/bin/agent-workflow-seam-doctor-test.rb @@ -336,6 +336,48 @@ def test_unresolved_policy_value_fails end end + def test_required_policy_values_reject_embedded_legacy_placeholders + with_repo do |root| + write_valid_binstub_contract(root) + write_policy( + root, + POLICY.merge( + "base_branch" => "Use ", + "ci_parity_environment" => "Run in " + ) + ) + write_skill(root, "No commands here.\n") + + out, status = run_doctor(root) + + refute status.success? + assert_includes out, "unresolved policy value for key: base_branch" + assert_includes out, "unresolved policy value for key: ci_parity_environment" + end + end + + def test_optional_policy_values_recursively_reject_embedded_legacy_placeholders + with_repo do |root| + write_valid_binstub_contract(root) + write_policy( + root, + POLICY.merge( + "custom_scalar" => "Use ", + "custom_array" => ["Run in "], + "custom_mapping" => { "nested" => "Use
" } + ) + ) + write_skill(root, "No commands here.\n") + + out, status = run_doctor(root) + + refute status.success? + %w[custom_scalar custom_array custom_mapping].each do |key| + assert_includes out, "unresolved policy value for key: #{key}" + end + end + end + def test_optional_repo_prefix_accepts_valid_value_and_remains_optional with_repo do |root| write_valid_binstub_contract(root)