jared/cc-os

Watch

Fork 0

Guard-map test-reviewer always settles error for missing red_tests #1416

New issue

Closed

opened 2026-10-27 16:02:33 +00:00 by jared · 2 comments

jared commented

2026-10-27 16:02:33 +00:00

Owner

Problem

On the poodr-guard-implementation map, the test-reviewer always settles error. The map makes the new tests green by design, and the mutation check is the red proof. But the test-reviewer brief carries no red_tests and no mutation result. The agent rule says an empty red_tests means error with MISSING: red_tests. So every guard-map run fails at test-reviewer, and the orchestrator must hand-finish it.

Observed

Ticket #1406, implementation 181, rerouted to poodr-guard-implementation at seam-designer. Test-writer dispatch 1575 passed with green tests and a mutation patch. Mutation check dispatch 1577 passed: the patch if completed_at -> if completed_at && false turned a new test red. Test-reviewer dispatch 1579 (handoff 1609) settled:

verdict: error
summary: Review of plugins/os/tests/handoff/document_test.rb cannot settle: red_tests evidence is missing.
MISSING: red_tests

The run ended implementation_failed, reason verdict:error.

Reproduce

n/a: needs a live guard-map run; the evidence is implementation 181 of #1406 (dispatches 1570-1579).

Design

