Skip to content

Commit 1398546

Browse files
Testclaude
andcommitted
fix(output): activate tool JSON output for codeclimate/sarif via reports config or CLI flag
Pre-3.3.2 setStructuredFormat(true) only fired when --format=codeclimate|sarif. Reports requested via flows.options.reports.<X> or --report-X=PATH left tools emitting human text, and every parser (phpstan, phpcs, phpmd, psalm, parallel-lint — all do json_decode over stdout) returned []. The codeclimate or sarif payload came out empty. Fix detects whether a codeclimate/sarif payload will be produced via any path (primary --format, config reports, CLI report flag) and propagates the flag accordingly. Reuses collectReportTargets() so CLI > config precedence and --no-reports semantics stay consistent. Adds 16-row decision-table unit matrix and 3 release tests against the .phar: phpstan via reports config, phpstan via --report-sarif CLI, and a multi-tool flow (phpcs+phpmd) confirming the bypass affected every tool, not just phpstan. Also includes FEAT-13 (--fast-dirty execution mode) in v3.4 ROADMAP. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent 40b50bc commit 1398546

5 files changed

Lines changed: 429 additions & 3 deletions

File tree

ROADMAP.md

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -398,6 +398,69 @@ Output JSON:
398398

399399
**Score**: 6.5/10. Coste muy bajo (reusa lógica de filterJobForMode, sin ejecución), valor real en monorepos y CI orquestado. Encaja con la línea de "data-driven" de v3.3.
400400

