os-sdlc: AutocorrectPrepass JSON parse breaks on any rubocop stderr output

jared commented

2026-08-02 16:38:53 +00:00

Owner

AutocorrectPrepass runs REPORT_COMMAND via Open3.capture2e, so stderr (e.g. a cop-crash warning like 'An error occurred while ... cop was inspecting ...') lands in front of the JSON and JSON.parse fails with 'rubocop json report was not parseable'. Found 2026-08-02 when a TautologicalAssertion cop crash (since fixed) broke the residual phase. Fix: capture stdout and stderr separately (capture3), parse stdout only, surface stderr.

jared added the

bug

ready-for-agent

update

labels

2026-08-02 16:38:53 +00:00

jared referenced this issue

2026-08-02 19:53:10 +00:00

os-sdlc test suite writes stray '(string)' artifact into plugin tree #234

jared commented

2026-08-02 19:53:15 +00:00

Author

Owner

Triage: confirmed bug, fully specified. Agent Brief -- Category: bug/update. Summary: AutocorrectPrepass parses rubocop output via Open3.capture2e, mixing stderr into the JSON stream; any stderr (e.g. a cop-crash warning) breaks JSON.parse. Current: capture2e interleaves stdout/stderr. Desired: use capture3 to separate streams, parse stdout only, surface stderr separately (e.g. logged/warned). Acceptance: a rubocop run producing stderr output no longer breaks the residual phase's JSON parsing. Out of scope: the underlying cop crash itself (already fixed). Labels: bug, ready-for-agent, update.

jared commented

2026-08-02 20:12:22 +00:00

Author

Owner

Salvaged from the retired sdlc-229 worktree: two uncommitted red tests for autocorrect_prepass_test.rb that are acceptance tests for THIS issue — both fail today because capture2e mixes rubocop stderr into the JSON parse ("Configuration file not found" leaks into stdout). Apply with git apply when implementing. Patch:

diff --git a/plugins/os-sdlc/tests/autocorrect_prepass_test.rb b/plugins/os-sdlc/tests/autocorrect_prepass_test.rb
index 8c28025..c158f54 100644
--- a/plugins/os-sdlc/tests/autocorrect_prepass_test.rb
+++ b/plugins/os-sdlc/tests/autocorrect_prepass_test.rb
@@ -83,6 +83,36 @@ class AutocorrectPrepassTest < Minitest::Test
     assert_equal %w[a.rb b.rb], result.residual.map { |entry| entry["file"] }
   end
 