From takedown run docs/takedown/1416-20261027-123627, design revise-2 (the run failed at green-suite on an unrelated test, see #1425; a re-run failed at critique-1 on a pipeline defect).

Approach

Two gaps cause the error. The guard map's test-reviewer enrich omits red_tests, so the brief has no such key. On a red run MutationCheck#verdict returns lines: [], so the mutation step output is empty and Facts#red_tests could not parse anything even if it were enriched.
MutationCheck passes the captured mutated-run output to verdict. On :red it returns SuiteOutput.parse(output).lines as the gate lines, so the failing-test lines reach the step output and stderr. The guard map test-reviewer then enriches red_tests. There is no new fact and no second parser.
A pin test over every production map checks that each test-reviewer node enriches test_changes and red_tests, which are the inputs agents/test-reviewer.md requires.

Required work:

Make MutationCheck return the mutated run's failing-test lines through SuiteOutput.parse on a red result — check: present SuiteOutput.parse in plugins/os-sdlc/lib/os_sdlc/implementation/mutation_check.rb
Add red_tests to the test-reviewer enrich on the guard map — check: present red_tests in plugins/os-sdlc/maps/poodr-guard-implementation.yaml
Remove the empty lines: [] from the red verdict — check: absent lines: [] in plugins/os-sdlc/lib/os_sdlc/implementation/mutation_check.rb

Acceptance criteria

Given the guard map, when the mutation patch turns a new test red, then the mutation-check gate stderr names the failing test (for example FooTest#test_answer) and the status stays 0.
Given every production map, when a node runs the test-reviewer agent, then its enrich lists both test_changes and red_tests.

Survived, broken, unappliable, and no-mutation results are unchanged and need no new criterion: their tests already pass.

Owner sketch

class OsSdlc::Implementation::MutationCheck
  def outcome
    applied = apply_patch
    return applied if applied

    output, status = ProjectShell.capture2e(command, chdir: root)
    verdict(classify(output, status), output)
  end

  def classify(output, status)
    StepOutcome.new(step: "diff-test", exit_status: status.exitstatus, output: output)
               .classify(expect_red: true)
  end

  def verdict(classification, output)
    case classification
    when :red then DispatchCheckResult.new(status: 0, lines: SuiteOutput.parse(output).lines)
    when :unexpected_pass then failure(survived_line)
    else failure(broken_line)
    end
  end
end

In plugins/os-sdlc/maps/poodr-guard-implementation.yaml (test-reviewer node, line 91) the enrich becomes [ticket_brief, red_tests, round_tests, test_changes, objections, retired_tests, change-auditor].

New test, plugins/os-sdlc/tests/map_catalog_test_reviewer_enrich_test.rb:

# frozen_string_literal: true

require_relative "test_helper"

class MapCatalogTestReviewerEnrichTest < Minitest::Test
  REQUIRED = %w[test_changes red_tests].freeze

  def test_every_production_test_reviewer_node_enriches_what_the_agent_requires
    reviewers = production_map_names.flat_map { |name| reviewer_nodes(name) }

    refute_empty reviewers

    gaps = reviewers.filter_map do |map_name, node|
      missing = REQUIRED - node.enrich
      "#{map_name} #{node.name} enrich lacks #{missing.join(", ")}" unless missing.empty?
    end

    assert_empty gaps
  end

  private

  def production_map_names
    Dir.glob(File.join(OsSdlc::MapCatalog::PLUGIN_MAPS_DIR, "*.yaml")).map { |path| File.basename(path, ".yaml") }
  end

  def reviewer_nodes(name)
    map = catalog.load(name)
    map.node_names.map { |node_name| [name, map.node(node_name)] }
       .select { |_name, node| node.agent == "test-reviewer" }
  end

  def catalog
    OsSdlc::MapCatalog.new(OsSdlc::MapCatalog::PLUGIN_MAPS_DIR,
                           drafts_dir: File.join(Dir.tmpdir, "os-sdlc-reviewer-enrich", "drafts"))
  end
end

New test in plugins/os-sdlc/tests/boot_mutation_check_test.rb (class OsSdlc::BootMutationCheckTest):

def test_a_patch_that_breaks_the_guarded_code_reports_the_test_it_turned_red
  @fixture.record_test_writer_handoff(mutation: @fixture.mutation(MutationCheckScenario::BREAKING_PATCH))

  _out, err, status = run_check

  assert_equal 0, status.exitstatus, err
  assert_includes err, "FooTest#test_answer"
end

Origin

Trigger: /os-sdlc:implement 1406, implementation 181 (dispatches 1570-1579)
Improvised this session: orchestrator hand-finished #1406 per the escalation procedure
Chain: run 181 verdict:error ← test-reviewer MISSING: red_tests (handoff 1609) ← guard-map test-reviewer enrich has no red proof (DESIGN, plugins/os-sdlc/maps/poodr-guard-implementation.yaml:91) and the agent rule demands red_tests (DESIGN, plugins/os-sdlc/agents/test-reviewer.md:24)
Root candidate: none found; searched poodr-guard-implementation, guard map, mutation-check, red_tests, test-reviewer error, round_tests
Where: plugins/os-sdlc/maps/poodr-guard-implementation.yaml; plugins/os-sdlc/agents/test-reviewer.md; plugins/os-sdlc/lib/os_sdlc/dispatch/brief/facts.rb
Session: d14200e8-6fb4-4e54-b125-73537f7a28fc
Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/d14200e8-6fb4-4e54-b125-73537f7a28fc.jsonl

VERDICT: CREATE

## Problem On the `poodr-guard-implementation` map, the test-reviewer always settles `error`. The map makes the new tests green by design, and the mutation check is the red proof. But the test-reviewer brief carries no `red_tests` and no mutation result. The agent rule says an empty `red_tests` means `error` with `MISSING: red_tests`. So every guard-map run fails at test-reviewer, and the orchestrator must hand-finish it. ## Observed Ticket #1406, implementation 181, rerouted to `poodr-guard-implementation` at seam-designer. Test-writer dispatch 1575 passed with green tests and a mutation patch. Mutation check dispatch 1577 passed: the patch `if completed_at` -> `if completed_at && false` turned a new test red. Test-reviewer dispatch 1579 (handoff 1609) settled: ``` verdict: error summary: Review of plugins/os/tests/handoff/document_test.rb cannot settle: red_tests evidence is missing. MISSING: red_tests ``` The run ended `implementation_failed`, reason `verdict:error`. ## Reproduce n/a: needs a live guard-map run; the evidence is implementation 181 of #1406 (dispatches 1570-1579). ## Design From takedown run docs/takedown/1416-20261027-123627, design revise-2 (the run failed at green-suite on an unrelated test, see #1425; a re-run failed at critique-1 on a pipeline defect). ## Approach Two gaps cause the error. The guard map's test-reviewer enrich omits `red_tests`, so the brief has no such key. On a red run `MutationCheck#verdict` returns `lines: []`, so the mutation step output is empty and `Facts#red_tests` could not parse anything even if it were enriched. `MutationCheck` passes the captured mutated-run output to `verdict`. On `:red` it returns `SuiteOutput.parse(output).lines` as the gate lines, so the failing-test lines reach the step output and stderr. The guard map test-reviewer then enriches `red_tests`. There is no new fact and no second parser. A pin test over every production map checks that each test-reviewer node enriches `test_changes` and `red_tests`, which are the inputs `agents/test-reviewer.md` requires. Required work: 1. Make `MutationCheck` return the mutated run's failing-test lines through `SuiteOutput.parse` on a red result — check: present `SuiteOutput.parse` in `plugins/os-sdlc/lib/os_sdlc/implementation/mutation_check.rb` 2. Add `red_tests` to the test-reviewer enrich on the guard map — check: present `red_tests` in `plugins/os-sdlc/maps/poodr-guard-implementation.yaml` 3. Remove the empty `lines: []` from the red verdict — check: absent `lines: []` in `plugins/os-sdlc/lib/os_sdlc/implementation/mutation_check.rb` ## Acceptance criteria 1. Given the guard map, when the mutation patch turns a new test red, then the mutation-check gate stderr names the failing test (for example `FooTest#test_answer`) and the status stays 0. 2. Given every production map, when a node runs the `test-reviewer` agent, then its enrich lists both `test_changes` and `red_tests`. Survived, broken, unappliable, and no-mutation results are unchanged and need no new criterion: their tests already pass. ## Owner sketch ```ruby class OsSdlc::Implementation::MutationCheck def outcome applied = apply_patch return applied if applied output, status = ProjectShell.capture2e(command, chdir: root) verdict(classify(output, status), output) end def classify(output, status) StepOutcome.new(step: "diff-test", exit_status: status.exitstatus, output: output) .classify(expect_red: true) end def verdict(classification, output) case classification when :red then DispatchCheckResult.new(status: 0, lines: SuiteOutput.parse(output).lines) when :unexpected_pass then failure(survived_line) else failure(broken_line) end end end ``` In `plugins/os-sdlc/maps/poodr-guard-implementation.yaml` (test-reviewer node, line 91) the enrich becomes `[ticket_brief, red_tests, round_tests, test_changes, objections, retired_tests, change-auditor]`. New test, `plugins/os-sdlc/tests/map_catalog_test_reviewer_enrich_test.rb`: ```ruby # frozen_string_literal: true require_relative "test_helper" class MapCatalogTestReviewerEnrichTest < Minitest::Test REQUIRED = %w[test_changes red_tests].freeze def test_every_production_test_reviewer_node_enriches_what_the_agent_requires reviewers = production_map_names.flat_map { |name| reviewer_nodes(name) } refute_empty reviewers gaps = reviewers.filter_map do |map_name, node| missing = REQUIRED - node.enrich "#{map_name} #{node.name} enrich lacks #{missing.join(", ")}" unless missing.empty? end assert_empty gaps end private def production_map_names Dir.glob(File.join(OsSdlc::MapCatalog::PLUGIN_MAPS_DIR, "*.yaml")).map { |path| File.basename(path, ".yaml") } end def reviewer_nodes(name) map = catalog.load(name) map.node_names.map { |node_name| [name, map.node(node_name)] } .select { |_name, node| node.agent == "test-reviewer" } end def catalog OsSdlc::MapCatalog.new(OsSdlc::MapCatalog::PLUGIN_MAPS_DIR, drafts_dir: File.join(Dir.tmpdir, "os-sdlc-reviewer-enrich", "drafts")) end end ``` New test in `plugins/os-sdlc/tests/boot_mutation_check_test.rb` (class `OsSdlc::BootMutationCheckTest`): ```ruby def test_a_patch_that_breaks_the_guarded_code_reports_the_test_it_turned_red @fixture.record_test_writer_handoff(mutation: @fixture.mutation(MutationCheckScenario::BREAKING_PATCH)) _out, err, status = run_check assert_equal 0, status.exitstatus, err assert_includes err, "FooTest#test_answer" end ``` ## Origin - Trigger: /os-sdlc:implement 1406, implementation 181 (dispatches 1570-1579) - Improvised this session: orchestrator hand-finished #1406 per the escalation procedure - Chain: run 181 `verdict:error` ← test-reviewer `MISSING: red_tests` (handoff 1609) ← guard-map test-reviewer enrich has no red proof (DESIGN, plugins/os-sdlc/maps/poodr-guard-implementation.yaml:91) and the agent rule demands `red_tests` (DESIGN, plugins/os-sdlc/agents/test-reviewer.md:24) - Root candidate: none found; searched poodr-guard-implementation, guard map, mutation-check, red_tests, test-reviewer error, round_tests - Where: plugins/os-sdlc/maps/poodr-guard-implementation.yaml; plugins/os-sdlc/agents/test-reviewer.md; plugins/os-sdlc/lib/os_sdlc/dispatch/brief/facts.rb - Session: d14200e8-6fb4-4e54-b125-73537f7a28fc - Transcript: /home/jared/.claude/projects/-home-jared-dev-cc-os/d14200e8-6fb4-4e54-b125-73537f7a28fc.jsonl VERDICT: CREATE

jared added the

plugin/os-sdlc

filed-by/agent

category/code-change

labels

2026-10-27 16:02:33 +00:00

jared referenced this issue

2026-10-27 16:02:41 +00:00

Test Handoff::Document from strings, keep one CLI test per path #1406

jared referenced this issue

2026-10-27 16:52:51 +00:00

Run CRC bins in process without a fork (InProcessBin) #1421

jared referenced this issue

2026-10-27 16:57:01 +00:00

Fold os-me eval/reply_shape/run.rb onto the os-me harness classes #1422

jared commented

2026-10-27 17:01:41 +00:00

Author

Owner

Work started on branch ticket-1416 (os-sdlc pipeline, poodr-ticket-implementation; body from takedown design revise-2).

jared commented

2026-10-27 17:10:55 +00:00

Author

Owner

Resolution

Done: MutationCheck now returns the mutated run's failing-test lines (SuiteOutput.parse) on a red result instead of lines: []; the poodr-guard-implementation test-reviewer node enriches red_tests. A pin test checks every production map's test-reviewer node enriches test_changes and red_tests. Design from takedown run docs/takedown/1416-20261027-123627 revise-2 (that run failed green-suite on an unrelated scratch-repo defect; re-run failed critique-1, #1426).

Evidence: Commit 56a72968 on ticket-1416, implementation 190: 8 dispatches all pass, suite gate exit 0; new tests boot_mutation_check_test.rb and map_catalog_test_reviewer_enrich_test.rb red before, green after.

Follow-ups: #1425 (green-suite output hides failures), #1426 (critique label check) filed; tickets_bin_test history dependence fixed in this session.

## Resolution **Done:** MutationCheck now returns the mutated run's failing-test lines (SuiteOutput.parse) on a red result instead of lines: []; the poodr-guard-implementation test-reviewer node enriches red_tests. A pin test checks every production map's test-reviewer node enriches test_changes and red_tests. Design from takedown run docs/takedown/1416-20261027-123627 revise-2 (that run failed green-suite on an unrelated scratch-repo defect; re-run failed critique-1, #1426). **Evidence:** Commit 56a72968 on ticket-1416, implementation 190: 8 dispatches all pass, suite gate exit 0; new tests boot_mutation_check_test.rb and map_catalog_test_reviewer_enrich_test.rb red before, green after. **Follow-ups:** #1425 (green-suite output hides failures), #1426 (critique label check) filed; tickets_bin_test history dependence fixed in this session.

jared closed this issue

2026-10-27 17:10:55 +00:00

jared referenced this issue from a commit

2026-10-27 17:22:39 +00:00

feat(os-sdlc): Audited POODR boundaries for #1416; MutationCheck red result carries SuiteOutput lines and guard map enriches red_tests; design and contracts hold. (#1416)

jared referenced this issue from a commit

2026-10-27 17:22:39 +00:00

docs(os-sdlc): takedown run records for #1416, #1415 and #1164