401+
### FEAT-13 · `--fast-dirty` — modo de ejecución sobre el working tree completo
402+
403+
**Casos de uso reales**:
404+
405+
1. **Hooks de IA agentic** (Claude Code, Cursor, Cline, Copilot agent…). El agente toca ficheros y queremos aplicar el **mismo** flow que en precommit, con `--format=json` para ahorrar tokens. Hoy los ficheros no están en stage → `--fast` los pierde, `--fast-branch` analiza demasiado, y la única salida es un script bash de ~10 líneas en cada hook que computa la lista y la pasa por `--files`. El QA debería ser uno: el flow del hook humano es el flow del hook IA.
406+
407+
2. **Dev en revisión intermedia sin commitear**. Patrón estándar de desarrollo iterativo: "voy a revisar lo que llevo antes de commitear". Hoy o haces `git add -A && githooks flow qa --fast` (ensucia el stage) o te escribes el `--files` a mano. Ambas son fricción.
408+
409+
**Hueco que rellena**:
410+
411+
| Modo | Set | Para |
412+
|---|---|---|
413+
| `--fast` | staged | precommit |
414+
| `--fast-branch` | branch diff vs main + staged | CI feature branch |
415+
| `--fast-dirty` | HEAD diff (staged+unstaged) ∪ untracked | hooks IA, revisión intermedia |
416+
| full | paths completos | CI main/release |
417+
418+
**Definición precisa del set**:
419+
420+
```bash
421+
{ git diff --name-only --diff-filter=ACMR HEAD
422+
git ls-files --others --exclude-standard
423+
} | sort -u
424+
```
425+
426+
Tracked modificados vs HEAD (staged o no) ∪ untracked no ignorados.
427+
428+
**Decisiones del diseño**:
429+
430+
| Decisión | Valor |
431+
|---|---|
432+
| Nombre del flag | `--fast-dirty` (idiomático git: "dirty working tree" es término oficial; corto; sin ambigüedad con `--fast` que cubre solo staged) |
433+
| Implementación | `ExecutionContext::getWorktreeDiffFilesLazy()` análogo a `getBranchDiffFilesLazy()`; nuevo variant `ExecutionMode::FAST_DIRTY`; reconocer `fast-dirty` en `EffectiveOptionsResolver` |
434+
| Fallback | Tree limpio → todos los jobs accelerable skip, exit 0. NO cae a full (sería sorprendente y rompería el caso "git status limpio tras commit") |
435+
| Mutuamente excluyente con | `--fast`, `--fast-branch`, `--files`, `--files-from` (todos definen el set) |
436+
| Compatible con | `--exclude-pattern`, `--format`, `--report-json`, `--output`, `--fail-fast`, `--processes`, budgets |
437+
| Untracked | **Incluidos**. Documentar en docs que si hay scratch files en paths cubiertos, usar `.gitignore` o `--exclude-pattern`. Sin código nuevo (`--no-untracked` solo si aparece reporte real) |
438+
| Coexistencia con `--files-from=-` (stdin) | El stdin sigue funcionando, tiene su caso de uso (input arbitrario externo). FEAT-13 cubre el caso "set canónico del working tree" con semántica nombrada y testeada, sin pipa |
439+
440+
**Composición con FEAT-1** (`only-files` / `exclude-files`):
441+
442+
Dos niveles ortogonales — admisión (FEAT-1) y filtrado de input (modo + `paths` del job). FEAT-1 debe leer del set unificado vía `ExecutionContext::getFilteredFiles()`, no hardcodear por modo: si lo hace, FEAT-13 compone gratis.
443+
444+
- **Skip por only-files**: dirty `src/AppB/User.php`, job con `only-files: ['src/AppA/**']``skipped: true, skipReason: "no files match only-files"`.
445+
- **Admisión + filtrado**: dirty `src/AppA/Foo.php` + `src/AppB/Bar.php`, job accelerable con `only-files: ['src/AppA/**']` y `paths: ['src/AppA']` → admite; al binario solo `src/AppA/Foo.php` (intersección con `paths`).
446+
- **Non-accelerable admitido**: dirty `composer.lock`, job non-accelerable con `only-files: ['composer.lock']` → corre la suite entera (FEAT-1 solo admite, no recorta input).
447+
448+
Test matrix de FEAT-1 debe incluir un caso por modo accelerable, no solo `--fast`.
449+
450+
**Tests obligatorios**:
451+
- Unit: cómputo del set (tree limpio, solo unstaged, solo untracked, mezcla, dentro de un git worktree real, repo con submodules).
452+
- Integration: flow run con working tree dirty, jobs accelerable y non-accelerable.
453+
- Mutua exclusión con `--fast`/`--fast-branch`/`--files`/`--files-from`.
454+
455+
**Out of scope**:
456+
- Distinguir tracked dirty vs untracked vía flag (esperar reporte real de "scratch indeseado analizado").
457+
- Modo "fast-vs-upstream" (commits locales no en remote) para pre-push hooks; es un caso distinto, no encaja en este modo.
458+
- Aplicar `--fast-dirty` por defecto a hooks específicos (decisión del usuario en config).
459+
460+
**Score**: 7/10. Coste bajo (~1 día código + tests, patrón ya existe en `getBranchDiffFilesLazy`); dos casos confirmados (uno con viento de cola fuerte: agentic IA en 2026); self-contained; aditivo. No bloquea ni depende de FEAT-1/2/3.
461+
462+
**Por qué v3.4 y no "Pendiente de análisis"**: coste tan bajo que no compite con el resto del bloque en planning, y el caso agentic-IA tiene timing — retrasarlo a v3.5 cede el espacio a otra herramienta.
463+
401464
### Validación de commit messages como tipo nativo (`commit-msg`)
402465

403466
Tipo de job nativo que valida el subject del commit con reglas declarativas (`min-length`, `max-length`, `pattern`, `forbid-trailing-period`, `subject-case`, `forbid-empty`, `merge-allowed`) y preset `conventional-commits`. Se cablea al hook git `commit-msg`. Movido el 2026-04-28: a nivel funcional v3.3 ya cumple el objetivo del usuario (multi-flow + multi-reporte + budgets); commit-msg pasa a "nice-to-have" sin urgencia. **Spec de diseño ya redactada** en `spec/spec-design-commit-message-validation.md` (~620 líneas, 17 AC, 34 REQ): cuando se reabra, la implementación arranca directa de ahí.

app/Commands/Concerns/FormatsOutput.php