+  # Ticket #229 (a): a project without its own .rubocop.yml must not crash the
+  # pre-pass -- it should fall back to the merged/plugin config chain
+  # (OsSdlc::LintWorklistRubocopConfig), the same way bin/lint-worklist already does,
+  # instead of pointing rubocop at a nonexistent <project_dir>/.rubocop.yml and dying on
+  # "Configuration file not found".
+  def test_call_uses_merged_config_when_project_has_no_rubocop_yml
+    with_tmpdir do |dir|
+      project_dir = File.join(dir, "no_config_project")
+      FileUtils.mkdir_p(project_dir)
+      File.write(File.join(project_dir, "offenses.rb"), "class Offenses\nend\n")
+
+      result = OsSdlc::AutocorrectPrepass.new.call(project_dir: project_dir)
+
+      refute_nil result
+    end
+  end
+
+  # Ticket #229 (b): when rubocop's stdout can't be parsed as JSON (e.g. because rubocop
+  # exited on a config error), the raised HarnessError must include the real stderr
+  # content -- not just a generic "unexpected character" JSON-parse snippet that never
+  # names the actual cause.
+  def test_call_raises_harness_error_that_includes_stderr_naming_the_real_cause
+    stderr_message = "Configuration file not found: /nonexistent/.rubocop.yml (RuboCop::ConfigNotFoundError)"
+    recording_runner = StderrCapturingRunner.new(first: ["", "", 0], second: ["", stderr_message, 2])
+    prepass = OsSdlc::AutocorrectPrepass.new(runner: recording_runner)
+
+    error = assert_raises(OsSdlc::HarnessError) { prepass.call(project_dir: "projects/decision-dice") }
+    assert_includes error.message, stderr_message
+  end
+
   def multi_file_json
     { "files" => [offending_file("a.rb", "Style/Documentation", "Missing top-level class documentation comment."),
                   offending_file("b.rb", "Layout/TrailingWhitespace", "Trailing whitespace detected."),
@@ -107,6 +137,19 @@ class TwoCallRunner
   end
 end
 
+# A hand-rolled fake matching the capture3-shaped runner interface Ticket #229's fix 2
+# calls for: [output, stderr, exit_status] per call, so HarnessError has real stderr
+# content available to surface.
+class StderrCapturingRunner
+  def initialize(first:, second:)
+    @responses = [first, second]
+  end
+
+  def run(_command)
+    @responses.shift
+  end
+end
+
 module OsSdlc
   class CliAutocorrectPrepassTest < Minitest::Test
     include OsSdlcTestHelper

Salvaged from the retired sdlc-229 worktree: two uncommitted red tests for autocorrect_prepass_test.rb that are acceptance tests for THIS issue — both fail today because capture2e mixes rubocop stderr into the JSON parse ("Configuration file not found" leaks into stdout). Apply with `git apply` when implementing. Patch: ```diff diff --git a/plugins/os-sdlc/tests/autocorrect_prepass_test.rb b/plugins/os-sdlc/tests/autocorrect_prepass_test.rb index 8c28025..c158f54 100644 --- a/plugins/os-sdlc/tests/autocorrect_prepass_test.rb +++ b/plugins/os-sdlc/tests/autocorrect_prepass_test.rb @@ -83,6 +83,36 @@ class AutocorrectPrepassTest < Minitest::Test assert_equal %w[a.rb b.rb], result.residual.map { |entry| entry["file"] } end + # Ticket #229 (a): a project without its own .rubocop.yml must not crash the + # pre-pass -- it should fall back to the merged/plugin config chain + # (OsSdlc::LintWorklistRubocopConfig), the same way bin/lint-worklist already does, + # instead of pointing rubocop at a nonexistent <project_dir>/.rubocop.yml and dying on + # "Configuration file not found". + def test_call_uses_merged_config_when_project_has_no_rubocop_yml + with_tmpdir do |dir| + project_dir = File.join(dir, "no_config_project") + FileUtils.mkdir_p(project_dir) + File.write(File.join(project_dir, "offenses.rb"), "class Offenses\nend\n") + + result = OsSdlc::AutocorrectPrepass.new.call(project_dir: project_dir) + + refute_nil result + end + end + + # Ticket #229 (b): when rubocop's stdout can't be parsed as JSON (e.g. because rubocop + # exited on a config error), the raised HarnessError must include the real stderr + # content -- not just a generic "unexpected character" JSON-parse snippet that never + # names the actual cause. + def test_call_raises_harness_error_that_includes_stderr_naming_the_real_cause + stderr_message = "Configuration file not found: /nonexistent/.rubocop.yml (RuboCop::ConfigNotFoundError)" + recording_runner = StderrCapturingRunner.new(first: ["", "", 0], second: ["", stderr_message, 2]) + prepass = OsSdlc::AutocorrectPrepass.new(runner: recording_runner) + + error = assert_raises(OsSdlc::HarnessError) { prepass.call(project_dir: "projects/decision-dice") } + assert_includes error.message, stderr_message + end + def multi_file_json { "files" => [offending_file("a.rb", "Style/Documentation", "Missing top-level class documentation comment."), offending_file("b.rb", "Layout/TrailingWhitespace", "Trailing whitespace detected."), @@ -107,6 +137,19 @@ class TwoCallRunner end end +# A hand-rolled fake matching the capture3-shaped runner interface Ticket #229's fix 2 +# calls for: [output, stderr, exit_status] per call, so HarnessError has real stderr +# content available to surface. +class StderrCapturingRunner + def initialize(first:, second:) + @responses = [first, second] + end + + def run(_command) + @responses.shift + end +end + module OsSdlc class CliAutocorrectPrepassTest < Minitest::Test include OsSdlcTestHelper ```

jared referenced this issue

2026-08-02 21:09:24 +00:00

Lint#resolve_config_path has the same missing-.rubocop.yml failure shape fixed in AutocorrectPrepass (#229) #235

jared closed this issue

2026-08-03 11:16:00 +00:00

jared referenced this issue from a commit

2026-08-03 13:52:17 +00:00

Parse rubocop stdout only in AutocorrectPrepass residual pass (#236)

jared referenced this issue from a commit

2026-08-03 13:52:17 +00:00

Record #236 fix in os-sdlc status leaf