test: add failing test for multiple JSON output

[snyk-tim]

Summary

This PR adds a failing test that demonstrates an issue where only the first workflow.Data item is output when multiple items with application/json content type are passed to the output workflow with --json flag.

Question for the team: Is this intended behavior, or should multiple JSON items be output?

Problem

When a workflow returns multiple workflow.Data items (e.g., scanning multiple lock files), only the first JSON result is printed to stdout. All subsequent results are silently dropped.

Expectation

When encountering multiple workflow.Data they should all be output with the correct renderer. In this case with application/json I would expect the output to be JSONL/NDJSON. One line per JSON separated by a \n character.

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[snyk-io]

✅ Snyk checks have passed. No issues have been found so far.

Status	Scanner	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues
✅	Licenses	0	0	0	0	0 issues
✅	Code Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

[bastiandoetsch]

[Critical] This is an ungated failing test — go test ./pkg/local_workflows/ fails here today ("...first-project..." does not contain "second-project"), empirically confirmed. There's no t.Skip, build tag, or other gate, and CI runs this in required gates, so merging it alone turns main red for every subsequent pipeline.

The bug it documents is real and well-diagnosed: HandleContentTypeOther loops per item and useWriterWithOther → PopWritersByMimetype deletes the writer from the map after the first use (writer.go:58-62), so the second application/json item finds no writer and is dropped. Nice reproduction.

Resolution (one required before merge): (a) land the production fix in this same PR so the test goes green on commit (preferred — a failing test should accompany its fix), or (b) t.Skip("<TICKET>: multiple application/json items dropped by PopWritersByMimetype — un-skip when fixed") to keep CI green while preserving the repro.

— AI review

[bastiandoetsch]

[Should Fix] This assertion is a tautology that gives false confidence. outputWorkflowEntryPoint always returns the empty slice declared at output_workflow.go:39 and discards the input/leftover returned by the Handle* calls, so output is []workflow.Data{} regardless of how many items were dropped — it passes in both the buggy and fixed states. It reads as if it verifies 'no data left unprocessed', but the dropped second item is in fact neither written nor returned; this assertion hides that. Drop it, or replace with one that pins a real contract (assert the second item is written, or surfaced as unconsumed — currently it's neither).

— AI review

[bastiandoetsch]

The bug diagnosis here is genuinely good — a precise, deterministic reproduction of a real PopWritersByMimetype defect. The issues are about how it's committed, not the investigation. Beyond the two inline items:

[Suggestion] The Contains("second-project") check demonstrates the symptom but doesn't pin the correct multi-JSON output contract — a fix that concatenates two JSON objects into malformed output would still make it pass. Decide and assert the intended format (JSON array vs NDJSON vs separated objects) so the test drives a correct fix, not just any fix.
[Suggestion] The subtest name ('should output all application/json items...') describes the post-fix green state while the body asserts the failing state with a '(BUG: this fails)' message — self-contradicting once fixed. After the fix, the name is right and the BUG parenthetical/comments (lines 467-469, 491-493, 496) should become a neutral regression-guard description.
[Suggestion] Reference the tracking ticket in the skip reason/comment so there's a breadcrumb to the fix.

— AI review

[basti-snyk]

Should-Fix: This adds a knowingly-failing test with no accompanying fix. Verified by running it: assert.Contains(outputStr, "second-project") fails — writer output is {"project":"first-project",...} only. That fails the parent Test_Output_outputWorkflowEntryPoint and makes go test ./pkg/local_workflows/... exit non-zero. If this branch merges to main on its own it red-lines CI for every consumer of the package until the fix lands.

The bug it demonstrates is real and the root cause is correctly identified: writerMapImpl.PopWritersByMimetype (output_workflow/writer.go) does delete(w.writers, mimeType), so the single JSON writer is consumed by the first item and the second finds none.

Before merge, either land the production fix in the same PR/stack (test turns green), or quarantine with t.Skip("<ticket>: multiple application/json items drop all but the first") referencing the bug, removed by the fix commit. If this PR is deliberately the RED step of a stack whose next commit carries the fix, this is acceptable — flagging so the decision is explicit.

— AI review

[basti-snyk]

Suggestion (non-blocking): assert.Equal(t, []workflow.Data{}, output) is vacuous here — outputWorkflowEntryPoint initializes output := []workflow.Data{} and returns it unchanged (handler results are assigned back into input, never output), so this passes for the same trivial reason in every existing test and cannot detect the multiple-JSON bug. The meaningful signal is the writer content asserted below.

— AI review

[basti-snyk]

ℹ️ Non-blocking AI review summary.

Reviewed origin/main...HEAD with four reviewer agents (semantic, adversarial, security, code-review). The test is technically sound and, verified by running it, accurately demonstrates a real production bug (PopWritersByMimetype deletes the writer after the first item, dropping all subsequent same-mimetype items). No correctness bug in the test's own logic.

Should-Fix

• A permanently-red test committed without its fix will break the pkg/local_workflows suite / main CI if merged alone — see inline comment on the t.Run. Pair with the fix or t.Skip-quarantine until the fix lands.

Suggestions

• The assert.Equal(t, []workflow.Data{}, output) assertion is vacuous — see inline comment.
• The assert.Contains(..., "second-project") guard would also pass if the fix emitted malformed output (e.g. two concatenated JSON objects {...}{...}, which is not valid JSON). If this test doubles as the post-fix regression guard, confirm the intended output shape (JSON array vs NDJSON vs concatenation) and assert framing/order, not just substring presence.
• defer setup.config.Set(OUTPUT_CONFIG_KEY_JSON, nil) (line 473) is dead cleanup — setupTest builds a fresh config per call, so there is no shared state to reset. Harmless.

Security scan: nothing attributable to this test-only diff.

— AI review