Lines changed: 30 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,9 +57,6 @@ private function applyFormat(FlowExecutor $executor, ?FlowPlan $plan = null): vo
5757

5858
if ($isStructured) {
5959
$handler = $this->resolveProgressHandler();
60-
if ($format === 'codeclimate' || $format === 'sarif') {
61-
$executor->setStructuredFormat(true);
62-
}
6360
} elseif (
6461
$plan === null
6562
|| $plan->getOptions()->getProcesses() <= 1
@@ -72,13 +69,43 @@ private function applyFormat(FlowExecutor $executor, ?FlowPlan $plan = null): vo
7269
$handler = new DashboardOutputHandler();
7370
}
7471

72+
if ($this->needsToolJsonOutput($format, $plan)) {
73+
$executor->setStructuredFormat(true);
74+
}
75+
7576
// CI decorator only for text format — structured formats write clean output
7677
if (!$isStructured) {
7778
$handler = $this->wrapWithCIDecorator($handler);
7879
}
7980
$executor->setOutputHandler($handler);
8081
}
8182

83+
/**
84+
* Whether tool-level JSON output (e.g. phpstan `--error-format=json`,
85+
* psalm `--output-format=json`) must be requested at runtime.
86+
*
87+
* Codeclimate and SARIF formatters parse each tool's stdout to extract
88+
* issues. If the tools emit human text, the parsers find no JSON and
89+
* the report comes out empty. The flag must be on whenever a codeclimate
90+
* or sarif payload will be generated, regardless of how it was requested:
91+
*
92+
* - `--format=codeclimate|sarif` (primary output).
93+
* - `reports.codeclimate` / `reports.sarif` in config (multi-report).
94+
* - `--report-codeclimate=PATH` / `--report-sarif=PATH` on CLI.
95+
*
96+
* `collectReportTargets()` already resolves CLI > config precedence and
97+
* honours `--no-reports` (which suppresses config but not CLI flags).
98+
*/
99+
private function needsToolJsonOutput(string $format, ?FlowPlan $plan): bool
100+
{
101+
if ($format === 'codeclimate' || $format === 'sarif') {
102+
return true;
103+
}
104+
$options = $plan !== null ? $plan->getOptions() : null;
105+
$reports = $this->collectReportTargets($options);
106+
return isset($reports['codeclimate']) || isset($reports['sarif']);
107+
}
108+
82109
/**
83110
* Wrap handler with CI decorator if auto-detected and not disabled.
84111
*/

docs/changelog.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,12 @@
22

33
All notable changes to this project are documented here.
44

5+
## [3.3.2]
6+
7+
### Fixed
8+
9+
- Code Climate and SARIF reports requested via `flows.options.reports.codeclimate` / `reports.sarif` in config or via `--report-codeclimate=PATH` / `--report-sarif=PATH` CLI flags came out empty (`[]` / no findings) when the primary `--format` was anything other than `codeclimate` / `sarif`. The flag that asks each tool for JSON output only activated on `--format=codeclimate|sarif`, so every tool ran with its default human-text format and the report parsers (which all do `json_decode()` over stdout) found nothing to extract. Affects every tool with a JSON-dependent parser: PHPStan (`--error-format=json`), PHPCS (`--report=json`), PHPMD (positional `json` format), Psalm (`--output-format=json`) and parallel-lint (`--json`). Fixed: tool-level JSON output is now requested whenever a codeclimate or sarif payload will be produced, regardless of how it was requested.
10+
511
## [3.3.1]
612

713
### Fixed

tests/System/Release/V32FeaturesReleaseTest.php

Lines changed: 151 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -586,6 +586,157 @@ public function conf_check_warns_when_cores_conflicts_with_native_flag()
586586
$this->assertStringContainsString("'cores' overrides 'parallel'", $output);
587587
}
588588

589+
// ========================================================================
590+
// BUG-16 regression: codeclimate/sarif reports requested via reports.<X>
591+
// config or --report-X CLI must trigger tool-level JSON output. Pre-3.3.2,
592+
// structuredFormat activated only on --format=codeclimate|sarif, so phpstan
593+
// ran with text output and the parser produced []. These tests exercise the
594+
// .phar end-to-end with phpstan against a file that has a real error so the
595+
// empty-payload regression cannot slip back into a release.
596+
// ========================================================================
597+
598+
/** @test */
599+
public function bug16_reports_codeclimate_in_config_captures_phpstan_issues_without_format_flag()
600+
{
601+
$srcDir = self::TESTS_PATH . '/src';
602+
@mkdir($srcDir, 0777, true);
603+
$phpstanFile = $this->phpFileBuilder->buildWithErrors([\Tests\Utils\PhpFileBuilder::PHPSTAN]);
604+
file_put_contents("$srcDir/Bad.php", $phpstanFile);
605+
606+
$reportPath = self::TESTS_PATH . '/qa-cc.json';
607+
@unlink($reportPath);
608+
609+
$this->configurationFileBuilder
610+
->setV3GlobalOptions(['reports' => ['codeclimate' => $reportPath]])
611+
->setV3Flows(['qa' => ['jobs' => ['phpstan_src']]])
612+
->setV3Jobs([
613+
'phpstan_src' => ['type' => 'phpstan', 'level' => 0, 'paths' => [$srcDir]],
614+
]);
615+
file_put_contents($this->configPath, $this->configurationFileBuilder->buildV3Php());
616+
617+
// No --format. Pre-fix, this triggered the bug: phpstan ran with text
618+
// output and the codeclimate file was '[]'.
619+
shell_exec("$this->githooks flow qa --config=$this->configPath 2>/dev/null");
620+
621+
$this->assertFileExists($reportPath);
622+
$decoded = json_decode((string) file_get_contents($reportPath), true);
623+
$this->assertIsArray($decoded);
624+
$this->assertNotEmpty(
625+
$decoded,
626+
'codeclimate report must contain phpstan issues; an empty array means structuredFormat was not propagated'
627+
);
628+
629+
$hasPhpstanIssue = false;
630+
foreach ($decoded as $issue) {
631+
if (
632+
($issue['check_name'] ?? '') === 'phpstan'
633+
|| strpos($issue['check_name'] ?? '', '.') !== false
634+
) {
635+
$hasPhpstanIssue = true;
636+
break;
637+
}
638+
}
639+
$this->assertTrue($hasPhpstanIssue, 'codeclimate payload must include at least one phpstan issue');
640+
641+
@unlink($reportPath);
642+
}
643+
644+
/** @test */
645+
public function bug16_reports_codeclimate_in_config_captures_issues_from_all_tool_types()
646+
{
647+
// Same bug, different surface: pre-fix the bypass affected EVERY tool
648+
// whose parser depends on JSON tool output (phpcs, phpmd, phpstan,
649+
// psalm, parallel-lint — all of them). Verifies that activating
650+
// structuredFormat reaches each Job's applyStructuredOutputFormat()
651+
// through the Phar build, not just phpstan.
652+
$srcDir = self::TESTS_PATH . '/src';
653+
@mkdir($srcDir, 0777, true);
654+
// Build a single file with phpcs + phpmd violations.
655+
// (phpstan is tested in the previous case; combining all here keeps
656+
// the assertion focused on "more than one tool reports".)
657+
$multiToolFile = $this->phpFileBuilder->buildWithErrors([
658+
\Tests\Utils\PhpFileBuilder::PHPCS,
659+
\Tests\Utils\PhpFileBuilder::PHPMD,
660+
]);
661+
file_put_contents("$srcDir/Bad.php", $multiToolFile);
662+
663+
$reportPath = self::TESTS_PATH . '/qa-cc.json';
664+
@unlink($reportPath);
665+
666+
$this->configurationFileBuilder
667+
->setV3GlobalOptions(['reports' => ['codeclimate' => $reportPath]])
668+
->setV3Flows(['qa' => ['jobs' => ['phpcs_src', 'phpmd_src']]])
669+
->setV3Jobs([
670+
'phpcs_src' => ['type' => 'phpcs', 'standard' => 'PSR12', 'paths' => [$srcDir]],
671+
'phpmd_src' => ['type' => 'phpmd', 'paths' => [$srcDir]],
672+
]);
673+
file_put_contents($this->configPath, $this->configurationFileBuilder->buildV3Php());
674+
675+
shell_exec("$this->githooks flow qa --config=$this->configPath 2>/dev/null");
676+
677+
$this->assertFileExists($reportPath);
678+
$decoded = json_decode((string) file_get_contents($reportPath), true);
679+
$this->assertIsArray($decoded);
680+
$this->assertNotEmpty(
681+
$decoded,
682+
'codeclimate report must contain issues from at least one tool; empty means structuredFormat was not propagated'
683+
);
684+
685+
// At least one of each tool must have produced an issue: pre-fix every
686+
// tool's stdout was text and json_decode returned null, so neither
687+
// would appear in the payload.
688+
$tools = [];
689+
foreach ($decoded as $issue) {
690+
$check = (string) ($issue['check_name'] ?? '');
691+
if (strpos($check, 'Squiz.') === 0 || strpos($check, 'PSR12.') === 0 || strpos($check, 'Generic.') === 0) {
692+
$tools['phpcs'] = true;
693+
}
694+
// phpmd uses bare rule names like 'ExcessiveParameterList', 'ShortVariable'
695+
if ($check !== '' && strpos($check, '.') === false && $check !== 'phpstan') {
696+
$tools['phpmd'] = true;
697+
}
698+
}
699+
$this->assertArrayHasKey('phpcs', $tools, 'codeclimate payload must include at least one phpcs issue (Squiz/PSR12/Generic.* prefix)');
700+
$this->assertArrayHasKey('phpmd', $tools, 'codeclimate payload must include at least one phpmd issue (bare rule name)');
701+
702+
@unlink($reportPath);
703+
}
704+
705+
/** @test */
706+
public function bug16_cli_report_sarif_captures_phpstan_issues_without_format_flag()
707+
{
708+
$srcDir = self::TESTS_PATH . '/src';
709+
@mkdir($srcDir, 0777, true);
710+
$phpstanFile = $this->phpFileBuilder->buildWithErrors([\Tests\Utils\PhpFileBuilder::PHPSTAN]);
711+
file_put_contents("$srcDir/Bad.php", $phpstanFile);
712+
713+
$reportPath = self::TESTS_PATH . '/qa.sarif';
714+
@unlink($reportPath);
715+
716+
$this->configurationFileBuilder
717+
->setV3Flows(['qa' => ['jobs' => ['phpstan_src']]])
718+
->setV3Jobs([
719+
'phpstan_src' => ['type' => 'phpstan', 'level' => 0, 'paths' => [$srcDir]],
720+
]);
721+
file_put_contents($this->configPath, $this->configurationFileBuilder->buildV3Php());
722+
723+
// CLI flag without --format. Pre-fix, structuredFormat stayed false and
724+
// SARIF.runs[0].results was empty.
725+
shell_exec("$this->githooks flow qa --report-sarif=$reportPath --config=$this->configPath 2>/dev/null");
726+
727+
$this->assertFileExists($reportPath);
728+
$decoded = json_decode((string) file_get_contents($reportPath), true);
729+
$this->assertIsArray($decoded);
730+
$this->assertSame('2.1.0', $decoded['version'] ?? null);
731+
$results = $decoded['runs'][0]['results'] ?? [];
732+
$this->assertNotEmpty(
733+
$results,
734+
'SARIF runs[0].results must contain phpstan findings; empty means structuredFormat was not propagated'
735+
);
736+
737+
@unlink($reportPath);
738+
}
739+
589740
/** @test */
590741
public function dashboard_falls_back_to_streaming_when_stdout_is_not_a_tty()
591742
{

0 commit comments

Comments
 (0)