diff --git a/docs/design/review-toolchain-adapters.md b/docs/design/review-toolchain-adapters.md index 6b9c90e8f0a..455808e1cec 100644 --- a/docs/design/review-toolchain-adapters.md +++ b/docs/design/review-toolchain-adapters.md @@ -2,11 +2,11 @@ ## Status -Accepted, implemented. This document covers the extraction of the toolchain -adapter boundary: the npm-specific `qwen review build-test` behavior moves -behind an internal contract without changing its command-line interface or -report format. The phase that adds the first second adapter appends its own -section here. +P0 extraction plus P1 Maven implementation. P0 extracted the existing +npm-specific `qwen review build-test` behavior behind an internal toolchain +adapter without changing its command-line interface or report format. P1 adds +the Maven adapter, which widens the report's `toolchain` discriminant to +`"npm" | "maven" | "unsupported"` — see Report semantics below. ## Problem @@ -168,6 +168,39 @@ P0 changes: - `packages/cli/src/commands/review/build-test.test.ts` - Remains the end-to-end compatibility suite for the command facade. +P1 changes: + +- `packages/cli/src/commands/review/build-test.ts` + - Widens the `toolchain` discriminant, registers the Maven adapter, and + fails closed on mixed-root ambiguity. +- `packages/cli/src/commands/review/lib/toolchain.ts` + - Widens the `install` contract's documented semantics to cover Maven's + best-effort `dependency:go-offline` warm-up alongside `npm ci`. +- `packages/cli/src/commands/review/lib/npm-toolchain.ts` + - Names the Maven adapter in the mixed-root selection guard's rationale. +- `packages/cli/src/commands/review/lib/maven-toolchain.ts` + - Owns Maven reactor discovery, changed-file ownership, the scoped + lifecycle run, and the Surefire/Failsafe evidence. +- `packages/cli/src/commands/review/lib/maven-toolchain.test.ts` + - Pins reactor parsing, ownership, classification, and evidence behavior. +- `packages/cli/src/commands/review/lib/disk.ts` + - Shared disk-space preflight used by both adapters. +- `packages/cli/src/commands/review/base-tree.ts` + - Skips Maven merge bases before checkout (root-pom probe, + npm-applicability probe, nested-pom probe). +- `packages/cli/src/commands/review/test-plan.ts` + - Settles Maven command claims and Surefire test-count claims against the + recorded runs. +- `packages/cli/src/commands/review/lib/agent-briefs.ts` + - Agent 7's Maven branch and the fail-closed fallback rules. +- `packages/cli/src/commands/review/test-delta.ts` + - Keeps the base-side rerun grammar npm-only: Maven lifecycle commands + the Maven adapter records are skipped and disclosed, never re-executed + in the base worktree. +- Test pins: `build-test.test.ts`, `base-tree.test.ts`, `test-plan.test.ts`, + `agent-prompt.test.ts`, and `test-delta.test.ts` grow the Maven branches + beside `lib/maven-toolchain.test.ts`. + ## Testing Focused tests must prove: @@ -181,23 +214,324 @@ Focused tests must prove: remains unchanged through `runBuildTest`. 6. The serialized report shape remains unchanged. +P1 adds the Maven oracle set, pinned by `lib/maven-toolchain.test.ts` plus +the Maven branches of the `test-plan`, `base-tree`, `build-test`, +`agent-prompt`, and `test-delta` suites: + +1. Selector safety: a directory name carrying `,`, `:`, `%`, or a leading + `-`/`!` cannot reach a `-pl` selector and widens the run to the full + reactor; any other name passes as a safe bare token when its characters + allow, or is quoted for the platform shell (POSIX single-quote wrap, + win32 `"…"` under the `%`-rejection and filename gates) — never + interpolated bare into a `shell: true` command line when unsafe. +2. Ownership: changed paths map to the nearest ancestor project, skipping + `src/` fixture trees; a changed POM is reactor-wide; documentation (doc + extensions in doc-shaped locations only) and repository metadata are + exempted. A project Maven rejects as absent from the active reactor becomes + the unsupported handoff. +3. One root-cwd wrapper/Maven lifecycle command with `-pl -am`, + preceded by a best-effort `dependency:go-offline` warm-up on its own + deadline; reactor-wide inputs — and a `-pl` selector past the + launch-safe length — disable narrowing. +4. Fresh Surefire/Failsafe evidence: quote-aware, multi-suite parsing; stale + XML ignored; a green exit over fresh failing reports — or over framed + errors Maven did not fail on — is a failure, never a pass. +5. Timeout and spawn death are always infrastructure, never a finding — no + input exception exists for them. Acquisition failures are infrastructure + with the diff-inputs exceptions, never a finding. +6. Downstream consumers: `base-tree` skips Maven bases before checkout, + `test-plan` settles Maven claims against recorded runs, Agent 7's + brief carries the Maven branch, and `test-delta` refuses Maven lifecycle + commands in the npm-only rerun grammar. + Verification commands: ```bash -cd packages/cli && npx vitest run src/commands/review/ +cd packages/cli && npx vitest run src/commands/review/build-test.test.ts src/commands/review/lib/npm-toolchain.test.ts src/commands/review/lib/maven-toolchain.test.ts src/commands/review/test-plan.test.ts src/commands/review/base-tree.test.ts src/commands/review/agent-prompt.test.ts src/commands/review/test-delta.test.ts +npm run build npm run typecheck ``` +## P1: Maven multi-module verification + +P1 is driven by active use in `alibaba/fastjson2` and `alibaba/druid`, not by a +hypothetical future language plugin. Both are root Maven reactors with checked-in +wrappers, shared core modules, downstream extension or starter modules, nested or +profile-activated modules, and broad CI matrices. Maven support is complete only +when it produces useful deterministic evidence for those repository shapes. + +### Reference constraints + +Fastjson2 and Druid establish these requirements: + +- Always run a checked-in wrapper from the resolved reactor root (`./mvnw`, or + `mvnw.cmd` on win32, where `./mvnw` is not runnable). On POSIX a checked-in + `./mvnw` without the executable bit (a `core.fileMode=false` checkout) also + falls back to the system `mvn`, because running it would die with exit 126 + and turn the whole run into an infrastructure handoff that verifies + nothing. A checked-in wrapper that is empty (0 bytes) also falls back to + the system `mvn` on both platforms: it passes the existence and exec-bit + gates, exits 0, and would otherwise certify a build that never started. A + DIRECTORY carrying the wrapper name falls back for a different reason: it + passes the same gates but dies exit 126 on execution on POSIX, and cannot + execute at all on win32 (where the gate is a regular file with non-zero + size, not an exec bit). Druid's older wrapper depends + on the process cwd and fails when invoked by absolute path from another + repository. When no wrapper exists, + use the system `mvn`. +- Module directory and artifactId are not interchangeable. Druid's `core` + directory produces artifactId `druid`; report paths use module directories, + while Maven remains responsible for resolving the selected reactor projects. +- Core changes must exercise Maven's upstream reactor expansion. The selected + command uses `-am`; downstream (`-amd`) expansion selects the whole reactor on + exactly the repositories that motivated P1, and a run that spends its entire + deadline timing out proves nothing, so downstream coverage stays with the + project's CI matrix. P1 does not claim this is a recursively computed + dependency-graph closure. +- Root `pom.xml` and `.mvn/**` affect the whole reactor and disable module + narrowing. Of the two wrapper scripts, only the one this platform executes + does: every wrapper repo ships both `mvnw` and `mvnw.cmd`, and a change + confined to the other platform's wrapper cannot affect this run, so it is + inert for narrowing (the report discloses when it was changed but not + exercised). +- Profile modules must not be treated as unconditionally active. P1 discovers + module ownership from a nearest-ancestor filesystem walk over `pom.xml` + locations (never from `` aggregation lists), and Maven remains the + authority on whether a selected project belongs to the active reactor under + the current JDK and profiles. A rejected selector fails closed and is never + reported as a successful partial verification. +- External smoke runs must not use `clean`. Existing Surefire/Failsafe reports + may be stale, so only XML files created or updated by the current invocation + are evidence. + +### Adapter selection + +P1 still uses root-level `applies(root)` detection and requires exactly one +applicable adapter. A root where both npm and Maven apply fails closed to +`toolchain: "unsupported"`, even when the current diff appears to touch only one +side. P1 does not yet model nested toolchain roots or changed-file ownership +across toolchains. + +This is intentionally conservative. P1 does not aggregate multiple toolchains, +and refusing an ambiguous mixed root is safer than silently validating only the +frontend or only the Java half. + +### Reactor and module ownership + +P1 does not model the Maven reactor. Maven is the authority on which projects +it contains, and this adapter reads that answer back rather than recomputing +it: + +1. Exempt documentation (doc extensions in doc-shaped locations only) and + repository metadata; such paths select nothing. The exemption runs BEFORE + ownership: a README-only or `.github/`-only diff maps to no project. +2. Assign each changed path to the nearest ancestor directory holding a + `pom.xml`, skipping directories strictly beneath a `src/` tree: a POM + there is OFTEN maven-invoker or archetype test data, but a reactor can + also aggregate a real module under a bare `src/` path + (`src/core`). `src/test/` and `src/it/` are the + principled fixture shapes. Fail closed to reactor-wide when the walk + skipped a src-nested POM and either that POM is not one of those fixture + shapes or the walk would collapse to the ROOT project: the skipped POM + may be a real module, `-pl .` compiles only the root, and + `-pl -am` adds only UPSTREAM projects, so a mis-skipped + collapse leaves the changed module untested under a green verdict. A + root collapse with NO skipped POM is trusted: root-owned `src/` changes + narrow to `-pl . -am`. +3. Use repository-relative project paths as the `-pl` selectors, and fail + closed to the full reactor when a directory name cannot be expressed in one + (`,` and `:` change what a selector means to Maven; `%` expands in cmd.exe; + a leading `-` or `!` reads as an option or an exclusion). +4. Treat any changed POM as reactor-wide. A POM is parent config for + everything that aggregates or inherits it, and `-pl -am` would + compile the aggregator and test nothing that changed. +5. Let Maven reject the selector. `Could not find the selected project in the +reactor` is the authoritative answer for a standalone or profile-inactive + project — evaluated against the real effective model, the active profiles, + and the current JDK, and returned before anything is compiled. That + rejection becomes the structured unsupported handoff. + +An earlier revision of this design parsed the POMs directly: literal +`` recursion, CDATA and comment handling, `` `relativePath` +resolution with the artifactId match Maven itself applies, named and deleted +parent files, and an aggregation-plus-inheritance closure over all of it. That +is a second, weaker model of exactly what the next command evaluates for real, +and it was weakest on the shapes that motivated P1: profile-activated modules +in Druid and Flink, where a text-level parse cannot evaluate activation and had +to fail closed. Removing it deleted ~670 lines of adapter source and ~1360 +lines of its tests, and moved the profile-activation answer from an +approximation to Maven's own. + +Parent inheritance, dependencies, optional edges, dependency management, and +reactor ordering remain Maven's job through `-am`, as before. + +### Commands + +P1 performs one lifecycle invocation per verification target to avoid paying for +the reactor twice. When dependency acquisition is enabled (the default), a +best-effort warm-up runs first on its own deadline: + +- Dependency warm-up: `./mvnw --batch-mode --no-transfer-progress [-pl -am] dependency:go-offline -q`. + A review worktree is cold by construction, and without this step the cold + resolve shares the single lifecycle deadline with compilation and the tests. + The warm-up never blocks the lifecycle run: its known gaps resolve inside the + lifecycle command as before, and a partial local repository — unlike a + partial `node_modules` — is content-addressed and resumable. +- Normal verification: `./mvnw --batch-mode --no-transfer-progress [-pl -am] test`. +- Build-only base preparation: `./mvnw --batch-mode --no-transfer-progress [-pl -am] test-compile`. +- When no checked-in wrapper exists, use `mvn` with the same arguments. + +The command always runs with the reactor root as cwd. P1 does not inject project +profiles or `clean`; project rules and CI remain responsible for broader JDK, +OS, profile, integration-test, and packaging matrices. + +The `-pl` selector is capped: a mid-level aggregator change closes over every +aggregation and inheritance descendant, and the comma-joined selector can +approach cmd.exe's 8191-character command-line limit on the large reactors P1 +targets. Past the cap the run widens to the full reactor and discloses it. + +### Report semantics + +`BuildTestReport.toolchain` widens to `"npm" | "maven" | "unsupported"`. +Existing fields are generalized without changing their JSON shape: + +- `affected`: changed Maven module directories, or `.` for a reactor-wide + change. +- `buildSet`: selectors handed to Maven. It does not pretend to enumerate every + project Maven adds through `-am`. +- `widenedWith`: remains npm-specific and is empty for Maven. +- `install`: the Maven warm-up command when dependency acquisition is enabled + (null when it is not). Whatever the warm-up misses still resolves inside the + lifecycle command, whose result is the one the verdicts read. +- `build`: contains the Maven `test-compile` command in build-only mode. +- `test`: contains the Maven `test` command in normal mode. +- `timedOut`, `ok`, and `note`: retain their current cross-toolchain meaning. + +Command results carry five optional classification flags consumed by +`test-plan`: + +- `CommandResult.infrastructure`: the adapter classified the failure as + environmental (Maven/Java or dependency acquisition, an unlaunchable + wrapper), so a Test Plan claim must not be settled against it. +- `CommandResult.swallowedFailure`: the command exited 0 but its output + records failures Maven did not fail on (a fail-never setting, or a + skip-tests setting that suppressed the whole test phase), so a Test Plan + claim must not be ruled reproduced against it. +- `CommandResult.evidenceCapped`: the adapter refused to certify the run + because part of its evidence was never read or cannot corroborate a pass + (fresh reports past the parse cap, reports rejected by the parser, reports + unseen past a truncated sweep, failure-evidence lines dropped by the + output trim's rescue cap, or a `-l`/`--log-file` setting in + `.mvn/maven.config` that redirects the whole build output away from the + stdout the failure scans read) + and a Test Plan claim must not be settled against it. The flag is + exit-code independent: on an exit-0 run it withholds a pass; on a + non-zero exit the exit remains definitive. +- `CommandResult.testsSuppressed`: a skip setting suppressed the entire test + phase (`Tests are skipped.`) — zero tests ran, so count claims must not + adjudicate against the run and a contradiction is worded as suppression, + not recorded failures. +- `CommandResult.neverRan`: the command exited 0 but cannot prove the + toolchain started. With an unmodified launcher that means no fresh reports + and no Maven-framed output (a stub wrapper); a wrapper the diff itself + modified always lands here, because it can print `[INFO]` lines and write + fresh reports itself — nothing about such a run's evidence proves a build + started. A `-q`/`--quiet` setting in `.mvn/maven.config` can also land a + real run here (it strips every framed line). Either way the run verified + nothing, and a Test Plan claim must not be ruled reproduced against it. + +Command results additionally carry `maven` — the lifecycle phase, `-pl` +module set, and `-am` flag the adapter rendered the command from — so +`test-plan` settles scope claims against structured values instead of +parsing the command line back. + +Dependency/plugin resolution failures and unavailable wrapper/runtime are +infrastructure outcomes (the unlaunchable-wrapper guarantee on POSIX only — +win32 wrapper-launch deaths remain attributed to the diff until the +predicate gap noted in Risks is closed), except when the diff changed the +inputs that could +have caused them: dependency-input changes (POMs, `.mvn/**`, the settings or +repository locations `.mvn/maven.config` references, and the wrapper file +this platform executes) suppress the resolution carve-out, and a change to +the executed wrapper — the script OR its `.mvn/wrapper/**` configuration, +which names the distribution the script downloads — suppresses the +launch-failure carve-out, so a PR-caused breakage is filed against the PR, +not the environment. Unframed launch diagnostics (`mvn: command not found`, +JAVA_HOME errors) count only in the output preceding the first Maven-framed +line; once Maven is talking, those words in a test's own stdout cannot +launder a source failure into infrastructure. Timeout and spawn +death are always infrastructure — no input exception exists for them — but +when the interrupted run still produced fresh failing reports, those failures +stay visible as test evidence, and when its captured output ALSO records +Surefire `Tests run:` summaries with non-zero failures, or source or goal +failures a fail-never/fail-at-end setting never exited on, the note +discloses them — none is framed as purely environmental. Compiler and +test failures remain deterministic build/test evidence, and a zero exit that +Maven's own `[ERROR]`/`[FATAL]` framing contradicts (a fail-never setting) +counts as a failure, not a pass. +Classification uses both command output and whether the current invocation +produced fresh Surefire/Failsafe reports; a resolution failure with no fresh +reports is filed as a source defect only when the diff changed the +resolution inputs. + +### Test reports + +Before invoking Maven, record existing Surefire/Failsafe XML paths and mtimes. +After it returns, parse only reports created or updated after the invocation +started. P1 uses a small, purpose-built parser for the root `` +attributes and `` failure/error children; it does not add a general XML +runtime dependency to the CLI package. + +Normalized Maven evidence must retain module-relative identity so two modules +with the same test class cannot be conflated. Fresh report summaries are appended +to the bounded command output for Agent 7 and test-plan consumption; raw stale +reports are ignored. Surefire writes one XML per test class, so clean reports roll +up per project dir and the failing-report and failing-case lines are capped; the +block is appended after the command output is trimmed and carries its own bound. + +### Downstream integration + +P1 updates the existing consumers that otherwise reject or misread Maven: + +- `base-tree` builds only npm merge bases in this release. Its A/B consumer + (`test-delta`) reruns npm test commands, and Agent 7's Maven branch discloses + that base-side Maven attribution is unavailable, so a Maven base build would be + cost without a consumer; lift this gate when Maven delta attribution exists. +- Agent 7 has an explicit Maven branch, describes modules rather than npm + workspaces, and treats wrapper/dependency acquisition failures as + infrastructure. +- `test-plan` recognizes Maven/Surefire test counts and actual Maven command + execution. +- Maven test failures do not enter the npm-only `test-delta` rerun path in P1. + Agent 7 discloses that base A/B attribution was not performed rather than + asking an npm grammar to rerun Maven. +- Deterministic Maven findings continue using `Source: [build]` and + `Source: [test]`; `compose-review` needs no toolchain-specific change. + +Full Maven-aware base test-delta and failure demotion are deferred until their +identity schema can explicitly carry module and report provenance. P1 must not +infer Java test ownership from npm `--workspace` conventions. + +### P1 scope boundaries + +P1 does not implement: + +- Gradle; +- JaCoCo or changed-line coverage; +- Maven mutation testing; +- arbitrary user-selected profiles; +- automatic JDK/OS matrix execution; +- multi-toolchain result aggregation; +- Maven-aware base-side test-delta. + ## Future phases -### A second toolchain +### Gradle -The boundary exists so a second language lands as a registration rather than -another branch in `build-test.ts`. Whichever comes first — Maven, Gradle — -should prefer a checked-in wrapper, take its project model from the build tool -itself rather than re-deriving one from the manifests, select the projects the -diff changed, and parse the JUnit XML the run produced. A build it cannot -model must fail closed to an unsupported handoff, never to a partial green. +A Gradle adapter should prefer `gradlew`, discover projects from Gradle's own +model where possible, select changed projects, execute project-scoped compile +and test tasks, and parse JUnit XML. Dynamic builds that cannot be modeled must +fail closed to an unsupported handoff. ### Coverage artifacts @@ -209,26 +543,44 @@ for a concrete untested behavior, not an automatic Critical threshold. A later orchestration layer may detect multiple verification roots and invoke one adapter per target. It should aggregate evidence while preserving each -command's toolchain, root, module, and infrastructure status. This phase avoids -specifying that before two real adapters demonstrate the common boundary. +command's toolchain, root, module, and infrastructure status. P0 avoids +specifying this before two real adapters demonstrate the common boundary. ## Risks +- **Win32 wrapper-launch deaths are not yet infrastructure:** a broken but + present `mvnw.cmd` produces cmd.exe diagnostics that match none of the + POSIX launch-failure shapes, so on Windows such an environment outage stays + attributed to the diff. The infrastructure guarantee for an unlaunchable + wrapper holds on POSIX only; close the predicate gap before relying on it + on win32. - **Accidental report drift:** protected by the existing `build-test` suite and explicit report-shape assertions. -- **Adapter abstraction without behavior:** this phase is justified only if npm +- **Adapter abstraction without behavior:** P0 is justified only if npm detection and execution move behind the adapter rather than adding an empty interface around unchanged branching. - **Premature generalization:** the contract intentionally excludes coverage, mutation, CI discovery, and multi-toolchain aggregation. -- **False applicability:** the npm adapter applies only when the root +- **False applicability:** P0's npm adapter applies only when the root `package.json` can scope something — workspaces or a root build/test script. A package.json with neither workspaces nor build/test scripts (husky, a lint - config, a script-less docs site) does not apply, so a future adapter can own - such a root alone rather than losing it to a manifest npm cannot scope. + config, a script-less docs site) does not apply, so the Maven adapter owns + such a root alone. npm applies only when a declared `workspaces` field is + fully modeled and resolves to at least one package, or — with no workspaces + declared — the root defines a build/test script (the shape that makes both + adapters apply and deliberately fails closed as a mixed root under the P1 + selection rule). A root whose workspaces gate refuses npm (an unmodeled or + zero-package glob) falls to the Maven adapter ALONE with the mixed-root + disclosure note, not the fail-closed handoff — all finer-grained support + decisions remain in the one execution path whose existing tests already + fail closed to a structured handoff. ## Open questions -Report-schema widening for a second toolchain — the `toolchain` discriminant -and any per-command classification flags — is deferred to the phase that -introduces that behavior, along with multi-toolchain aggregation. +None. P1 settled the report-schema widening it introduced (`toolchain` +discriminant, `CommandResult.infrastructure`, +`CommandResult.swallowedFailure`, `CommandResult.evidenceCapped`, +`CommandResult.testsSuppressed`, `CommandResult.neverRan`, +`CommandResult.rescueOverflow`, `CommandResult.maven`); +multi-toolchain aggregation remains a decision for the phase that introduces +that behavior. diff --git a/packages/cli/src/commands/review/agent-prompt.test.ts b/packages/cli/src/commands/review/agent-prompt.test.ts index 155d8f0a349..6b12e38020b 100644 --- a/packages/cli/src/commands/review/agent-prompt.test.ts +++ b/packages/cli/src/commands/review/agent-prompt.test.ts @@ -2166,8 +2166,30 @@ describe('buildRoleBrief — every agent, not just the territory ones', () => { // send the reader to fix a prompt that is correct. const p = buildRoleBrief(PR_PLAN, '7'); expect(p).not.toContain(PLAN.diffPathAbsolute); - expect(p).toContain('npm run build'); + expect(p).toContain('`toolchain: "maven"`'); + expect(p).toContain('Do not run `test-delta` for Maven in this release'); expect(p).toContain('Source: [build]'); + // The only steering against hand-run full builds — the pattern the same + // paragraph records as timing out 71 times and verifying nothing. The + // prohibition is scoped to what build-test runs: the unsupported + // fallback below it is the sanctioned hand-run path, and the wording + // must not forbid it. + expect(p).toContain( + 'Do **not** substitute hand-written npm or Maven commands ' + + 'for what `build-test` runs — the `toolchain: "unsupported"` fallback ' + + 'below is the only sanctioned hand-run path', + ); + // The unsupported bullet's steering rules: a fail-closed adapter + // result must not be replaced by an ad hoc command, the mixed-root + // note names the Maven half the npm run did not verify (and forbids + // filling it ad hoc), and a CI-named command lifts neither rule. + // Reverting the bullet to the old precedence list left all tests green + // before these pins. + expect(p).toContain( + 'do not replace that fail-closed result with an ad hoc Maven command', + ); + expect(p).toContain('do not run Maven ad hoc to fill the gap'); + expect(p).toContain('does **not** lift the two rules above'); }); it('pins Agent 7 to the PR worktree and hands it the test-efficacy probe', () => { diff --git a/packages/cli/src/commands/review/base-tree.test.ts b/packages/cli/src/commands/review/base-tree.test.ts index 6bb61c7c353..6d811761a10 100644 --- a/packages/cli/src/commands/review/base-tree.test.ts +++ b/packages/cli/src/commands/review/base-tree.test.ts @@ -27,7 +27,12 @@ import { } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { runBaseTree, type BaseTreeReport } from './base-tree.js'; +import type { Argv } from 'yargs'; +import { + baseTreeCommand, + runBaseTree, + type BaseTreeReport, +} from './base-tree.js'; import { baseWorktreePath } from './lib/paths.js'; import type { BuildTestReport } from './build-test.js'; @@ -250,6 +255,594 @@ describe('runBaseTree', () => { expect( existsSync(join(baseWorktreePath(worktree), '.qwen-review-base-ok')), ).toBe(false); + // "Not buildable" is a SETTLED answer too: the failed marker is what + // keeps later shards from re-paying the same cold checkout. + expect( + existsSync(join(baseWorktreePath(worktree), '.qwen-review-base-failed')), + ).toBe(true); + }); + + it('does NOT run a Maven merge-base build nothing could consume', () => { + // A/B attribution reruns npm test commands (test-delta); Agent 7's brief + // says the same for Maven in this release. Commit the pom so the base + // tree selects the Maven adapter, and pin that the build never runs. + writeFileSync(join(repo, 'pom.xml'), ''); + git(repo, 'add', 'pom.xml'); + git(repo, 'commit', '-qam', 'maven base'); + const mavenSha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: mavenSha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + expect(r.note).toContain('not run'); + // The attribution guard is what stops a downstream agent from reading an + // unavailable A/B as something attributable to the PR — pin it like the + // sibling failed-build note does. + expect(r.note).toMatch(/never a finding against the PR/); + // The gate answers from the object store (git cat-file) BEFORE the + // checkout: a large Java reactor never materialises a tree just to + // learn it will not be built. + expect(existsSync(baseWorktreePath(worktree))).toBe(false); + }); + + it('does NOT check out a nested-pom base the Maven gate exists to skip', () => { + // Standalone module poms with no root aggregator miss the root `pom.xml` + // probe, but the base is Maven just the same and cannot be consumed. + mkdirSync(join(repo, 'app'), { recursive: true }); + writeFileSync(join(repo, 'app', 'pom.xml'), ''); + git(repo, 'add', 'app'); + git(repo, 'commit', '-qam', 'nested maven base'); + const nestedSha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: nestedSha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + expect(existsSync(baseWorktreePath(worktree))).toBe(false); + }); + + it('does NOT treat a deep fixture pom as a Maven base', () => { + // Vendored samples, archetype fixtures, and maven-invoker ITs live deeper + // than `/pom.xml`; counting one would permanently — and silently — + // disable A/B attribution for a repo that merely ships one. + const fixture = join(repo, 'src', 'test', 'resources', 'projects', 'it'); + mkdirSync(fixture, { recursive: true }); + writeFileSync(join(fixture, 'pom.xml'), ''); + git(repo, 'add', 'src'); + git(repo, 'commit', '-qam', 'fixture pom'); + const fixtureSha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: fixtureSha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('still detects a nested pom under a git-quoted directory name', () => { + // Non-ASCII names (and names with quotes, tabs, or backslashes) come + // back C-quoted from `ls-tree` under core.quotePath; the probe must + // resolve the raw name anyway, or the base slips past the gate. + mkdirSync(join(repo, 'm\u00f3dulo'), { recursive: true }); + writeFileSync(join(repo, 'm\u00f3dulo', 'pom.xml'), ''); + git(repo, 'add', 'm\u00f3dulo'); + git(repo, 'commit', '-qam', 'non-ascii nested maven base'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + expect(existsSync(baseWorktreePath(worktree))).toBe(false); + }); + + it('does NOT let a husky-only package.json suppress the nested-pom probe', () => { + // A script-less, workspace-less manifest is not an npm project under + // the adapter's applies rule, so a standalone Maven module beside it + // must still be caught before checkout. + mkdirSync(join(repo, 'app'), { recursive: true }); + writeFileSync(join(repo, 'app', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ scripts: { prepare: 'husky' } }), + ); + git(repo, 'add', 'app', 'package.json'); + git(repo, 'commit', '-qam', 'husky + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + }); + + it('suppresses the nested-pom probe for an npm-applicable package.json', () => { + // A build/test script (or workspaces) makes the base npm's to consume; + // the probe stays home and the build decides. + mkdirSync(join(repo, 'app'), { recursive: true }); + writeFileSync(join(repo, 'app', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ scripts: { build: 'tsc' } }), + ); + git(repo, 'add', 'app', 'package.json'); + git(repo, 'commit', '-qam', 'npm + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('does NOT treat an unmodeled workspace glob as npm-applicable', () => { + // `packages/**` scopes nothing the npm adapter can model (applies() + // declines it); suppressing the nested-pom probe for the blob would make + // a standalone-module Maven base pay the cold checkout this gate exists + // to prevent. The fixture pairs the unmodeled glob with a modeled one + // resolving a real package: dropping the conjunct then makes the blob + // npm-applicable and this test red. + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + mkdirSync(join(repo, 'app'), { recursive: true }); + writeFileSync( + join(repo, 'app', 'package.json'), + JSON.stringify({ + name: 'app', + scripts: { build: 'tsc', test: 'vitest' }, + }), + ); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/**', 'app'] }), + ); + git(repo, 'add', 'java', 'app', 'package.json'); + git(repo, 'commit', '-qam', 'unmodeled glob + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + expect(existsSync(baseWorktreePath(worktree))).toBe(false); + }); + + it('does NOT treat a zero-package workspace glob as npm-applicable', () => { + // A modeled glob resolving to NO package at the base scopes nothing + // either — the nested-pom probe must still run. + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/*'] }), + ); + git(repo, 'add', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'empty glob + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + }); + + it('suppresses the nested-pom probe when a modeled glob resolves to a base package', () => { + // The positive control for the two tests above: a modeled glob with at + // least one member package at the base IS npm-applicable, so the probe + // stays home even beside a nested pom, and the build decides. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync( + join(repo, 'packages', 'app', 'package.json'), + JSON.stringify({ name: '@x/app', scripts: { build: 'tsc' } }), + ); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/*'] }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'workspace + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('suppresses the nested-pom probe for OBJECT-form workspaces too', () => { + // npm accepts `{ workspaces: { packages: [...] } }` as well as the + // array form; the gate's blobIsNpmProject must too, or a base declaring + // the object form reads npm-inapplicable beside a nested pom — a false + // Maven handoff that permanently disables A/B attribution there while + // the on-disk twin accepts the same repo. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync( + join(repo, 'packages', 'app', 'package.json'), + JSON.stringify({ name: '@x/app', scripts: { build: 'tsc' } }), + ); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: { packages: ['packages/*'] } }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'object workspaces + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('models ./-prefixed workspace globs like their bare form', () => { + // The on-disk twin strips a leading `./` from each glob; without it + // here, `workspaceDirFor` never matched the expanded dirs and an npm + // base was misclassified as Maven, losing A/B attribution. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync( + join(repo, 'packages', 'app', 'package.json'), + JSON.stringify({ name: '@x/app', scripts: { build: 'tsc' } }), + ); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['./packages/*'] }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'dot-slash workspace + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('models the empty-string workspace glob as the root package, like the disk twin', () => { + // `"workspaces": [""]` names the root itself as a member: on disk the + // manifest probe joins `//package.json` and applies() accepts it. + // The blob twin probed the unresolvable `:/package.json` instead + // and misread the base as Maven-only, permanently disabling A/B there. + mkdirSync(join(repo, 'app'), { recursive: true }); + writeFileSync(join(repo, 'app', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ name: '@x/root', workspaces: [''] }), + ); + git(repo, 'add', 'app', 'package.json'); + git(repo, 'commit', '-qam', 'root-as-member glob + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('suppresses the nested-pom probe for literal workspace members too', () => { + // Every twin-parity branch has a dedicated test except the LITERAL + // member (`workspaces: ["packages/app"]`, no `*`): a future edit + // breaking it would make blobIsNpmProject refuse a literal-workspaces + // merge base, and beside a standalone nested Maven module the probe + // would permanently disable A/B attribution for that repo shape while + // every other base-tree test stayed green. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync( + join(repo, 'packages', 'app', 'package.json'), + JSON.stringify({ name: '@x/app', scripts: { build: 'tsc' } }), + ); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/app'] }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'literal workspace member + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('does NOT count an unreadable member manifest as an npm package', () => { + // applies() requires at least one readable package: a manifest that + // does not parse lands in `skipped` on disk, so counting it on blob + // EXISTENCE alone suppressed the nested-pom probe for a standalone- + // module Maven base. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync(join(repo, 'packages', 'app', 'package.json'), '{oops'); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/*'] }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'broken member manifest + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + }); + + it('does NOT count a member whose manifest parses to no usable name', () => { + // hasUsableManifestAt mirrors readWorkspacePackages' skip rule: a + // manifest without a non-empty string `name` is not a package. A base + // whose only members lack names must stay npm-inapplicable, or the + // nested-pom probe is suppressed for a base the disk side rejects. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync( + join(repo, 'packages', 'app', 'package.json'), + JSON.stringify({ scripts: { build: 'tsc' } }), + ); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/*'] }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'nameless member + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + }); + + it('does NOT count a negation-excluded workspace member as npm-applicable', () => { + // The on-disk twin puts a negated member in `skipped`, not `packages`; + // excluding the ONLY member leaves nothing npm-applicable, so the + // nested-pom probe must still run. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync( + join(repo, 'packages', 'app', 'package.json'), + JSON.stringify({ name: '@x/app', scripts: { build: 'tsc' } }), + ); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/*', '!packages/app'] }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'negated member + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + }); + + it('still counts a workspace when its negation excludes nothing', () => { + // The positive twin: a negation matching no member leaves the real + // member npm-applicable, so the probe stays home and the build decides. + mkdirSync(join(repo, 'packages', 'app'), { recursive: true }); + writeFileSync( + join(repo, 'packages', 'app', 'package.json'), + JSON.stringify({ name: '@x/app', scripts: { build: 'tsc' } }), + ); + mkdirSync(join(repo, 'java'), { recursive: true }); + writeFileSync(join(repo, 'java', 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ workspaces: ['packages/*', '!packages/ghost'] }), + ); + git(repo, 'add', 'packages', 'java', 'package.json'); + git(repo, 'commit', '-qam', 'harmless negation + nested maven'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + }); + + it('does NOT treat a DIRECTORY named pom.xml as a Maven base', () => { + // `git cat-file -e` exits 0 for trees too; the probe must require a + // BLOB, or a directory named pom.xml beside an npm-buildable layout + // misfires the gate and permanently disables A/B for that base. + mkdirSync(join(repo, 'pom.xml'), { recursive: true }); + writeFileSync(join(repo, 'pom.xml', 'inner.txt'), 'not a pom'); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ scripts: { build: 'tsc' } }), + ); + git(repo, 'add', 'pom.xml', 'package.json'); + git(repo, 'commit', '-qam', 'pom.xml dir + npm base'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + expect(r.note).not.toContain('Maven'); + }); + + it('does NOT treat a nested DIRECTORY named pom.xml as a Maven base', () => { + // The nested variant of the same misfire: `app/pom.xml` as a tree + // entry must not fire the nested-pom probe. + mkdirSync(join(repo, 'app', 'pom.xml'), { recursive: true }); + writeFileSync(join(repo, 'app', 'pom.xml', 'inner.txt'), 'not a pom'); + git(repo, 'add', 'app'); + git(repo, 'commit', '-qam', 'nested pom.xml dir base'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(builds).toHaveLength(1); + expect(r.available).toBe(true); + expect(r.note).not.toContain('Maven'); + }); + + it('treats a base carrying BOTH a root pom.xml and an npm package.json as Maven', () => { + // The root-pom branch of the gate is unconditional: the npm half does + // not rescue a Maven root. Symmetrizing the gate to condition the root + // branch on !npmAtBase would let this polyglot base pay the cold + // checkout, and multi-toolchain aggregation (a declared future phase) + // would leave that symmetrized gate as the only defense. + writeFileSync(join(repo, 'pom.xml'), ''); + writeFileSync( + join(repo, 'package.json'), + JSON.stringify({ scripts: { build: 'tsc' } }), + ); + git(repo, 'add', 'pom.xml', 'package.json'); + git(repo, 'commit', '-qam', 'polyglot base'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + expect(r.note).toMatch(/never a finding against the PR/); + }); + + it.skipIf(process.platform === 'win32')( + 'still detects a nested pom under a directory named with a line terminator', + () => { + // A regex `.` cannot span `\n`, and a `\n` in a name is a standard + // core.quotePath escape — the probe parses the NUL-delimited entries + // structurally, or this base slips past the gate. + mkdirSync(join(repo, 'bad\ndir'), { recursive: true }); + writeFileSync(join(repo, 'bad\ndir', 'pom.xml'), ''); + git(repo, 'add', 'bad\ndir'); + git(repo, 'commit', '-qam', 'newline-dir nested maven base'); + const sha = git(repo, 'rev-parse', 'HEAD'); + + const builds: string[] = []; + const r = run({ plan: { mergeBaseSha: sha } }, (w) => { + builds.push(w); + return okBuild; + }); + + expect(r.available).toBe(false); + expect(builds).toEqual([]); + expect(r.note).toContain('Maven'); + }, + ); + + it('is NOT available for a Maven build report the delta machinery cannot consume', () => { + const mavenBuild = { + ok: true, + toolchain: 'maven', + build: [{ command: './mvnw test-compile', exitCode: 0 }], + note: 'built', + } as unknown as BuildTestReport; + + const r = run({}, () => mavenBuild); + + expect(r.available).toBe(false); + expect( + existsSync(join(baseWorktreePath(worktree), '.qwen-review-base-ok')), + ).toBe(false); + // The sibling handoff pin's twin: a later verifier shard must not repay + // the cold checkout plus a full Maven build to relearn the same + // "unavailable". + expect( + existsSync(join(baseWorktreePath(worktree), '.qwen-review-base-failed')), + ).toBe(true); }); it('is NOT available when npm scoped nothing to compile', () => { @@ -302,3 +895,20 @@ describe('runBaseTree', () => { expect(r.note).toMatch(/base worktree could not be created/); }); }); + +describe('the base-tree CLI option contract', () => { + it("says the --install step is the npm toolchain's alone", () => { + // The help text is the reviewer's only window into the flag; without the + // caveat it implies an npm-ci-style install runs for every toolchain, + // and Maven never runs one (it resolves inside the lifecycle command). + const options: Record = {}; + const recorder = { + option: (name: string, spec: { describe?: string }) => { + options[name] = spec; + return recorder; + }, + } as unknown as Argv; + (baseTreeCommand.builder as (y: Argv) => Argv)(recorder); + expect(options['install']?.describe).toContain('npm toolchain only'); + }); +}); diff --git a/packages/cli/src/commands/review/base-tree.ts b/packages/cli/src/commands/review/base-tree.ts index 9b6b798b3d9..2fbb5fb8982 100644 --- a/packages/cli/src/commands/review/base-tree.ts +++ b/packages/cli/src/commands/review/base-tree.ts @@ -60,6 +60,10 @@ import { type SweepResult, } from './lib/worktree.js'; import { runBuildTest, type BuildTestReport } from './build-test.js'; +import { + hasUnmodeledWorkspaceGlob, + workspaceDirFor, +} from './lib/workspaces.js'; export interface BaseTreeReport { /** @@ -106,6 +110,205 @@ function git(cwd: string, ...args: string[]): void { } } +/** + * Every sibling capture in these commands raises Node's 1 MiB spawnSync + * default (`gh.ts`, `git.ts`, `build-test.ts`, `test-delta.ts`): on + * overflow the child returns `error.code: 'ENOBUFS'` with `status: null`, + * which the probes below would swallow into the same answer as 'absent' — + * silently settling a base the probes never answered. + */ +const GIT_PROBE_MAX_BUFFER = 64 * 1024 * 1024; + +/** + * Accepted symlink carve-out for all three git probes below: they are + * symlink-blind where the disk-side twins follow links. `gitBlob` reads a + * symlinked manifest as the link-target PATH text (JSON.parse fails, the + * manifest reads as absent), `gitHasPath` accepts a symlinked `pom.xml` + * blob, and `gitTreeChildDirs` keeps only mode-040000 entries (a symlinked + * workspace member is dropped). Every outcome is conservative — lost A/B + * attribution for a symlink-shaped base, never a wrong verdict — and the + * repository shapes this gate screens for (root reactors, workspace + * monorepos) do not hang their manifests on symlinks. Resolving + * mode-120000 entries (bounded hop count, entry-mode gates) is left out + * until a real base needs it. + */ +function gitHasPath(cwd: string, sha: string, path: string): boolean { + // A BLOB, not mere existence: `cat-file -e` exits 0 for a DIRECTORY + // too, and a dir named `pom.xml` is not a Maven project — reading one + // as such misfires the gate into a false "Maven base" note that + // permanently disables A/B attribution for that base. + const r = spawnSync('git', ['cat-file', '-t', `${sha}:${path}`], { + cwd, + encoding: 'utf8', + maxBuffer: GIT_PROBE_MAX_BUFFER, + }); + return !r.error && r.status === 0 && (r.stdout ?? '').trim() === 'blob'; +} + +function gitBlob(cwd: string, sha: string, path: string): string | null { + const r = spawnSync('git', ['cat-file', 'blob', `${sha}:${path}`], { + cwd, + encoding: 'utf8', + maxBuffer: GIT_PROBE_MAX_BUFFER, + }); + if (r.error || r.status !== 0) return null; + return r.stdout ?? ''; +} + +/** + * The base-tree twin of readWorkspacePackages' manifest gate: a member + * counts only when its manifest parses and has a usable `name` — the disk + * side puts every other manifest in `skipped`, not `packages`, and + * `npmToolchainAdapter.applies` requires at least one package. + */ +function hasUsableManifestAt(cwd: string, sha: string, path: string): boolean { + const blob = gitBlob(cwd, sha, path); + if (blob === null) return false; + try { + const pkg = JSON.parse(blob) as { name?: unknown } | null; + return pkg !== null && typeof pkg.name === 'string' && pkg.name !== ''; + } catch { + return false; + } +} + +/** + * A root package.json the npm adapter would actually apply to — mirroring + * `npmToolchainAdapter.applies` against the BASE tree: MODELED workspace + * globs that resolve to at least one package there, or a root build/test + * script. A husky/lint-config/docs-tooling-only manifest applies to nothing, + * and an unmodeled glob (`packages/**`, `foo-*`) or a zero-package glob + * scopes nothing either, so suppressing the nested-pom probe for any of them + * would let a standalone-module Maven base pay the cold checkout this gate + * exists to prevent. + */ +function blobIsNpmProject(blob: string, cwd: string, sha: string): boolean { + try { + const pkg = JSON.parse(blob) as { + workspaces?: unknown; + scripts?: Record; + }; + const ws = pkg.workspaces; + const globs = ( + Array.isArray(ws) + ? ws + : Array.isArray((ws as { packages?: unknown } | undefined)?.packages) + ? ((ws as { packages: unknown[] }).packages as unknown[]) + : [] + ).filter((g): g is string => typeof g === 'string'); + if (globs.length > 0) { + return ( + !hasUnmodeledWorkspaceGlob(globs) && + workspaceDirsAt(cwd, sha, globs).some( + (dir) => + // A directory a negation excludes is not a workspace — the same + // check readWorkspacePackages applies on disk. + workspaceDirFor(`${dir}/package.json`, globs) === dir && + hasUsableManifestAt( + cwd, + sha, + // The root-as-member shape (`"workspaces": [""]`): the disk + // twin joins `//package.json` and applies; the blob ref + // must drop the leading slash or `:/package.json` is + // unresolvable and the twins diverge. + dir === '' ? 'package.json' : `${dir}/package.json`, + ), + ) + ); + } + return ( + typeof pkg.scripts === 'object' && + pkg.scripts !== null && + ('build' in pkg.scripts || 'test' in pkg.scripts) + ); + } catch { + return false; + } +} + +/** + * The dirs the workspace globs expand to in the tree at `sha` — the base-tree + * twin of readWorkspacePackages' on-disk expansion (negations excluded there, + * as here, by the caller's workspaceDirFor check). + */ +function workspaceDirsAt(cwd: string, sha: string, globs: string[]): string[] { + const dirs = new Set(); + for (const glob of globs) { + if (glob.startsWith('!')) continue; + // Strip a leading `./` exactly as workspaceDirCandidates does on disk. + const g = glob.replace(/^\.\//, '').replace(/\/$/, ''); + if (g.endsWith('/*')) { + const base = g.slice(0, -2); + for (const child of gitTreeChildDirs(cwd, sha, base)) { + dirs.add(base ? `${base}/${child}` : child); + } + } else { + dirs.add(g); + } + } + return [...dirs]; +} + +/** Direct children (mode 040000) of `dir` in the tree at `sha` — all of them + * when `dir` is empty. */ +function gitTreeChildDirs(cwd: string, sha: string, dir: string): string[] { + // `sha:dir` lists the CHILDREN of dir with bare names (a pathspec without + // the colon lists dir itself); `sha:` alone lists the root tree. + const r = spawnSync('git', ['ls-tree', '-z', `${sha}:${dir}`], { + cwd, + encoding: 'utf8', + maxBuffer: GIT_PROBE_MAX_BUFFER, + }); + if (r.error || r.status !== 0) return []; + const dirs: string[] = []; + // `-z` output is NUL-delimited and NEVER C-quoted, so names with non-ASCII + // bytes, quotes, tabs, backslashes — or line terminators — survive. Parse + // structurally, not with a regex: `.` cannot span a line terminator, and a + // `\n` in a name is the standard core.quotePath escape. The first tab ends + // the OID: an object id is hex and contains no tab. + for (const entry of (r.stdout ?? '').split('\0')) { + if (!entry.startsWith('040000 tree ')) continue; + const tab = entry.indexOf('\t'); + if (tab >= 0) dirs.push(entry.slice(tab + 1)); + } + return dirs; +} + +/** + * Nested-pom bases (standalone modules, no root aggregator) miss the root + * `pom.xml` probe but are Maven just the same; when the base carries no + * npm-applicable root `package.json` either, a depth-1 listing settles it + * before checkout. Only a DIRECT child counts: a pom deeper than + * `/pom.xml` is a vendored sample, an archetype fixture, or a + * maven-invoker IT, and counting one would permanently — and silently — + * disable A/B attribution for a repo that merely ships one. + */ +function gitTreeHasNestedPom(cwd: string, sha: string): boolean { + const dirs = gitTreeChildDirs(cwd, sha, ''); + if (dirs.length === 0) return false; + // The batch probe below is line-based: a name carrying a line terminator + // (PR-choosable) would split into two wrong refs, so those names keep + // the per-dir argv probe, which hands the whole path as ONE argument. + if (!dirs.every((dir) => !/[\r\n]/.test(dir))) { + return dirs.some((dir) => gitHasPath(cwd, sha, `${dir}/pom.xml`)); + } + // ONE batched `cat-file` for every root dir, not one spawn each: step 4's + // verifier shards all offer this command, the gate's answer is a pure + // function of baseSha's tree, and the per-dir spawns multiplied into + // N shards x O(D) synchronous processes on the review's critical path. + // `--batch-check` answers a missing path with ` missing` on its own + // line and still exits 0; a DIRECTORY named pom.xml answers `tree` and is + // not a nested pom, exactly as the per-dir blob gate before it. + const r = spawnSync('git', ['cat-file', '--batch-check'], { + cwd, + encoding: 'utf8', + input: dirs.map((dir) => `${sha}:${dir}/pom.xml`).join('\n') + '\n', + maxBuffer: GIT_PROBE_MAX_BUFFER, + }); + if (r.error || r.status !== 0) return false; + return (r.stdout ?? '').split('\n').some((line) => / blob \d+$/.test(line)); +} + export function runBaseTree(args: BaseTreeArgs): BaseTreeReport { const unavailable = (note: string): BaseTreeReport => ({ available: false, @@ -201,6 +404,33 @@ export function runBaseTree(args: BaseTreeArgs): BaseTreeReport { } catch { // No failed-marker: proceed to build. } + // A/B attribution reruns the recorded npm test commands (test-delta); no + // other toolchain has a delta consumer in this release — Agent 7's brief + // says the same for Maven. The gate sits AFTER the marker checks: step 4 + // launches its verifier shards together, and once a tree stands (built or + // failed) every later shard is answered by its marker without re-scanning. + // It still runs before the checkout, so a Maven base never pays for a tree + // that would not be built — including the nested-pom shape (standalone + // modules, no root aggregator), settled by a depth-1 listing when the base + // has no npm-applicable root package.json either. A husky-only manifest + // leaves no consumable npm half, so it must not suppress the probe. + const mavenBaseNote = + `the merge base is a Maven project, and this release's A/B attribution only reruns npm test ` + + 'commands — a base-side Maven build could not be consumed, so it was not run ' + + '(never a finding against the PR)'; + // The root pom decides the gate alone; probing it FIRST spares every + // root-pom Maven base the npm workspace scan the second check pays. + if (gitHasPath(worktree, baseSha, 'pom.xml')) { + return unavailable(mavenBaseNote); + } + const npmAtBase = (() => { + if (!gitHasPath(worktree, baseSha, 'package.json')) return false; + const blob = gitBlob(worktree, baseSha, 'package.json'); + return blob !== null && blobIsNpmProject(blob, worktree, baseSha); + })(); + if (!npmAtBase && gitTreeHasNestedPom(worktree, baseSha)) { + return unavailable(mavenBaseNote); + } // A real mutual-exclusion lock around sweep+add+build, not just the marker. // The reuse fast path covers the AFTER-build window; this covers the build // itself: measured in review, shard B's opening sweep deleted the tree shard @@ -367,7 +597,9 @@ export const baseTreeCommand: CommandModule = { .option('install', { type: 'boolean', default: true, - describe: 'Run `npm ci` first when node_modules is absent', + describe: + 'Run `npm ci` first when node_modules is absent (npm toolchain only; ' + + 'Maven resolves dependencies inside its lifecycle command)', }), handler: (argv) => { const args = argv as unknown as BaseTreeArgs; diff --git a/packages/cli/src/commands/review/build-test.test.ts b/packages/cli/src/commands/review/build-test.test.ts index beeda8cbde7..53739837746 100644 --- a/packages/cli/src/commands/review/build-test.test.ts +++ b/packages/cli/src/commands/review/build-test.test.ts @@ -5,7 +5,13 @@ */ import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest'; -import { mkdtempSync, mkdirSync, writeFileSync, rmSync } from 'node:fs'; +import { + mkdtempSync, + mkdirSync, + writeFileSync, + rmSync, + chmodSync, +} from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { @@ -14,6 +20,7 @@ import { unresolvedWorkspaceDeps, buildRunEnv, } from './build-test.js'; +import { mavenToolchainAdapter } from './lib/maven-toolchain.js'; import { npmToolchainAdapter, unresolvedWorkspaceDeps as toolchainUnresolvedWorkspaceDeps, @@ -134,9 +141,9 @@ describe('runBuildTest', () => { it('treats a package.json with no build role as no npm project at all', () => { // Docs sites, husky, and lint configs put a script-less package.json in - // repos with nothing npm can scope. It must not make npm apply, or such a - // root would claim the selection away from a second adapter that could - // have verified the diff. + // repos with nothing npm can scope. It must not make npm apply — that is + // what used to collide with a root pom.xml and drop the whole repo to + // `unsupported` where the Maven adapter could have verified the diff. // The handoff note is still npm's precise one: the repo IS npm-shaped, // and naming why it cannot be scoped beats a generic "no project here". writeFileSync(join(root, 'package.json'), JSON.stringify({ name: 'r' })); @@ -203,16 +210,101 @@ describe('runBuildTest', () => { ok: true, timedOut: [], note: - 'No supported npm project here to scope. Fall back to the ' + + 'No supported npm or Maven project here to scope. Fall back to the ' + 'build/test precedence in your brief — installing dependencies first — ' + 'and give each command a deadline it can actually meet.', }); }); + it('prefers npm and discloses the Maven half when both apply at the root', () => { + // Running NOTHING at all on this shape shipped a broken JS build + // through review with zero evidence — npm is what a review verified + // here before the Maven adapter existed, so it runs, and the note + // discloses the Maven half a green npm run must not certify. + writeFileSync( + join(root, 'package.json'), + JSON.stringify({ name: 'polyglot', scripts: { build: 'exit 0' } }), + ); + writeFileSync(join(root, 'pom.xml'), ''); + writePlan(['src/a.ts']); + + const rep = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 60, + install: false, + exec: (command) => ({ + command, + exitCode: 0, + seconds: 1, + timedOut: false, + output: '', + }), + }); + + expect(rep.toolchain).toBe('npm'); + expect(rep.build.length).toBeGreaterThan(0); + expect(rep.ok).toBe(true); + expect(rep.note).toContain('Maven also applies'); + expect(rep.note).toContain('NOT verified'); + }); + + it('runs the Maven half when npm concedes a mixed root without executing', () => { + // The mixed-root note says "this run executed the npm toolchain only" + // — a lie when npm's run() concedes WITHOUT executing (a cold + // yarn/pnpm/bun repo — the common case for a review worktree): + // nothing ran, and the Maven half was never attempted. The Maven + // adapter must run instead of certifying nothing; its own mixed-root + // caveat discloses the unscopable npm side. + rmSync(join(root, 'package-lock.json')); + rmSync(join(root, 'node_modules'), { recursive: true, force: true }); + writeFileSync(join(root, 'yarn.lock'), ''); + pkg('.', { + name: 'polyglot', + scripts: { build: 'exit 0', test: 'exit 0' }, + }); + writeFileSync( + join(root, 'pom.xml'), + 'core', + ); + mkdirSync(join(root, 'core'), { recursive: true }); + writeFileSync(join(root, 'core', 'pom.xml'), ''); + writePlan(['core/src/Main.java']); + + const rep = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 60, + install: true, + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return { + command, + exitCode: 0, + seconds: 1, + timedOut: false, + output: '[INFO] BUILD SUCCESS', + }; + }, + }); + + expect(rep.toolchain).toBe('maven'); + expect(rep.ok).toBe(true); + expect(rep.note).not.toContain('executed the npm toolchain only'); + expect(rep.note).toContain('Mixed root: a root package.json exists'); + }); + it('coerces fractional and zero deadlines at the spawn boundary', () => { // spawnSync validates `timeout` as an unsigned integer: a decimal // --timeout used to throw ERR_OUT_OF_RANGE out of the whole call (no // report, no --out file), and --timeout 0 armed no kill timer at all. + // The boundary is in build-test.ts, above every adapter, so the fixture + // stays npm — this case must not move when a toolchain is added. pkg('.', { name: 'r', scripts: { test: 'vitest run' } }); writePlan(['src/a.ts']); @@ -270,6 +362,97 @@ describe('runBuildTest', () => { ).toThrow(/--budget must be a finite number/); }); + it('leaves a Maven repo with a build-less package.json to the Maven adapter', () => { + writeFileSync( + join(root, 'package.json'), + JSON.stringify({ name: 'husky-only', scripts: { prepare: 'husky' } }), + ); + writeFileSync(join(root, 'pom.xml'), ''); + writePlan(['src/Main.java']); + const exec = vi.fn(); + const sentinel = { toolchain: 'maven' } as ReturnType; + const runSpy = vi + .spyOn(mavenToolchainAdapter, 'run') + .mockReturnValue(sentinel); + + const report = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 5, + install: false, + exec, + }); + + expect(report).toBe(sentinel); + expect(runSpy).toHaveBeenCalledOnce(); + runSpy.mockRestore(); + }); + + it('delegates Maven-only repositories through the facade', () => { + writeFileSync(join(root, 'pom.xml'), ''); + writePlan(['src/Main.java']); + const exec = vi.fn(); + const sentinel = { toolchain: 'maven' } as ReturnType; + const runSpy = vi + .spyOn(mavenToolchainAdapter, 'run') + .mockReturnValue(sentinel); + + const report = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 7, + install: false, + buildOnly: true, + exec, + }); + + expect(report).toBe(sentinel); + expect(runSpy).toHaveBeenCalledOnce(); + expect(runSpy).toHaveBeenCalledWith({ + root, + changedFiles: ['src/Main.java'], + timeout: 7, + install: false, + buildOnly: true, + exec, + }); + }); + + it('carries the Maven classification flags and command facts into the recorded CommandResult', () => { + // The adapter's own unit tests cover the classification, but nothing + // pinned that the fields survive into the report shape test-plan + // consumes — vitest transpiles without type-checking, so a renamed or + // dropped field anywhere in between reads as undefined downstream. + writeFileSync(join(root, 'pom.xml'), ''); + writePlan(['src/Main.java']); + const report = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 5, + install: false, + exec: (command: string) => ({ + command, + exitCode: 0, + seconds: 1, + timedOut: false, + output: + '[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:check on project fixture', + }), + }); + + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(false); + const recorded = report.test[0]; + expect(recorded?.swallowedFailure).toBe(true); + expect(recorded?.maven).toEqual({ + lifecycle: 'test', + modules: ['.'], + alsoMake: true, + }); + expect(recorded?.infrastructure).toBeUndefined(); + expect(recorded?.evidenceCapped).toBeUndefined(); + }); + it('reports `unsupported` — not a false "nothing to build" — for an unmodeled glob', () => { // `packages/**` matches real paths that the walker cannot resolve, so a diff // inside it would otherwise yield an empty affected set and a confident green. @@ -300,6 +483,122 @@ describe('runBuildTest', () => { expect(rep.note).not.toContain('no package to build'); }); + it('selects Maven when the npm half uses unmodeled workspace globs', () => { + // The guard exists for exactly this root: npm cannot scope `packages/**`, + // and applying anyway would block Maven selection into the same + // unsupported handoff this test's sibling pins. The fixture pairs the + // unmodeled glob with a modeled one resolving a real package: dropping + // the guard's conjunct then makes npm applicable and flips selection + // to the ambiguous handoff, turning this test red. + writeFileSync( + join(root, 'package.json'), + JSON.stringify({ + name: 'frontend', + workspaces: ['packages/**', 'apps/*'], + }), + ); + mkdirSync(join(root, 'apps/a'), { recursive: true }); + writeFileSync( + join(root, 'apps/a/package.json'), + JSON.stringify({ name: 'a', scripts: { build: 'tsc', test: 'vitest' } }), + ); + writeFileSync(join(root, 'pom.xml'), ''); + writePlan(['src/Main.java']); + const exec = vi.fn(); + const sentinel = { toolchain: 'maven' } as ReturnType; + const runSpy = vi + .spyOn(mavenToolchainAdapter, 'run') + .mockReturnValue(sentinel); + + const report = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 5, + install: false, + exec, + }); + + expect(report).toBe(sentinel); + expect(runSpy).toHaveBeenCalledOnce(); + runSpy.mockRestore(); + }); + + it.skipIf(process.platform === 'win32')( + 'records rescueOverflow from the real executor end to end', + () => { + // Both halves of the rescue-overflow contract are otherwise pinned + // through seams that bypass the real-executor wiring: direct + // trimOutput tests and fixture execs injecting rescueOverflow. This + // drives runBuildTest's OWN run executor (no injected exec) with + // output carrying >40 evidence lines in the omitted middle, so + // deleting the wiring ships red. + writeFileSync(join(root, 'pom.xml'), ''); + writeFileSync( + join(root, 'mvnw'), + [ + '#!/bin/sh', + 'i=0', + 'while [ $i -lt 40 ]; do', + ' echo "[INFO] padding line $i xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx"', + ' i=$((i+1))', + 'done', + 'i=0', + 'while [ $i -lt 60 ]; do', + ' echo "[ERROR] Failed to execute goal org.example:plugin:1:check (check) on project m$i: boom"', + ' i=$((i+1))', + 'done', + 'i=0', + 'while [ $i -lt 120 ]; do', + ' echo "[INFO] tail padding line $i yyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyyy"', + ' i=$((i+1))', + 'done', + 'exit 1', + '', + ].join('\n'), + ); + chmodSync(join(root, 'mvnw'), 0o755); + writePlan(['src/Main.java']); + + const report = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 30, + install: false, + }); + + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(false); + expect(report.test[0]?.rescueOverflow).toBe(true); + expect(report.test[0]?.evidenceCapped).toBe(true); + }, + ); + + it('selects Maven when the npm glob matches zero packages', () => { + writeFileSync( + join(root, 'package.json'), + JSON.stringify({ name: 'frontend', workspaces: ['packages/*'] }), + ); + writeFileSync(join(root, 'pom.xml'), ''); + writePlan(['src/Main.java']); + const exec = vi.fn(); + const sentinel = { toolchain: 'maven' } as ReturnType; + const runSpy = vi + .spyOn(mavenToolchainAdapter, 'run') + .mockReturnValue(sentinel); + + const report = runBuildTest({ + plan: planPath, + worktree: root, + timeout: 5, + install: false, + exec, + }); + + expect(report).toBe(sentinel); + expect(runSpy).toHaveBeenCalledOnce(); + runSpy.mockRestore(); + }); + it('reinstalls when node_modules exists but is INCOMPLETE (no .package-lock.json)', () => { // A partial tree — left by a timed-out install here, or by the agent's own shell // kill one level up — has the directory but not npm's completeness marker. Gating @@ -793,15 +1092,128 @@ describe('runBuildTest', () => { const summary = 'Tests 3 failed | 1132 passed (1135)'; const trimmed = trimOutput( 'head\n' + 'x'.repeat(3000) + `\n${summary}\n` + 'y'.repeat(9000), - ); + ).text; expect(trimmed).toContain(summary); expect(trimmed).toContain('runner summaries kept'); // The colored form a real pipe delivers is rescued too. const colored = `Tests\x1b[2m \x1b[22m\x1b[31m3 failed\x1b[39m | 1132 passed`; + expect( + trimOutput('h\n' + 'x'.repeat(3000) + `\n${colored}\n` + 'y'.repeat(9000)) + .text, + ).toContain(colored); + }); + + it('rescues Maven dependency-failure lines from a trimmed middle', () => { + // Maven infra classification runs on trimmed output; when the error + // summary lands in the omitted middle, the rescue is what keeps a + // network outage classified as infrastructure instead of a Critical. + const line = + '[ERROR] Could not resolve dependencies for project example:core:jar:1'; + const trimmed = trimOutput( + 'head\n' + 'x'.repeat(3000) + `\n${line}\n` + 'y'.repeat(9000), + ).text; + expect(trimmed).toContain(line); + expect(trimmed).toContain('dependency failures'); + // The colored form a `-Dstyle.color=always` reactor delivers — the SGR + // strip is what the predicate runs on, and the rescued line keeps its + // original bytes. + const colored = + '\x1b[1;31m[ERROR]\x1b[m Could not resolve dependencies for project example:core:jar:1'; + expect( + trimOutput('h\n' + 'x'.repeat(3000) + `\n${colored}\n` + 'y'.repeat(9000)) + .text, + ).toContain(colored); + }); + + it('rescues Maven source-failure lines from a trimmed middle', () => { + // The source markers outrank the infra carve-out; one lost to the trim + // would launder a compile failure into infrastructure. + const line = '[ERROR] COMPILATION ERROR :'; + const trimmed = trimOutput( + 'head\n' + 'x'.repeat(3000) + `\n${line}\n` + 'y'.repeat(9000), + ).text; + expect(trimmed).toContain(line); + expect(trimmed).toContain('source failures'); + // The colored form too — losing the SGR strip here would drop the marker + // that keeps a compile failure from laundering into infrastructure. + const colored = '\x1b[1;31m[ERROR]\x1b[m COMPILATION ERROR :'; + expect( + trimOutput('h\n' + 'x'.repeat(3000) + `\n${colored}\n` + 'y'.repeat(9000)) + .text, + ).toContain(colored); + }); + + it('rescues Maven goal-failure lines from a trimmed middle', () => { + // The swallowed-failure check runs on trimmed output; a fail-never + // plugin goal failure lost to the trim would read the run green. + const line = + '[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:check (validate) on project core: You have 1 Checkstyle violation.'; + const trimmed = trimOutput( + 'head\n' + 'x'.repeat(3000) + `\n${line}\n` + 'y'.repeat(9000), + ).text; + expect(trimmed).toContain(line); + expect(trimmed).toContain('goal failures'); + }); + + it('rescues Maven disk-failure lines from a trimmed middle', () => { + // The launch-failure classification runs on trimmed output; an ENOSPC + // line lost to the trim would file a disk failure against the PR (or, + // under fail-never, read the run green). + const line = + '[ERROR] Failed to write target/x.txt: No space left on device'; + const trimmed = trimOutput( + 'head\n' + 'x'.repeat(3000) + `\n${line}\n` + 'y'.repeat(9000), + ).text; + expect(trimmed).toContain(line); + expect(trimmed).toContain('disk failures'); + }); + + it('rescues the Maven skipped-tests marker from a trimmed middle', () => { + // The adapter's testsSuppressed guard reads the skip marker from the + // trimmed output: a large reactor's trailing Reactor Summary pushes every + // `Tests are skipped.` line into the omitted middle, and losing the marker + // certified a run that tested zero. + const line = '[INFO] Tests are skipped.'; + const trimmed = trimOutput( + 'head\n' + 'x'.repeat(3000) + `\n${line}\n` + 'y'.repeat(9000), + ).text; + expect(trimmed).toContain(line); + expect(trimmed).toContain('skipped-test markers'); + // The colored form too — the rescue strips SGR before the predicate and + // keeps the original bytes. + const colored = '\x1b[1;34m[INFO]\x1b[m Tests are skipped.'; + expect( + trimOutput('h\n' + 'x'.repeat(3000) + `\n${colored}\n` + 'y'.repeat(9000)) + .text, + ).toContain(colored); + }); + + it('rescues Surefire stdout summaries from a trimmed middle', () => { + // The adapter's exit-0 stdout cross-check is the ONE defense for + // relocated-`` runs, and it reads these summaries + // from the trimmed output: a large reactor's trailing Reactor Summary + // pushes them into the omitted middle exactly like the skip marker. + for (const framing of ['[INFO]', '[ERROR]']) { + const line = `${framing} Tests run: 5, Failures: 2, Errors: 0, Skipped: 0`; + const trimmed = trimOutput( + 'head\n' + 'x'.repeat(3000) + `\n${line}\n` + 'y'.repeat(9000), + ).text; + expect(trimmed).toContain(line); + } expect( trimOutput( - 'h\n' + 'x'.repeat(3000) + `\n${colored}\n` + 'y'.repeat(9000), - ), + 'head\n' + + 'x'.repeat(3000) + + '\n[INFO] Tests run: 1, Failures: 0, Errors: 0, Skipped: 0\n' + + 'y'.repeat(9000), + ).text, + ).toContain('Surefire stdout summaries'); + // The colored form too — the rescue strips SGR before the predicate. + const colored = + '\x1b[1;31m[ERROR]\x1b[m Tests run: 5, Failures: 2, Errors: 0, Skipped: 0'; + expect( + trimOutput('h\n' + 'x'.repeat(3000) + `\n${colored}\n` + 'y'.repeat(9000)) + .text, ).toContain(colored); }); @@ -815,10 +1227,107 @@ describe('runBuildTest', () => { ) + '\n' + 'y'.repeat(9000); - const trimmed = trimOutput(hostile); + const trimmed = trimOutput(hostile).text; expect(trimmed.length).toBeLessThan(hostile.length / 4); }); + it('discloses when rescued lines outgrow the rescue cap', () => { + // The omission marker claims every category was kept; once the cap + // drops matching lines it must say so instead of asserting the + // impossible. + const input = + 'h'.repeat(2500) + + '\n' + + Array.from({ length: 50 }, (_, i) => `Tests: ${i} failed`).join('\n') + + '\n' + + 't'.repeat(6500); + const trimmed = trimOutput(input).text; + expect(trimmed).toContain( + 'runner summaries kept — first 40 matches kept, failure evidence before benign lines, 10 more omitted', + ); + }); + + it('aligns both cuts to line boundaries so seam-straddling lines survive', () => { + // The cuts used to sit at fixed CHARACTER offsets while every rescue + // and classification predicate is line-anchored: a verdict-critical + // line straddling either cut fragmented into two pieces that matched + // nothing — neither rescued nor disclosed. No hostile input needed. + const headLine = + '[ERROR] Failed to execute goal org.example:plugin:1:check (check) on project core: boom'; + // `headLine` starts at offset 1999, straddling the old 2000-char head + // cut; the newline before it re-roots the head. + const headSeam = + 'head\n' + 'x'.repeat(1993) + '\n' + headLine + '\n' + 'y'.repeat(9000); + expect(trimOutput(headSeam).text).toContain(headLine); + expect(trimOutput(headSeam).evidenceDropped).toBe(false); + + const tailLine = '[ERROR] Tests run: 5, Failures: 2, Errors: 0, Skipped: 0'; + // `tailLine` straddles the old len-6000 tail cut. + const tailSeam = + 'head\n' + 'x'.repeat(3000) + '\n' + tailLine + '\n' + 'y'.repeat(5970); + expect(trimOutput(tailSeam).text).toContain(tailLine); + }); + + it('keeps failure-evidence lines when benign matches outgrow the rescue cap', () => { + // One shared cap in positional order let benign matches (green Surefire + // summaries) exhaust the slots and drop a failing module's summary — + // the adapter then read the trimmed output green over a failing run. + // Evidence takes the slots first. + const evidence = '[ERROR] Tests run: 5, Failures: 2, Errors: 0, Skipped: 0'; + const benign = Array.from( + { length: 50 }, + () => '[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0', + ).join('\n'); + const input = + 'head\n' + + 'x'.repeat(3000) + + `\n${benign}\n${evidence}\n` + + 'y'.repeat(9000); + const trimmed = trimOutput(input); + expect(trimmed.text).toContain(evidence); + expect(trimmed.evidenceDropped).toBe(false); + }); + + it('keeps the skipped-tests marker when benign matches outgrow the rescue cap', () => { + // The marker carries a VERDICT (suppression), not a count: with benign + // matches filling the 40 rescue slots, the benign classification + // dropped every `Tests are skipped.` line from the rescued middle + // while `evidenceDropped` stayed false — and the adapter certified a + // run that tested zero. As evidence, the marker takes a slot ahead of + // benign lines. + const marker = '[INFO] Tests are skipped.'; + const benign = Array.from( + { length: 50 }, + () => '[INFO] Tests run: 3, Failures: 0, Errors: 0, Skipped: 0', + ).join('\n'); + const input = + 'head\n' + + 'x'.repeat(3000) + + `\n${benign}\n${marker}\n` + + 'y'.repeat(9000); + const trimmed = trimOutput(input); + expect(trimmed.text).toContain(marker); + expect(trimmed.evidenceDropped).toBe(false); + }); + + it('fails closed when evidence lines themselves outgrow the rescue cap', () => { + // When the cap drops EVIDENCE lines the trimmed output no longer holds + // the verdict's inputs — the flag the Maven adapter folds into + // evidenceCapped must say so; benign-only overflow stays disclosed text. + const evidence = Array.from( + { length: 45 }, + (_, i) => + `[ERROR] Failed to execute goal org.example:plugin:1:check (check) on project m${i}: boom`, + ).join('\n'); + const input = + 'head\n' + 'x'.repeat(3000) + `\n${evidence}\n` + 'y'.repeat(9000); + const trimmed = trimOutput(input); + expect(trimmed.evidenceDropped).toBe(true); + expect(trimmed.text).toContain( + 'first 40 matches kept, failure evidence before benign lines, 5 more omitted', + ); + }); + it('buildOnly builds the same set but runs NO tests', () => { // For the merge-base tree an A/B probe compares against: base's suite was // green before this PR existed, so running it measures nothing about the diff --git a/packages/cli/src/commands/review/build-test.ts b/packages/cli/src/commands/review/build-test.ts index 2acfe20d029..b216c9b4734 100644 --- a/packages/cli/src/commands/review/build-test.ts +++ b/packages/cli/src/commands/review/build-test.ts @@ -42,6 +42,17 @@ import { spawnSync } from 'node:child_process'; import { existsSync, readFileSync, writeFileSync } from 'node:fs'; import { join, resolve } from 'node:path'; import { writeStdoutLine, writeStderrLine } from '../../utils/stdioHelpers.js'; +import { + ANSI_SGR_RE, + isDependencyFailureLine, + isDiskFailureLine, + isFailingSurefireSummaryLine, + isGoalFailureLine, + isSourceFailureLine, + isSurefireSummaryLine, + isTestsSkippedLine, + mavenToolchainAdapter, +} from './lib/maven-toolchain.js'; import { npmToolchainAdapter } from './lib/npm-toolchain.js'; import { selectToolchainAdapter, @@ -49,14 +60,30 @@ import { } from './lib/toolchain.js'; import { type TestScope } from './lib/workspace-scope.js'; -/** - * The root toolchains build-test can select. One today; the registry exists so - * the next one is a registration rather than another branch in this file. - */ +/** The root toolchains build-test can select. */ export const toolchainAdapters: readonly ReviewToolchainAdapter[] = [ npmToolchainAdapter, + mavenToolchainAdapter, ]; +/** + * What a Maven lifecycle command this run generated actually scopes. + * + * The adapter builds the command line FROM these values, so a consumer that + * needs them reads them here rather than parsing the string back. Parsing is + * for the free text a PR author writes in a Test Plan; re-deriving our own + * command's meaning from its rendering adds a second grammar that can drift + * from the one that produced it. + */ +export interface MavenCommandFacts { + /** The lifecycle phase the command ends in — `test` or `test-compile`. */ + lifecycle: string; + /** Repo-relative `-pl` module dirs, or null for a reactor-wide run. */ + modules: string[] | null; + /** Whether `-am` upstream expansion was passed. */ + alsoMake: boolean; +} + /** A command this run actually executed, and what it did. */ export interface CommandResult { command: string; @@ -66,6 +93,65 @@ export interface CommandResult { timedOut: boolean; /** Trimmed output: enough to correlate a failure with the diff. */ output: string; + /** + * The adapter classified this failure as infrastructure — Maven/Java or + * dependency acquisition, an unlaunchable wrapper: a result `test-plan` + * must not settle a Test Plan claim against. + */ + infrastructure?: boolean; + /** + * The command exited 0 but its output records failures Maven did not fail + * on (a fail-never or skip-tests setting swallowed them): `test-plan` + * must not rule a Test Plan claim reproduced against this run. + */ + swallowedFailure?: boolean; + /** + * The command's verdict cannot stand on its evidence: fresh reports the + * parser rejected or a truncated sweep never saw, the output trim + * dropped failure-evidence lines past its rescue cap, or a + * `.mvn/maven.config` log-file setting redirected the build output away + * from every scan. The adapter refused to certify the run, and + * `test-plan` must not settle a Test Plan claim against it. Exit-code + * independent — on an exit-0 run it withholds a pass; on a non-zero exit + * the exit remains definitive. (Reports past the parse CAP are the + * opposite: disclosed in the note, because the parsed reports remain + * evidence.) + */ + evidenceCapped?: boolean; + /** + * A skip setting suppressed the entire test phase (`Tests are skipped.`): + * zero tests ran, and `test-plan` must word the contradiction as + * suppression rather than recorded failures. + */ + testsSuppressed?: boolean; + /** + * The command exited 0 over fresh failing Surefire/Failsafe reports (a + * `testFailureIgnore`-style setting swallowed them). None of the other + * flags fire for this shape — they all key on the ABSENCE of fresh + * failing reports — yet the verdict is `ok: false`: consumers filtering + * on the flags must read this as a failed run, never as a pass. + */ + swallowedReports?: boolean; + /** + * The command exited 0 but cannot prove the toolchain started: no fresh + * reports and no toolchain output (an empty or stub wrapper passes the + * launch gates and exits 0). The run verified nothing, and `test-plan` + * must not rule a claim reproduced against it. + */ + neverRan?: boolean; + /** + * The output trim's rescue cap dropped failure-evidence lines from the + * omitted middle (dependency/source/goal/disk failures, module errors, + * failing Surefire summaries): the classifiers read an output whose + * verdict-relevant lines may be gone — the same epistemic state as + * `evidenceCapped`, which the Maven adapter folds it into. + */ + rescueOverflow?: boolean; + /** + * Present on a Maven LIFECYCLE command (not the dependency warm-up): what + * it scopes, as the adapter knew it when it built the command line. + */ + maven?: MavenCommandFacts; /** * The deadline the command was actually given (ms) — the whole-call budget * shortens it below the per-command default, and the timeout note must @@ -76,8 +162,8 @@ export interface CommandResult { export interface BuildTestReport { /** The scoped toolchain that ran, or `unsupported` when selection was unsafe. */ - toolchain: 'npm' | 'unsupported'; - /** Workspace dirs the diff changed. */ + toolchain: 'npm' | 'maven' | 'unsupported'; + /** Workspace or Maven module dirs the diff changed. */ affected: string[]; /** What was built, dependencies first — after any widening. */ buildSet: string[]; @@ -154,17 +240,37 @@ const MODULE_ERROR_RE = /Cannot find module '[^']+'|Could not resolve "[^"]+"/; */ const RUNNER_SUMMARY_RE = /^\s*(?:Tests?|Test Files):?\s+\d/; -/** SGR color sequences — stripped per line before the summary test, because a - * real runner interleaves them BETWEEN tokens (`Tests\x1b[2m \x1b[22m3 failed`), - * where no anchored pattern can step over them. The rescued line itself keeps - * its original bytes. */ -// eslint-disable-next-line no-control-regex -- ESC is the character under test -const ANSI_SGR_RE = /\x1b\[[0-9;]*m/g; - -export function trimOutput(s: string): string { - if (s.length <= KEEP_HEAD + KEEP_TAIL) return s; - const middle = s.slice(KEEP_HEAD, s.length - KEEP_TAIL); - // Rescue module-resolution errors from the omitted middle. The widening loop +/** SGR color sequences come from the Maven adapter's `ANSI_SGR_RE` export, + * shared so the rescue below and the adapter's own classification strip the + * same bytes: a real runner interleaves them BETWEEN tokens + * (`Tests\x1b[2m \x1b[22m3 failed`), where no anchored pattern can step + * over them. The rescued line itself keeps its original bytes. */ +export function trimOutput(s: string): { + /** The trimmed output the report carries. */ + text: string; + /** + * The rescue cap dropped failure-evidence lines from the omitted middle: + * classification reads an output whose verdict-relevant lines may be + * gone — the Maven adapter folds this into `evidenceCapped`. + */ + evidenceDropped: boolean; +} { + if (s.length <= KEEP_HEAD + KEEP_TAIL) + return { text: s, evidenceDropped: false }; + // Align both cuts to line boundaries: every rescue and classification + // predicate is line-anchored, so a verdict-critical line straddling a + // mid-line cut would fragment into two pieces that match nothing — + // neither rescued nor classified. + const headNewline = s.lastIndexOf('\n', KEEP_HEAD); + const headEnd = headNewline === -1 ? KEEP_HEAD : headNewline + 1; + const tailNewline = s.indexOf('\n', s.length - KEEP_TAIL); + const tailStart = tailNewline === -1 ? s.length - KEEP_TAIL : tailNewline + 1; + // Head and tail already cover the whole string line-wise — one long line + // spans the middle, and splitting it would break the very line the + // alignment exists to protect. + if (headEnd >= tailStart) return { text: s, evidenceDropped: false }; + const middle = s.slice(headEnd, tailStart); + // Rescue verdict-relevant lines from the omitted middle. The widening loop // reads this trimmed output to decide what to add to the build set — a `Cannot // find module` line lost to trimming (a long TypeScript log can push one past the // head and before the tail) would end the widening early and surface a real @@ -172,21 +278,80 @@ export function trimOutput(s: string): string { // CAPPED: the rescue exists to save a handful of summary/module-error lines, // and an uncapped predicate made the whole trim a no-op on 40k lines of // `Test : …` prose (measured in review — 1.6 MB in, 1.6 MB out). Past the - // cap the trim's bounded-output contract wins and the rest stays omitted. + // cap the trim's bounded-output contract wins and the rest stays omitted — + // but evidence lines take the slots FIRST: benign matches (green Surefire + // summaries, skip markers, runner summaries) must not exhaust the cap in + // positional order and drop the failure lines a verdict reads. Dropped + // evidence lines fail closed through `evidenceDropped`. const RESCUE_MAX = 40; - const rescued = middle - .split('\n') - .filter( - (l) => - MODULE_ERROR_RE.test(l) || - RUNNER_SUMMARY_RE.test(l.replace(ANSI_SGR_RE, '')), - ) - .slice(0, RESCUE_MAX); - const omitted = s.length - KEEP_HEAD - KEEP_TAIL; + const matched: Array<{ line: string; index: number; evidence: boolean }> = []; + middle.split('\n').forEach((line, index) => { + // The widening loop reads module errors from the ORIGINAL bytes. + if (MODULE_ERROR_RE.test(line)) { + matched.push({ line, index, evidence: true }); + return; + } + // Strip SGR ONCE per line: the classifiers below all read the same + // stripped copy. + const stripped = line.replace(ANSI_SGR_RE, ''); + // Maven infra classification runs on this trimmed output; a + // dependency-failure line lost to the trim would file a network + // outage against the PR, a source-failure line lost there would + // launder a compile error into infrastructure, a goal-failure line + // lost there would read a fail-never plugin failure green, and a + // disk-failure line lost there would file an ENOSPC death against + // the PR (or, under fail-never, read the run green) — the exact + // errors this command prevents. A FAILING Surefire stdout summary is + // the same class: the exit-0 cross-check's one defense for + // relocated-`` runs. + if ( + isDependencyFailureLine(stripped) || + isSourceFailureLine(stripped) || + isGoalFailureLine(stripped) || + isDiskFailureLine(stripped) || + isFailingSurefireSummaryLine(stripped) || + // The adapter's testsSuppressed guard reads the skip marker from this + // trimmed output; a large reactor's trailing Reactor Summary pushes + // every `Tests are skipped.` line into the omitted middle, and losing + // it certifies a run that tested zero. The marker carries a verdict + // (suppression), so it takes an evidence slot — benign matches must + // not exhaust the cap and drop it while `evidenceDropped` stays + // false. + isTestsSkippedLine(stripped) + ) { + matched.push({ line, index, evidence: true }); + return; + } + // Green Surefire summaries and runner summaries carry counts, never + // verdicts. + if (isSurefireSummaryLine(stripped) || RUNNER_SUMMARY_RE.test(stripped)) { + matched.push({ line, index, evidence: false }); + } + }); + const evidenceCount = matched.reduce( + (sum, item) => sum + (item.evidence ? 1 : 0), + 0, + ); + const kept = + matched.length <= RESCUE_MAX + ? matched + : [ + ...matched.filter((item) => item.evidence).slice(0, RESCUE_MAX), + ...matched + .filter((item) => !item.evidence) + .slice(0, RESCUE_MAX - Math.min(evidenceCount, RESCUE_MAX)), + ].sort((a, b) => a.index - b.index); + const rescued = kept.map((item) => item.line); + const evidenceDropped = evidenceCount > RESCUE_MAX; + const omitted = middle.length; + const dropped = matched.length - kept.length; const marker = rescued.length - ? `\n\n... [${omitted} characters omitted; module-resolution errors and runner summaries kept] ...\n${rescued.join('\n')}\n\n` + ? `\n\n... [${omitted} characters omitted; module-resolution errors, dependency failures, source failures, goal failures, disk failures, skipped-test markers, Surefire stdout summaries, and runner summaries kept${dropped > 0 ? ` — first ${RESCUE_MAX} matches kept, failure evidence before benign lines, ${dropped} more omitted` : ''}] ...\n${rescued.join('\n')}\n\n` : `\n\n... [${omitted} characters omitted] ...\n\n`; - return s.slice(0, KEEP_HEAD) + marker + s.slice(-KEEP_TAIL); + return { + text: s.slice(0, headEnd) + marker + s.slice(tailStart), + evidenceDropped, + }; } /** @@ -236,12 +401,14 @@ function run(command: string, cwd: string, timeoutMs: number): CommandResult { // also matches an external SIGTERM (a container stop), and it misses a non-default // `killSignal`. Check the authoritative one first. const timedOut = spawnTimedOut(r); + const trimmed = trimOutput(`${r.stdout ?? ''}${r.stderr ?? ''}`); return { command, exitCode: r.status, seconds: Math.round((Date.now() - started) / 1000), timedOut, - output: trimOutput(`${r.stdout ?? ''}${r.stderr ?? ''}`), + output: trimmed.text, + ...(trimmed.evidenceDropped ? { rescueOverflow: true } : {}), deadlineMs, }; } @@ -343,26 +510,25 @@ export function runBuildTest(args: BuildTestArgs): BuildTestReport { ); if (!adapter) { if (applicable.length > 1) { - // Unreachable with one registered adapter, and deliberately kept: the - // selection contract is "exactly one, or nothing", and the second - // adapter must land in a file that already refuses to guess between - // them rather than one that has to grow the branch. - return { - toolchain: 'unsupported', - affected: [], - buildSet: [], - widenedWith: [], - install: null, - build: [], - test: [], - ok: true, - timedOut: [], - note: - 'More than one toolchain applies at the repository root. build-test will ' + - 'not guess which one owns this diff, so it ran nothing — report the ' + - 'ambiguity as a handoff instead of substituting ad hoc build or test ' + - 'commands.', - }; + // Both toolchains apply at the root. Preferring npm preserves what a + // review verified on this shape before the Maven adapter existed — + // running NOTHING at all shipped a broken JS build through review + // with zero evidence. The note discloses the Maven half so a green + // npm run never certifies it. + const report = npmToolchainAdapter.run(runArgs); + // npm's applies() held but run() can still concede WITHOUT executing + // (a cold yarn/pnpm/bun repo — the common case for a review worktree): + // nothing ran, so there is no npm half to disclose and the Maven half + // must run instead of certifying nothing. The Maven adapter's own + // mixed-root caveat discloses the unscopable npm side of this root. + if (report.toolchain === 'unsupported') { + return mavenToolchainAdapter.run(runArgs); + } + report.note += + ' Mixed root: Maven also applies at the repository root (pom.xml), but ' + + 'this run executed the npm toolchain only — Maven-built modules were ' + + 'NOT verified here.'; + return report; } // A root package.json marks an npm-shaped repo that npm's own gate refused // (an unmodeled workspace glob, workspaces that resolve to no package, or @@ -386,7 +552,7 @@ export function runBuildTest(args: BuildTestArgs): BuildTestReport { ok: true, timedOut: [], note: - 'No supported npm project here to scope. Fall back to the ' + + 'No supported npm or Maven project here to scope. Fall back to the ' + 'build/test precedence in your brief — installing dependencies first — ' + 'and give each command a deadline it can actually meet.', }; @@ -397,8 +563,8 @@ export function runBuildTest(args: BuildTestArgs): BuildTestReport { export const buildTestCommand: CommandModule = { command: 'build-test', describe: - 'Build the workspaces the diff changes (and what they compile against), ' + - 'test those plus their dependents, with a deadline the commands can ' + + 'Build what the diff changes (npm: plus their dependents; Maven: plus ' + + 'their upstream closure), test it, with a deadline the commands can ' + 'actually meet', builder: (yargs) => yargs @@ -445,7 +611,9 @@ export const buildTestCommand: CommandModule = { type: 'boolean', default: true, describe: - 'Fetch dependencies first: `npm ci` when node_modules is absent', + 'Fetch dependencies first: `npm ci` when node_modules is absent (npm), ' + + 'or a best-effort `dependency:go-offline` warm-up with its own deadline ' + + '(Maven)', }) .option('build-only', { type: 'boolean', diff --git a/packages/cli/src/commands/review/lib/agent-briefs.ts b/packages/cli/src/commands/review/lib/agent-briefs.ts index 53e304a0bef..b5a0cbd652c 100644 --- a/packages/cli/src/commands/review/lib/agent-briefs.ts +++ b/packages/cli/src/commands/review/lib/agent-briefs.ts @@ -522,13 +522,14 @@ You are undirected on purpose. Do not restrict yourself to the list.`, readsDiff: false, brief: `You are **Agent 7: Build & Test Verification**. You do not review the diff — you run the project's own deterministic checks and report what they say. Your evidence is **the commands you ran and their output**; a return that names no command has not done this job. -**Run \`qwen review build-test\` (the exact command, with its \`--plan\` and \`--worktree\`, is below).** It installs if needed, then builds only the workspaces the diff changes plus everything they compile against, and tests the changed ones plus every workspace that depends on them — reading the plan for what changed and the root \`package.json\` for the workspace layout. Do **not** substitute \`npm run build\` / \`npm test\` by hand. The old brief did, with a 120-second deadline, and this repo's cold full build is 125 seconds: measured across the harness's own transcripts, that command timed out **71 times** and verified nothing. \`build-test\` scopes the build, gives it a deadline it can meet, and — this is the part a hand-run command gets wrong — reports a timeout as **infrastructure, not a finding**. A build that runs out of time is never a Critical against someone's pull request. +**Run \`qwen review build-test\` (the exact command, with its \`--plan\` and \`--worktree\`, is below).** It detects supported npm or Maven projects, scopes the run to what the diff changed, and reports the commands and evidence as JSON. Scope differs by toolchain — npm builds the changed workspaces, their dependencies, AND their dependents; Maven runs \`-pl -am\` — the changed modules plus their **upstream** dependency closure only. A Maven run therefore does NOT verify the downstream modules that consume the changed ones (a POM or API change can still break them); their coverage stays with the project's CI, so never report a module as verified when it was only a dependent of what ran. Do **not** substitute hand-written npm or Maven commands for what \`build-test\` runs — the \`toolchain: "unsupported"\` fallback below is the only sanctioned hand-run path. The old npm brief used a 120-second full-build deadline; measured across the harness's own transcripts, it timed out **71 times** and verified nothing. \`build-test\` scopes the run, gives each command a deadline it can meet, and — this is the part a hand-run command gets wrong — reports a timeout or dependency-acquisition failure as **infrastructure, not a finding**. A build that runs out of time is never a Critical against someone's pull request. Read the JSON it prints: -- \`toolchain: "npm"\` → use its \`build[]\` / \`test[]\` results. A failure in a file **the diff changed** is a **Critical** (\`Source: [build]\` or \`[test]\`); a failure in a file it did **not** touch is pre-existing — say so, do not file it against this PR. A non-empty \`timedOut\`, or a failed \`install\`, is environment/infrastructure — informational, never a Critical. On \`ok: true\`, name the workspaces built and the commands run; a return that names no command is a whiff. Report the TEST coverage from \`testScope\`, never from assumption. \`testScope.workspaces\` lists exactly the suites that ran — say "tests scoped to — the changed workspaces and their declared dependents that define a test script". \`testScope.notRun\`, when present, names suites the whole-call budget stopped before they ran — say they did not run, never fold them into the coverage. When \`testScope.caveat\` is present, the scope may be incomplete — quote the caveat and say exactly that. A green run is a claim about those suites only — do not phrase it as the whole suite passing. -- **When any \`test[]\` command failed (exit non-zero, not a timeout), MEASURE which failures are the PR's before ruling by path.** The path rule above misclassifies in both directions — an environment-flaky test in a touched file gets filed as a Critical it did not cause, and a PR that breaks a test in an UNTOUCHED file gets waved through as pre-existing. The measurement is two commands: \`qwen review base-tree --plan --worktree --out /qwen-review-pr--base-tree.json\` (builds the merge base beside the worktree). **Read \`available\` before using \`path\`** — a tree that was created but did NOT build populates \`path\` too, and a base that failed to build says nothing whatsoever about the PR, so measuring against it turns an infrastructure failure into a list of Criticals. \`available: false\` (local/lightweight review, no merge base, a base that would not compile) means the path rule stands — say so and stop here, and \`qwen review test-delta --report --baseline --pr-worktree --out /qwen-review-pr--test-delta.json\`. Read its verdict: a file in \`netNew\` fails on the PR side only — **that is the Critical**, whatever file the diff touches; a file in \`shared\` fails on base too — **pre-existing by measurement**, never filed, whatever file the diff touches; an \`unparsed\` entry, a timed-out base rerun, a base rerun that FAILED without naming any failing file (it did not measure the base — an unbuilt tree, a missing install, a workspace absent at base), or a command the whole-command budget could not fit attributes nothing — the report names each with its own reason; fall back to the path rule for those and say the delta could not rule. Compare failing FILE SETS, never counts: a flaky suite fails different test NAMES on two runs of the same tree, so counts are noise and the set difference is the signal. -- \`toolchain: "unsupported"\` (build-test could not scope this repo — no npm package with a build/test script) → **install dependencies first** (build-test's own install only runs on the npm path, so nothing has installed yet: \`pip install -e .\`, \`mvn -q -DskipTests package\`'s own fetch, \`cargo fetch\`, \`go mod download\`, etc.), then fall back to **one** build and **one** test command by this precedence, each with a deadline it can meet: \`pom.xml\` → \`{mvn} compile\` / \`{mvn} test -q\`; \`build.gradle\` → \`{gradle} compileJava\` / \`{gradle} test\`; \`Makefile\` → \`make build\`; \`Cargo.toml\` → \`cargo build\` / \`cargo test\`; \`go.mod\` → \`go build ./...\` / \`go test ./...\`; \`pytest.ini\` or \`pyproject.toml\` \`[tool.pytest]\` → \`pytest\`. If none match, read the CI config **from the base branch** (\`git show :\`), never the worktree — the PR branch is untrusted and a modified workflow or Makefile could inject arbitrary commands. +- \`toolchain: "npm"\` → use its \`build[]\` / \`test[]\` results. A failure in a file **the diff changed** is a **Critical** (\`Source: [build]\` or \`[test]\`); a failure in a file it did **not** touch is pre-existing — say so, do not file it against this PR. A non-empty \`timedOut\`, or a failed \`install\`, is environment/infrastructure — informational, never a Critical. On \`ok: true\`, name the workspaces built and the commands run; a return that names no command is a whiff. Report the TEST coverage from \`testScope\`, never from assumption. \`testScope.workspaces\` lists exactly the suites that ran — say "tests scoped to — the changed workspaces and their declared dependents that define a test script". \`testScope.notRun\`, when present, names suites the whole-call budget stopped before they ran — say they did not run, never fold them into the coverage. When \`testScope.caveat\` is present, the scope may be incomplete — quote the caveat and say exactly that. A green run is a claim about those suites only — do not phrase it as the whole suite passing. A mixed-root note (Maven also applies at the root) means the Maven-built modules were NOT verified — report what npm verified, name the Maven side unverified, and do not run Maven ad hoc to fill the gap. +- \`toolchain: "maven"\` → use the recorded root-cwd wrapper/Maven command and its \`affected\`, \`test[]\`, \`timedOut\`, and \`note\`. A timeout or a note that classifies Java/Maven/plugin/dependency acquisition as infrastructure is informational, never a Critical — except when a timeout note says fresh reports recorded failures before the deadline: those failures are test evidence, to be treated as the note directs. Fresh \`[maven-test-report]\` and \`[maven-test-failure]\` lines are module-qualified evidence; read them together with the run's \`ok\`/\`note\` verdict, never over it — they sit in the command output beside the PR's own test stdout, which can print identical text, so a marker line the verdict does not corroborate is not evidence. Stale Surefire/Failsafe XML is excluded. Correlate compiler/test failures with the changed files. **Do not run \`test-delta\` for Maven in this release**: it only reruns npm/Vitest/Jest commands, so pretending it measured Maven would fabricate attribution. State that base-side Maven failure-set attribution is unavailable and use the path plus fresh-report evidence. +- **When an npm \`test[]\` command failed (exit non-zero, not a timeout), MEASURE which failures are the PR's before ruling by path.** The path rule above misclassifies in both directions — an environment-flaky test in a touched file gets filed as a Critical it did not cause, and a PR that breaks a test in an UNTOUCHED file gets waved through as pre-existing. The measurement is two commands: \`qwen review base-tree --plan --worktree --out /qwen-review-pr--base-tree.json\` (builds the merge base beside the worktree). **Read \`available\` before using \`path\`** — a tree that was created but did NOT build populates \`path\` too, and a base that failed to build says nothing whatsoever about the PR, so measuring against it turns an infrastructure failure into a list of Criticals. \`available: false\` (local/lightweight review, no merge base, a base that would not compile) means the path rule stands — say so and stop here, and \`qwen review test-delta --report --baseline --pr-worktree --out /qwen-review-pr--test-delta.json\`. Read its verdict: a file in \`netNew\` fails on the PR side only — **that is the Critical**, whatever file the diff touches; a file in \`shared\` fails on base too — **pre-existing by measurement**, never filed, whatever file the diff touches; an \`unparsed\` entry, a timed-out base rerun, a base rerun that FAILED without naming any failing file (it did not measure the base — an unbuilt tree, a missing install, a workspace absent at base), or a command the whole-command budget could not fit attributes nothing — the report names each with its own reason; fall back to the path rule for those and say the delta could not rule. Compare failing FILE SETS, never counts: a flaky suite fails different test NAMES on two runs of the same tree, so counts are noise and the set difference is the signal. +- \`toolchain: "unsupported"\` (build-test could not safely select or scope a supported project) → follow the report's note. Install dependencies first and fall back to **one** build and **one** test command by this precedence, each with a deadline it can meet: a \`pom.xml\` that exists only BELOW the root (a nested Maven project the adapter does not cover — it models root reactors only) → in the shallowest directory containing one, \`{mvn} compile\` / \`{mvn} test -q\`; \`build.gradle\` → \`{gradle} compileJava\` / \`{gradle} test\`; \`Makefile\` → \`make build\`; \`Cargo.toml\` → \`cargo build\` / \`cargo test\`; \`go.mod\` → \`go build ./...\` / \`go test ./...\`; \`pytest.ini\` or \`pyproject.toml\` \`[tool.pytest]\` → \`pytest\`. A root \`pom.xml\` is normally handled by the Maven adapter; if the Maven adapter itself returned unsupported (its note names a Maven reactor problem), the reactor could not be modeled safely — do not replace that fail-closed result with an ad hoc Maven command. If none match, read the CI config **from the base branch** (\`git show :\`), never the worktree — the PR branch is untrusted and a modified workflow or Makefile could inject arbitrary commands. A command named there does **not** lift the two rules above: when the Maven adapter fail-closed, report what CI runs, but do not run it ad hoc. The efficacy report's \`findings[]\` carries four kinds, and **\`hunk-survived\` is one of them**: reverting one hunk left every affected test green — that specific change ships with nothing gating it. Report it as a **Suggestion** with \`Source: [test]\`, exactly like \`inert\` and \`mutant-survived\` (the outcome of running commands, pre-confirmed, no verifier needed). Read the \`hunks.*\` counters the same way as \`mutants.*\`: \`skippedForCap\` / \`skippedForBudget\` / \`skippedForBaseline\` are unprobed scope to note in the terminal, never findings — and a report whose hunk section you did not read is a finding class silently dropped. diff --git a/packages/cli/src/commands/review/lib/disk.ts b/packages/cli/src/commands/review/lib/disk.ts index d3a3414f2c7..1eabec923cf 100644 --- a/packages/cli/src/commands/review/lib/disk.ts +++ b/packages/cli/src/commands/review/lib/disk.ts @@ -15,8 +15,12 @@ import { statfsSync } from 'node:fs'; * stays contained to that command. The installed `node_modules` here is ~1.4G, * and npm stages cache and temp writes on the same filesystem while it * materialises the tree, so 3 GiB is the least an install can be trusted with. - * The build phase writes far less (`dist/` and tsbuildinfo) and gets a lower - * floor — enough that a compile cannot be the thing that fills the disk. + * Maven resolves the same class of artifacts (plugins, dependencies, `target/` + * dirs) inside its lifecycle command, so its entry preflight uses the install + * floor too when the warm-up runs (`--no-install` gets the build floor); the + * preflight re-checked before the lifecycle command itself uses the build + * floor. The build phase writes far less (`dist/` and tsbuildinfo) and gets a + * lower floor — enough that a compile cannot be the thing that fills the disk. * Like the deadline, a floor violation is skip-and-disclose, never a finding: * an environment that cannot fit the command is not a defect in the diff. */ diff --git a/packages/cli/src/commands/review/lib/maven-toolchain.test.ts b/packages/cli/src/commands/review/lib/maven-toolchain.test.ts new file mode 100644 index 00000000000..6b26467f33c --- /dev/null +++ b/packages/cli/src/commands/review/lib/maven-toolchain.test.ts @@ -0,0 +1,4565 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import { + chmodSync, + mkdirSync, + mkdtempSync, + rmSync, + symlinkSync, + utimesSync, + writeFileSync, +} from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; +import type { BuildTestReport, CommandResult } from '../build-test.js'; +import type { ToolchainRunArgs } from './toolchain.js'; +import { observedTestCounts } from '../test-plan.js'; +import { + detectMavenOwnership, + isDependencyFailureLine, + isSourceFailureLine, + mavenExecutable, + mavenToolchainAdapter, + reportPaths, + shellSelector, +} from './maven-toolchain.js'; + +const statfsSyncMock = vi.hoisted(() => vi.fn()); +vi.mock('node:fs', async (importOriginal) => { + const actual = (await importOriginal()) as Record; + const mock = { ...actual, statfsSync: statfsSyncMock }; + return { ...mock, default: mock }; +}); + +// Plenty of disk by default, so this suite behaves the same on a nearly-full +// machine as on an empty one — the low-disk case below opts in explicitly. +beforeEach(() => { + statfsSyncMock.mockReturnValue({ bavail: 16 * 1024 ** 3, bsize: 1 }); +}); + +const pom = (modules: string[] = []): string => ` + + 4.0.0 + example + fixture + 1 + + ${modules.map((module) => `${module}`).join('\n ')} + + +`; + +const result = ( + command: string, + overrides: Partial = {}, +): CommandResult => ({ + command, + exitCode: 0, + seconds: 1, + timedOut: false, + // A real Maven run always frames output; the empty-output exit-0 shape + // is the adapter's "never ran" classification, not a clean run. + output: '[INFO] BUILD SUCCESS', + ...overrides, +}); + +describe('maven toolchain adapter', () => { + let root: string; + let sandbox: string; + + beforeEach(() => { + sandbox = mkdtempSync(join(tmpdir(), 'maven-toolchain-')); + // One level deeper than the mkdtemp root: the reactor-escape fixtures + // create `../outside`, which must land inside this sandbox and get + // cleaned, not at a fixed path in the shared OS tmpdir. + root = join(sandbox, 'repo'); + mkdirSync(root); + }); + + afterEach(() => { + rmSync(sandbox, { recursive: true, force: true }); + }); + + function writeProject(dir: string, modules: string[] = []): void { + const path = join(root, dir); + mkdirSync(path, { recursive: true }); + writeFileSync(join(path, 'pom.xml'), pom(modules)); + } + + function writeReactor(): void { + writeProject('.', ['core', 'extension', 'nested-parent']); + writeProject('core'); + writeProject('extension'); + writeProject('nested-parent', ['nested-leaf']); + writeProject('nested-parent/nested-leaf'); + } + + function writeWrapper(): void { + writeFileSync(join(root, 'mvnw'), '#!/bin/sh\n'); + chmodSync(join(root, 'mvnw'), 0o755); + } + + /** The wrapper this platform actually executes (`mvnw.cmd` on win32): + * classification arms keyed on the EXECUTED wrapper need the fixture in + * its platform form, or they read the `mvn` fallback state instead. */ + function writeExecutedWrapper(): void { + if (process.platform === 'win32') { + writeFileSync(join(root, 'mvnw.cmd'), '@echo off\r\n'); + } else { + writeWrapper(); + } + } + const executedWrapperName = + process.platform === 'win32' ? 'mvnw.cmd' : 'mvnw'; + + /** + * The adapter over the sandbox reactor, with this suite's standard run + * arguments: the temp `root`, a 5s per-command deadline, no dependency + * warm-up, and an executor that reports every command as clean. + * + * Anything a case actually cares about it passes in `opts` — an `exec` that + * scripts a failure or records the command line, `install: true`, + * `buildOnly`, a `budget`. Spreading last means an override reads at the + * call site instead of hiding in lines of identical setup. + */ + const runAdapter = ( + changedFiles: string[], + opts: Partial> = {}, + ): BuildTestReport => + mavenToolchainAdapter.run({ + root, + changedFiles, + timeout: 5, + install: false, + exec: (command) => result(command), + ...opts, + }); + + it('marks Maven build files reactor-wide, scopes root sources to the root project, and leaves docs without targets', () => { + writeReactor(); + + expect( + detectMavenOwnership(root, [ + 'pom.xml', + '.mvn/maven.config', + 'mvnw', + 'mvnw.cmd', + ]), + ).toEqual({ + reactorWide: true, + modules: [], + }); + + // The root artifact's own src/ is owned by the root project '.': it + // verifies with `-pl . -am`, not the entire reactor. + expect( + detectMavenOwnership(root, ['src/main/java/example/Root.java']), + ).toEqual({ + reactorWide: false, + modules: ['.'], + }); + + expect(runAdapter(['docs/guide.md'])).toMatchObject({ + toolchain: 'maven', + affected: [], + buildSet: [], + build: [], + test: [], + ok: true, + }); + }); + + it('leaves module documentation changes without a Maven target', () => { + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/README.md', 'core/docs/guide.md'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(true); + expect(report.affected).toEqual([]); + expect(calls).toEqual([]); + }); + + it('still builds the owning module for documentation-extension files under its src/', () => { + // The src/ guard is re-rooted to the owning module: a .txt under a + // module's source tree is test data, not documentation. + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/src/test/resources/expected.txt'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.toolchain).toBe('maven'); + expect(calls).toEqual([ + 'mvn --batch-mode --no-transfer-progress -pl core -am test', + ]); + }); + + it('keeps verifying the owning module when a test-fixture POM sits under its src/', () => { + // maven-invoker ITs, archetype fixtures, and src/test/resources/projects/* + // trees are test DATA: Maven never builds them as reactor modules and no + // profile activates them. Reading one as a standalone project used to fail + // the WHOLE diff closed — including the real source change beside it. + writeReactor(); + writeProject('core/src/test/resources/projects/sample'); + const calls: string[] = []; + + const report = runAdapter( + [ + 'core/src/main/java/Core.java', + 'core/src/test/resources/projects/sample/App.java', + ], + { + exec: (command) => { + calls.push(command); + return result(command); + }, + }, + ); + + expect(report.toolchain).toBe('maven'); + expect(calls).toEqual([ + 'mvn --batch-mode --no-transfer-progress -pl core -am test', + ]); + }); + + it('fails closed to reactor-wide when a skipped src/ POM would collapse to the root', () => { + writeProject('.'); + writeProject('src/test/resources/projects/sample'); + const calls: string[] = []; + + const report = runAdapter(['src/test/resources/projects/sample/App.java'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + // The `src/` skip exists for test-data POMs, but the walk cannot tell a + // fixture from a REAL module nested under `src/` (a reactor can + // aggregate `src/core`). Collapsing to `-pl .` would + // compile only the root and certify the changed module untested, so the + // skip fails closed: the whole reactor runs instead. + expect(report.toolchain).toBe('maven'); + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.affected).toEqual(['.']); + }); + + it('scopes root-project source fixtures with documentation extensions to the root project', () => { + writeProject('.'); + const calls: string[] = []; + + const report = runAdapter(['src/test/resources/expected.txt'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.affected).toEqual(['.']); + expect(calls).toEqual([ + 'mvn --batch-mode --no-transfer-progress -pl . -am test', + ]); + }); + + it('prefers the wrapper, runs from root, narrows modules, and forwards timeout', () => { + writeReactor(); + // The wrapper a platform can actually execute: win32 `cmd.exe` runs + // `mvnw.cmd` and cannot run `./mvnw`; POSIX needs the executable bit. + const windows = process.platform === 'win32'; + if (windows) { + writeFileSync(join(root, 'mvnw.cmd'), '@echo off\n'); + } else { + writeWrapper(); + } + const calls: Array<[string, string, number]> = []; + // The deadline is wall clock from the top of the call: freeze it so + // the forwarded deadline asserts exactly. + const clock = 0; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock); + + let report: ReturnType; + try { + report = runAdapter( + [ + 'extension/src/main/java/example/Extension.java', + 'core/src/main/java/example/Core.java', + ], + { + timeout: 17, + exec: (command, cwd, timeout) => { + calls.push([command, cwd, timeout]); + return result(command); + }, + }, + ); + } finally { + nowSpy.mockRestore(); + } + + const executable = windows ? 'mvnw.cmd' : './mvnw'; + expect(calls).toEqual([ + [ + `${executable} --batch-mode --no-transfer-progress -pl core,extension -am test`, + root, + 17_000, + ], + ]); + expect(report).toMatchObject({ + toolchain: 'maven', + affected: ['core', 'extension'], + buildSet: ['core', 'extension'], + install: null, + build: [], + ok: true, + }); + expect(report.test[0]?.command).toContain('-pl core,extension -am test'); + // Agent 7 reads the note as evidence: it must say the run did NOT build + // downstream dependents, or a dependent module gets reported as verified. + expect(report.note).toContain('upstream dependencies only'); + expect(report.note).toContain('downstream dependents were NOT built'); + }); + + it('uses mvn and test-compile for build-only mode', () => { + writeReactor(); + const report = runAdapter(['core/src/main/java/example/Core.java'], { + buildOnly: true, + exec: (command, cwd) => { + expect(cwd).toBe(root); + return result(command); + }, + }); + + expect(report.test).toEqual([]); + expect(report.build[0]?.command).toBe( + 'mvn --batch-mode --no-transfer-progress -pl core -am test-compile', + ); + }); + + it('does not narrow reactor-wide changes', () => { + writeReactor(); + const report = runAdapter(['.mvn/maven.config']); + + expect(report.affected).toEqual(['.']); + expect(report.test[0]?.command).toBe( + 'mvn --batch-mode --no-transfer-progress test', + ); + // A full-reactor run must not carry the narrowed-run scope statement. + expect(report.note).not.toContain('downstream dependents were NOT built'); + }); + + it('discloses that a reactor-wide timeout is expected to exceed the deadline', () => { + // On the large reactors this adapter targets, a root-POM change selects + // the whole reactor, and `test` over it cannot finish in the default + // deadline — say so, so no agent spends turns re-deriving it. + writeReactor(); + + const report = runAdapter(['pom.xml'], { + timeout: 300, + exec: (command) => + result(command, { exitCode: null, timedOut: true, seconds: 300 }), + }); + + expect(report.note).toContain('infrastructure result'); + expect(report.note).toContain('reactor-wide'); + expect(report.note).toContain('same scope'); + }); + + it('classifies timeout and dependency resolution without fresh reports as infrastructure', () => { + writeReactor(); + const timeout = runAdapter(['core/src/Main.java'], { + timeout: 2, + exec: (command) => + result(command, { exitCode: null, timedOut: true, seconds: 2 }), + }); + expect(timeout.ok).toBe(false); + expect(timeout.timedOut).toEqual([timeout.test[0]?.command]); + expect(timeout.note).toContain('infrastructure result'); + + const resolution = runAdapter(['core/src/Main.java'], { + timeout: 2, + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + expect(resolution.note).toContain('infrastructure evidence'); + expect(resolution.test[0]).toMatchObject({ infrastructure: true }); + expect(timeout.test[0]?.infrastructure).toBeUndefined(); + }); + + it.skipIf(process.platform === 'win32')( + 'does not classify a changed wrapper permission failure as infrastructure', + () => { + // On win32 `mvnw` is the other platform's wrapper and is skipped by + // ownership, so the adapter sees no Maven target to run. + + writeReactor(); + writeWrapper(); + + const report = runAdapter(['mvnw'], { + exec: (command) => + result(command, { + exitCode: 126, + output: '/bin/sh: ./mvnw: Permission denied', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'does not hide a permission failure behind `./mvnw` spelled differently', + () => { + // On win32 the normalized `./mvnw` path is the other platform's + // wrapper and is skipped by ownership, so no permission-failure note + // is produced. + + // The guard compares normalized paths: `./mvnw` and absolute paths name + // the same wrapper the raw comparison missed. + writeReactor(); + writeWrapper(); + + const report = runAdapter(['./mvnw'], { + exec: (command) => + result(command, { + exitCode: 126, + output: '/bin/sh: ./mvnw: Permission denied', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }, + ); + + it.each([ + ['sh: 1: mvn: not found', 127], + // zsh names the command LAST; it also prints this in sh-compat mode. + ['zsh: command not found: mvn', 127], + ['sh: command not found: mvn', 127], + // PowerShell's phrasing when Maven is absent. + [ + "mvn: The term 'mvn' is not recognized as a name of a cmdlet, function, script file, or operable program", + 127, + ], + // fish's phrasing. + ['fish: Unknown command: mvn', 127], + // cmd.exe's wording when Maven is absent on Windows (exit 9009). + ["'mvn' is not recognized as an internal or external command", 9009], + [ + '[ERROR] Failed to execute goal on project core: java.io.IOException: No space left on device', + 1, + ], + ['Error: The JAVA_HOME environment variable is not defined correctly', 1], + // mvn.cmd/mvnw.cmd on Windows when JAVA_HOME is UNSET names the failure + // differently than the POSIX wrapper's "not defined correctly". + ['Error: JAVA_HOME not found in your environment.', 1], + // mvn.cmd/mvnw.cmd on Windows, when JAVA_HOME points at an invalid + // directory. + ['ERROR: JAVA_HOME is set to an invalid directory: C:\\old\\jdk', 1], + ['Unable to locate a Java Runtime', 1], + ])( + 'classifies unchanged Maven startup failures as infrastructure', + (output, exitCode) => { + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode, output }), + }); + + expect(report.note).toContain('infrastructure evidence'); + }, + ); + + it('does not classify unframed disk-full words as a launch failure', () => { + // `No space left on device` without Maven's `[ERROR]` framing is a test + // exercising a disk-full path, not an outage; free text cannot tell the + // two apart, so the framing decides, as for dependency failures. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: 'java.io.IOException: No space left on device', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it.skipIf(process.platform === 'win32')( + 'classifies an unchanged wrapper launch failure as infrastructure', + () => { + // The guard needs `executable === './mvnw'`, unreachable on win32. + writeReactor(); + writeWrapper(); + + const runWith = (exitCode: number, output: string) => + runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode, output }), + }); + + const denied = runWith(126, '/bin/sh: ./mvnw: Permission denied'); + expect(denied.note).toContain('infrastructure evidence'); + + // A CRLF-committed wrapper dies at shebang resolution on Linux. + const crlf = runWith( + 126, + '/bin/sh: ./mvnw: /bin/sh^M: bad interpreter: No such file or directory', + ); + expect(crlf.note).toContain('infrastructure evidence'); + + // Some shells report the same death with exit 127. + const crlf127 = runWith( + 127, + '/bin/sh: ./mvnw: /usr/bin/env: bad interpreter: No such file or directory', + ); + expect(crlf127.note).toContain('infrastructure evidence'); + + // bash >= 5.2 reports the same death with new wording. + const bash52 = runWith( + 127, + '/bin/sh: line 1: ./mvnw: cannot execute: required file not found', + ); + expect(bash52.note).toContain('infrastructure evidence'); + + // dash's bare wording. + const dash = runWith(127, 'sh: ./mvnw: not found'); + expect(dash.note).toContain('infrastructure evidence'); + + // A CRLF `#!/usr/bin/env sh` shebang names env, not the wrapper. + const envCrlf = runWith( + 127, + "/usr/bin/env: 'sh\\r': No such file or directory", + ); + expect(envCrlf.note).toContain('infrastructure evidence'); + }, + ); + + it('does not file a dependency failure as infrastructure when the diff changed build inputs', () => { + writeReactor(); + const output = + '[ERROR] Could not resolve dependencies for project example:core'; + + for (const changed of ['pom.xml', '.mvn/maven.config', 'core/pom.xml']) { + const report = runAdapter([changed], { + exec: (command) => result(command, { exitCode: 1, output }), + }); + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + } + + // No executable wrapper exists on disk, so the run used the system + // `mvn`: a wrapper file this run never executed cannot have caused its + // resolution failure, so the carve-out stays — with a disclosure. Only + // this platform's wrapper is exercised: a change confined to the OTHER + // platform's wrapper leaves no Maven target to run at all (see the + // other-platform-wrapper test below). + const platformWrapper = process.platform === 'win32' ? 'mvnw.cmd' : 'mvnw'; + for (const changed of [platformWrapper]) { + const report = runAdapter([changed], { + exec: (command) => result(command, { exitCode: 1, output }), + }); + expect(report.note).toContain('infrastructure evidence'); + expect(report.note).toContain('wrapper change itself was not exercised'); + } + }); + + it('does not treat an inner permission error as a wrapper startup failure', () => { + writeReactor(); + writeWrapper(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: 'Failed to write target/generated.txt: Permission denied', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('keeps dependency resolution classified as infrastructure after fresh reports', () => { + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:extension', + }); + }, + }); + + expect(report.test[0]?.output).toContain('[maven-test-report]'); + expect(report.note).toContain('infrastructure evidence'); + }); + + it('keeps fresh failing tests as source evidence despite infrastructure words', () => { + // The output is Maven-FRAMED: absent the fresh-failure guard it WOULD + // classify as infrastructure, so the assertions genuinely pin the + // precedence of fresh failing XML over the dependency carve-out. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:extension', + }); + }, + }); + + expect(report.test[0]?.output).toContain('[maven-test-failure]'); + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it('treats exit 0 with fresh failing reports as a failure, not a pass', () => { + // surefire `testFailureIgnore` (or -Dmaven.test.failure.ignore) lets + // `mvn test` exit 0 over failing tests; the verdict must read the XML. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.output).toContain('[maven-test-failure]'); + // The shape's own flag: none of the other verdict flags fire for an + // exit-0 run over fresh failing reports, and test-delta/test-plan's + // count mining filter on them. + expect(report.test[0]?.swallowedReports).toBe(true); + expect(report.note).toContain('exited 0'); + expect(report.note).toContain('test failures, not a pass'); + expect(report.note).not.toContain('Maven test passed'); + }); + + it('skips malformed report directories without aborting Maven', () => { + writeReactor(); + const reportPath = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(join(root, 'core', 'target'), { recursive: true }); + writeFileSync(reportPath, 'not a directory'); + const calls: string[] = []; + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toHaveLength(1); + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(true); + expect(report.test[0]?.output).not.toContain('[maven-test-report]'); + }); + + it('ignores stale XML and appends fresh module-qualified Surefire and Failsafe summaries', () => { + writeReactor(); + const staleDir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(staleDir, { recursive: true }); + const stale = join(staleDir, 'TEST-Stale.xml'); + writeFileSync( + stale, + '', + ); + utimesSync(stale, new Date(1_000), new Date(1_000)); + + const report = runAdapter( + ['core/src/Main.java', 'extension/src/Extension.java'], + { + exec: (command) => { + const coreDir = join(root, 'core', 'target', 'surefire-reports'); + const extensionDir = join( + root, + 'extension', + 'target', + 'failsafe-reports', + ); + mkdirSync(coreDir, { recursive: true }); + mkdirSync(extensionDir, { recursive: true }); + writeFileSync( + join(coreDir, 'TEST-SameTest.xml'), + '', + ); + writeFileSync( + join(extensionDir, 'TEST-SameTest.xml'), + '', + ); + return result(command, { + exitCode: 1, + output: '[ERROR] Tests failed', + }); + }, + }, + ); + + const output = report.test[0]?.output ?? ''; + expect(output).not.toContain('TEST-Stale.xml'); + expect(output).toContain( + '[maven-test-report] core (1 failing report(s)): tests=2, failures=1, errors=0, skipped=0', + ); + expect(output).toContain( + '[maven-test-failure] core/target/surefire-reports/TEST-SameTest.xml: example.SameTest#coreFailure', + ); + // The clean rollup carries the per-report CLAMPED passed total + // (3 tests - 1 skipped), not the raw pre-aggregated Σtests/Σskipped: + // test-plan parses counts per line with its own clamp, and raw totals + // would parse to a different count than the per-report truth. + expect(output).toContain( + '[maven-test-report] extension (1 report(s)): tests=2, failures=0, errors=0, skipped=0', + ); + expect(output).not.toContain( + 'extension/target/failsafe-reports/TEST-SameTest.xml', + ); + expect(report.note).toContain('module-qualified'); + }); + + it('quotes exotic module selectors for the platform shell', () => { + // Plain selectors stay bare; anything else is quoted for the shell the + // command actually runs under — POSIX quoting is literal in cmd.exe. + expect(shellSelector(['core', 'extension'])).toBe('core,extension'); + expect(shellSelector(['my module'], 'linux')).toBe("'my module'"); + expect(shellSelector(['my module'], 'win32')).toBe('"my module"'); + }); + + it.each([ + // `,` separates `-pl` arguments and `:` makes Maven read the selector as + // `[groupId]:artifactId` coordinates instead of a path: both change what + // the selector MEANS, so quoting cannot rescue them. + ['a,b'], + ['a:b'], + // cmd.exe expands %VAR% even inside `"…"`. + ['a%b'], + // A leading `-` is re-read as an option by commons-cli (`-pl -rf` dies + // with 'Missing argument for option: pl'); a leading `!` is Maven's + // exclusion operator — quoting preserves the bytes, not the semantics. + ['-rf'], + ['!foo'], + ])('refuses a selector it cannot express for %s', (module) => { + // These are directory names read off disk now, not entries a POM parser + // pre-filtered — the gate has to live in the selector itself. + expect(shellSelector([module], 'linux')).toBeNull(); + expect(shellSelector([module], 'win32')).toBeNull(); + }); + + it('widens to the full reactor for a module a selector cannot carry', () => { + // Failing closed to the whole reactor is the safe direction: it verifies + // more than asked, where a mis-quoted selector verifies the wrong thing. + writeProject('.', ['od,d']); + writeProject('od,d'); + const calls: string[] = []; + + const report = runAdapter(['od,d/src/main/java/Main.java'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.affected).toEqual(['.']); + expect(report.note).toContain('cannot express'); + }); + + it('runs the whole reactor for a module POM change', () => { + // A POM is parent config for everything that aggregates or inherits it. + // This adapter models none of those edges — Maven applies the real ones + // inside the command — so the scope widens instead of guessing a closure. + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['nested-parent/pom.xml'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.affected).toEqual(['.']); + }); + + it('reports unsupported when Maven rejects the selected project', () => { + // The out-of-reactor / profile-inactive answer comes from Maven, which + // evaluates profile activation, `` inheritance, and the current + // JDK, and rejects an unknown selector before compiling anything. Nothing + // here re-derives that from the POM text. + writeProject('.', ['core']); + writeProject('core'); + // On disk but absent from the reactor Maven actually assembles. + writeProject('admin'); + + const report = runAdapter(['admin/src/main/java/Admin.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not find the selected project in the reactor: admin\n', + }), + }); + + // `unsupported` is the structured handoff, not a failed build: it carries + // no build/test evidence for a verdict to read. + expect(report.toolchain).toBe('unsupported'); + expect(report.build).toEqual([]); + expect(report.test).toEqual([]); + expect(report.note).toContain('admin'); + expect(report.note).toContain('profile-inactive'); + }); + + it('collects fresh reports from a project directory nothing enumerated', () => { + // The report sweep walks the worktree instead of a list of reactor + // projects: which projects are active is Maven's answer, and a report + // directory only exists where Maven actually ran. + writeProject('.', ['core']); + writeProject('core'); + const reports = join( + root, + 'core', + 'generated-child', + 'target', + 'surefire-reports', + ); + mkdirSync(reports, { recursive: true }); + + const report = runAdapter(['core/src/main/java/Main.java'], { + exec: (command) => { + writeFileSync( + join(reports, 'TEST-child.GenTest.xml'), + '', + ); + return result(command); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.output).toContain('core/generated-child'); + expect(report.test[0]?.output).toContain('tests=4'); + }); + + it('selects the wrapper a platform can execute', () => { + writeProject('.'); + writeFileSync(join(root, 'mvnw'), '#!/bin/sh\n'); + chmodSync(join(root, 'mvnw'), 0o755); + writeFileSync(join(root, 'mvnw.cmd'), '@echo off\n'); + + expect(mavenExecutable(root, 'linux')).toBe('./mvnw'); + expect(mavenExecutable(root, 'darwin')).toBe('./mvnw'); + expect(mavenExecutable(root, 'win32')).toBe('mvnw.cmd'); + + rmSync(join(root, 'mvnw')); + rmSync(join(root, 'mvnw.cmd')); + expect(mavenExecutable(root, 'linux')).toBe('mvn'); + expect(mavenExecutable(root, 'win32')).toBe('mvn'); + }); + + it.skipIf(process.platform === 'win32')( + 'falls back to mvn for a wrapper without the executable bit', + () => { + // A `core.fileMode=false` checkout commits mvnw mode 644; running it + // would die with exit 126 and zero verification, so prefer system mvn. + writeProject('.'); + writeFileSync(join(root, 'mvnw'), '#!/bin/sh\n'); + expect(mavenExecutable(root, 'linux')).toBe('mvn'); + + chmodSync(join(root, 'mvnw'), 0o755); + expect(mavenExecutable(root, 'linux')).toBe('./mvnw'); + }, + ); + + it('leaves repository metadata without Maven targets', () => { + writeReactor(); + + const metadata = [ + '.github/workflows/ci.yml', + '.gitignore', + '.gitattributes', + 'LICENSE', + 'CODEOWNERS', + '.editorconfig', + ]; + expect(detectMavenOwnership(root, metadata)).toEqual({ + reactorWide: false, + modules: [], + }); + + const calls: string[] = []; + const report = runAdapter(['.github/workflows/ci.yml'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(true); + expect(calls).toEqual([]); + }); + + it('rolls clean reports up per project dir and caps failing reports', () => { + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const coreDir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(coreDir, { recursive: true }); + for (let i = 0; i < 150; i++) { + writeFileSync( + join(coreDir, `TEST-Clean${i}.xml`), + '', + ); + } + for (let i = 0; i < 120; i++) { + writeFileSync( + join(coreDir, `TEST-Fail${i}.xml`), + '', + ); + } + return result(command, { exitCode: 1, output: '[ERROR] Tests failed' }); + }, + }); + + const output = report.test[0]?.output ?? ''; + // 150 clean reports become ONE rollup line, keeping the count shape + // test-plan parses; 270 per-report lines would have bypassed the trim. + expect(output).toContain( + '[maven-test-report] core (150 report(s)): tests=300, failures=0, errors=0, skipped=0', + ); + expect(output).not.toContain('TEST-Clean0.xml'); + // Failing reports roll up per PROJECT like the clean side — the + // per-report lines' byte-order cap lost module attribution for + // everything past the bound. The per-report case markers survive (they + // are the module-qualified failure evidence); the COUNT line is the + // rollup. + expect(output).toContain( + '[maven-test-report] core (120 failing report(s)): tests=120, failures=120, errors=0, skipped=0', + ); + expect(output).toContain( + '[maven-test-failure] core/target/surefire-reports/TEST-Fail0.xml: example.FailTest#fails', + ); + }); + + it('carries clamped passed totals in the clean omission marker', () => { + // An anomalous report (Surefire does not guarantee tests >= skipped) + // inside the omitted batch must not cancel the passed counts of its + // batchmates — clamp the aggregated totals and it cancels two. + const modules = Array.from({ length: 120 }, (_, i) => `mod${i}`); + writeProject('.', modules); + for (const module of modules) writeProject(module); + + const report = runAdapter(['mod0/src/main/java/Main.java'], { + exec: (command) => { + for (const module of modules) { + const dir = join(root, module, 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Clean.xml'), + // mod99 sorts into the omitted tail of the per-project rollups. + module === 'mod99' + ? '' + : '', + ); + } + return result(command); + }, + }); + + const output = report.test[0]?.output ?? ''; + expect(output).toContain( + '[maven-test-report] 20 more clean project rollup(s) omitted: ' + + 'tests=19, failures=0, errors=0, skipped=0', + ); + // 100 kept rollup lines pass one test each; the omitted batch passes + // 19. The second reading is the changed module's (`mod0`) subtotal — + // emitted beside the reactor-wide sum so a count claim scoped to the + // changed modules can settle on either. + expect(observedTestCounts(report)).toEqual([119, 1]); + }); + + it('carries clamped passed totals in the failing omission marker', () => { + const modules = Array.from({ length: 103 }, (_, i) => `mod${i}`); + writeProject('.', modules); + for (const module of modules) writeProject(module); + + const report = runAdapter(['mod0/src/main/java/Main.java'], { + exec: (command) => { + for (const module of modules) { + const dir = join(root, module, 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Fail.xml'), + // mod99 sorts into the omitted tail and passes zero despite + // recording a test; its batchmates each pass one. + module === 'mod99' + ? '' + : '', + ); + } + return result(command, { exitCode: 1, output: '[ERROR] Tests failed' }); + }, + }); + + const output = report.test[0]?.output ?? ''; + expect(output).toContain( + '[maven-test-report] 3 more failing project rollup(s) omitted: ' + + 'tests=2, failures=0, errors=0, skipped=0', + ); + // 100 kept failing rollups pass one test each; the omitted batch + // passes 2. The second reading is the changed module's (`mod0`) + // subtotal, emitted beside the reactor-wide sum. + expect(observedTestCounts(report)).toEqual([102, 1]); + }); + + it('caps the clean per-project rollup lines', () => { + const modules = Array.from({ length: 120 }, (_, i) => `mod${i}`); + writeProject('.', modules); + for (const module of modules) writeProject(module); + + const report = runAdapter(['mod0/src/main/java/Main.java'], { + exec: (command) => { + for (const module of modules) { + const dir = join(root, module, 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Clean.xml'), + '', + ); + } + return result(command); + }, + }); + + const output = report.test[0]?.output ?? ''; + expect( + output.match(/\[maven-test-report\] mod\d+ \(1 report\(s\)\)/g), + ).toHaveLength(100); + expect(output).toContain( + '[maven-test-report] 20 more clean project rollup(s) omitted: ' + + 'tests=20, failures=0, errors=0, skipped=0', + ); + // The green note is the only test-count evidence on a passing Maven run; + // its totals are computed BEFORE the cap, over all 120 reports. + expect(report.note).toContain( + 'Maven test passed with fresh reports: 120 tests', + ); + }); + + it('caps failing case lines', () => { + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const coreDir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(coreDir, { recursive: true }); + const cases = Array.from( + { length: 250 }, + (_, i) => + ``, + ).join(''); + writeFileSync( + join(coreDir, 'TEST-Big.xml'), + `${cases}`, + ); + return result(command, { exitCode: 1, output: '[ERROR] Tests failed' }); + }, + }); + + const output = report.test[0]?.output ?? ''; + expect(output).toContain( + '[maven-test-failure] 50 more failing case(s) omitted', + ); + expect(output.match(/\[maven-test-failure\] core\//g)).toHaveLength(200); + }); + + it('treats unframed network words as source evidence, and Maven-framed ones as infrastructure', () => { + writeReactor(); + + const unframed = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: 'java.net.ConnectException: Connection refused', + }), + }); + expect(unframed.note).toContain('Correlate compiler or test errors'); + expect(unframed.note).not.toContain('infrastructure evidence'); + + const framed = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Failed to execute goal on project core: Could not transfer artifact org.example:dep:jar:1: Connection refused', + }), + }); + expect(framed.note).toContain('infrastructure evidence'); + }); + + it('does not launder a compile failure into infrastructure when dependency words share the output', () => { + // A flaky mirror, or an upstream module pulled in by `-am`, can put one + // `[ERROR] Could not transfer artifact` line in the same output as a + // real compile error. The compile failure writes no Surefire XML, so + // only the source markers keep it from reading as infrastructure. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: [ + '[ERROR] Could not transfer artifact org.foo:bar:jar:1.0 from/to central: Connection timed out', + '[ERROR] COMPILATION ERROR :', + '[ERROR] /tmp/x/core/src/main/java/Main.java:[12,5] cannot find symbol', + ].join('\n'), + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it('keeps source-failure markers framed — unframed words stay infrastructure', () => { + writeReactor(); + + const runWith = (output: string) => + runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode: 1, output }), + }); + + const dependencyLine = + '[ERROR] Could not resolve dependencies for project example:core'; + + // Every Maven-framed marker outranks the dependency carve-out... + for (const marker of [ + '[ERROR] COMPILATION ERROR :', + '[ERROR] /tmp/x/core/src/main/java/Main.java:[12,5] cannot find symbol', + '[ERROR] There are test failures.', + ]) { + const report = runWith(`${dependencyLine}\n${marker}`); + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + } + + // ...but the same words in a test's own stdout do not. + const unframed = runWith(`${dependencyLine}\nCOMPILATION ERROR`); + expect(unframed.note).toContain('infrastructure evidence'); + expect(unframed.test[0]).toMatchObject({ infrastructure: true }); + }); + + it('classifies a spawn-level death without an exit code as infrastructure', () => { + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode: null, output: '' }), + }); + + expect(report.ok).toBe(false); + expect(report.timedOut).toEqual([]); + expect(report.note).toContain('without an exit code'); + expect(report.note).toContain('infrastructure evidence'); + }); + + it('discloses successful tests without fresh XML', () => { + writeReactor(); + const report = runAdapter(['core/src/Main.java']); + + expect(report.ok).toBe(true); + expect(report.note).toContain('no fresh Surefire/Failsafe XML'); + }); + + it('keeps fresh failing reports as test evidence when the run times out', () => { + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + timeout: 2, + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { exitCode: null, timedOut: true, seconds: 2 }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.note).toContain('ran out of time'); + expect(report.note).toContain('1 failure(s) and 1 error(s)'); + expect(report.note).toContain('treat those as test failures'); + expect(report.note).not.toContain('not a defect in the diff'); + expect(report.test[0]?.output).toContain('[maven-test-failure]'); + }); + + it('keeps fresh failing reports as test evidence when the run dies without an exit code', () => { + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { exitCode: null }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.note).toContain('ended without an exit code'); + expect(report.note).toContain('1 failure(s) and 0 error(s)'); + expect(report.note).toContain('treat those as test failures'); + expect(report.note).not.toContain( + 'This is infrastructure evidence, not a source finding.', + ); + }); + + it.skipIf(process.platform === 'win32')( + 'does not classify a launch failure as infrastructure when the wrapper changed', + () => { + // On win32 `mvnw` is the other platform's wrapper and is skipped by + // ownership, so the adapter sees no Maven target to run. + + // The PR's own wrapper edit may be what broke startup; the pinned intent + // (changed-wrapper failures are never environmental) covers the + // launch-failure disjunct too, not just the 126/127 wrapper one. + writeReactor(); + writeWrapper(); + + const report = runAdapter(['mvnw'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + 'Error: The JAVA_HOME environment variable is not defined correctly', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'does not launder a PR-caused fallback launch failure into infrastructure', + () => { + // The diff drops the wrapper's executable bit; mavenExecutable falls + // back to system mvn, executedWrapper is null, and a runner without + // system Maven dies 127. That death is the diff's own doing — filing + // it as infrastructure would let a PR that broke the build ship with + // no finding. + writeReactor(); + writeFileSync(join(root, 'mvnw'), '#!/bin/sh\n'); // no executable bit + + const report = runAdapter(['mvnw'], { + exec: (command) => + result(command, { exitCode: 127, output: 'sh: 1: mvn: not found' }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }, + ); + + it.skipIf(process.platform === 'win32')( + 'discloses when a changed wrapper falls back to the system mvn', + () => { + // On win32 `mvnw` is dropped by ownership, only the Java source is + // owned, and the run narrows to `-pl core -am test` instead of the + // reactor-wide command pinned here. + + // No executable bit: mavenExecutable falls back to system mvn, so the + // wrapper the diff changes is never executed — the run must say so. + writeReactor(); + writeFileSync(join(root, 'mvnw'), '#!/bin/sh\n'); + const calls: string[] = []; + + const report = runAdapter( + ['mvnw', 'core/src/main/java/example/Core.java'], + { + exec: (command) => { + calls.push(command); + return result(command); + }, + }, + ); + + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.note).toContain('wrapper change itself was not exercised'); + // The fallback is a green run: keying neverRan's wrapper disjunct on + // ANY changed wrapper (instead of the executed one) would read it as + // never run and fail it. + expect(report.ok).toBe(true); + expect(report.test[0]?.neverRan).toBeUndefined(); + }, + ); + + it('does not treat a test-fixture POM as a dependency input', () => { + // A fixture pom.xml under a module's src/ tree cannot change the reactor's + // dependency resolution; a genuine outage there stays infrastructure. + writeReactor(); + writeProject('core/src/test/resources/projects/sample'); + + const report = runAdapter( + ['core/src/test/resources/projects/sample/pom.xml'], + { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }, + ); + + expect(report.note).toContain('infrastructure evidence'); + }); + + it('reports insufficient disk space instead of running Maven on a full disk', () => { + statfsSyncMock.mockReturnValue({ bavail: 5.4e8, bsize: 1 }); // ~0.5G free + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.ok).toBe(false); + expect(report.build).toEqual([]); + expect(report.test).toEqual([]); + expect(calls).toEqual([]); + expect(report.note).toContain('Insufficient disk space'); + }); + + it('re-checks the disk floor after the warm-up, before the lifecycle', () => { + // The warm-up is the phase that fills the disk: a cold reactor's + // dependency:go-offline can consume the headroom the preflight + // passed, and the lifecycle must not run on the now-full disk. + statfsSyncMock + .mockReturnValueOnce({ bavail: 16 * 1024 ** 3, bsize: 1 }) + .mockReturnValueOnce({ bavail: 0, bsize: 1 }); + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/src/Main.java'], { + timeout: 60, + install: true, + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + // Only the warm-up ran; the lifecycle was skipped and disclosed. + expect(calls).toHaveLength(1); + expect(calls[0]).toContain('dependency:go-offline'); + expect(report.ok).toBe(false); + expect(report.build).toEqual([]); + expect(report.test).toEqual([]); + expect(report.note).toContain('Insufficient disk space'); + expect(report.note).toContain('warm-up'); + }); + + it('attributes failures to the failing cases in declaration order', () => { + // Surefire writes passing cases self-closing, in execution order: a + // passing case must not absorb the following case's failure, and the + // real failing cases must be the ones reported. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '' + + '' + + '' + + '', + ); + return result(command, { exitCode: 1, output: '[ERROR] Tests failed' }); + }, + }); + + const output = report.test[0]?.output ?? ''; + expect(output).toContain('example.CoreTest#beta'); + expect(output).toContain('example.CoreTest#delta'); + expect(output).not.toContain('CoreTest#alpha'); + expect(output).not.toContain('CoreTest#gamma'); + }); + + it('keeps counts and identity when attribute values carry `>`', () => { + // A \`>\` is legal unescaped inside a quoted XML attribute value — + // parameterized-test and @DisplayName names carry them. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '', + ); + return result(command, { exitCode: 1, output: '[ERROR] Tests failed' }); + }, + }); + + expect(report.ok).toBe(false); + const output = report.test[0]?.output ?? ''; + expect(output).toContain('tests=1, failures=1, errors=0, skipped=0'); + expect(output).toContain('example.T#fails [x > y]'); + }); + + it('aggregates every suite in one report file', () => { + // Aggregate JUnit writers (jest-junit, karma) emit several + // elements per file; reading only the first undercounts later suites' + // failures to zero and discards the failing cases. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Aggregate.xml'), + '' + + '' + + '' + + '' + + '' + + '' + + '', + ); + return result(command); + }, + }); + + expect(report.ok).toBe(false); + const output = report.test[0]?.output ?? ''; + expect(output).toContain('tests=2, failures=1, errors=0, skipped=0'); + expect(output).toContain('example.Two#fails'); + expect(report.note).toContain('exited 0'); + }); + + it('ignores oversized report files rather than parsing them', () => { + // Evidence files are PR-controlled: the size cap keeps a multi-megabyte + // file from burning the outer deadline, at the cost of its evidence. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Huge.xml'), + '' + + ' { + // A fail-never setting (-fn/--fail-never) makes Maven exit 0 over a + // compilation failure; no Surefire XML exists for freshFailures to see. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-fn\n'); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[ERROR] COMPILATION ERROR :\n' + + '[ERROR] /x/core/src/main/java/example/Main.java:[12,5] cannot find symbol', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.test[0]?.infrastructure).toBeUndefined(); + expect(report.note).toContain('exited 0'); + expect(report.note).toContain('fail-never'); + expect(report.note).not.toContain('Maven test passed'); + }); + + it('classifies a fail-never dependency failure as infrastructure', () => { + // The same masking at the dependency phase — unless the diff changed + // the resolution inputs — stays environmental like the exit-1 form. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + expect(report.note).toContain('infrastructure evidence'); + expect(report.note).toContain('fail-never'); + }); + + it('keeps a fail-never dependency failure PR-attributed when the inputs changed', () => { + // The exit-0 half of the dependency carve-out exception: with resolution + // inputs changed, the swallowed failure stays a failed run — not green, + // and not laundered into an environmental result. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-fn\n'); + + const report = runAdapter(['core/pom.xml'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBeUndefined(); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.note).toContain('fail-never'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('keeps Kotlin compile failures source-attributed beside dependency words', () => { + writeReactor(); + + const report = runAdapter(['core/src/Main.kt'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not transfer artifact org.example:lib:pom:1 from central: Connection timed out\n' + + '[ERROR] Failed to execute goal org.jetbrains.kotlin:kotlin-maven-plugin:1.9.0:compile (default-compile) on project core: Compilation failure\n' + + '[ERROR] /x/core/src/main/kotlin/example/Main.kt: (12, 5): Unresolved reference: foo', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it('still builds the owning module for compilable files under a doc prefix', () => { + // The docs?/ prefix exempts documentation EXTENSIONS only: a .java file + // under doc/ is compilable input, not documentation. + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/README.md', 'core/doc/Helper.java'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.toolchain).toBe('maven'); + expect(report.affected).toEqual(['core']); + expect(calls).toEqual([ + 'mvn --batch-mode --no-transfer-progress -pl core -am test', + ]); + }); + + it('leaves module repository metadata without Maven targets', () => { + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/LICENSE', 'core/.gitignore'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(true); + expect(report.affected).toEqual([]); + expect(calls).toEqual([]); + }); + + it('treats settings referenced by .mvn/maven.config as dependency inputs', () => { + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-s\nsettings.xml\n'); + writeFileSync(join(root, 'settings.xml'), '\n'); + + const report = runAdapter(['settings.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('fails closed past the read cap for .mvn/maven.config', () => { + // The one PR-controlled read without a size cap: a config past the + // cap is treated like an unreadable one (its referenced locations + // unknown), while the config FILE itself stays a dependency input + // through the `.mvn/` prefix — so the suppression still stands. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync( + join(root, '.mvn', 'maven.config'), + `-s\nsettings.xml ${'x'.repeat(2 * 1024 * 1024)}\n`, + ); + writeFileSync(join(root, 'settings.xml'), '\n'); + + // Oversized: the settings reference is unknown, so a dependency + // outage over a changed settings.xml keeps the infrastructure + // carve-out... + const oversized = runAdapter(['settings.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + expect(oversized.note).toContain('infrastructure evidence'); + + // ...and under the cap the identical config suppresses it. + writeFileSync(join(root, '.mvn', 'maven.config'), '-s\nsettings.xml\n'); + const undersized = runAdapter(['settings.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + expect(undersized.note).toContain('Correlate compiler or test errors'); + expect(undersized.note).not.toContain('infrastructure evidence'); + }); + + it('leaves no Maven target when only the other platform wrapper changed', () => { + // POSIX executes ./mvnw, win32 mvnw.cmd; a change confined to the other + // platform's wrapper cannot affect this platform's run, so no reactor-wide + // run burns the deadline verifying nothing. + writeReactor(); + const win32 = process.platform === 'win32'; + const executed = win32 ? 'mvnw.cmd' : 'mvnw'; + const other = win32 ? 'mvnw' : 'mvnw.cmd'; + writeFileSync(join(root, executed), win32 ? '@echo off\n' : '#!/bin/sh\n'); + if (!win32) chmodSync(join(root, executed), 0o755); + writeFileSync(join(root, other), win32 ? '#!/bin/sh\n' : '@echo off\n'); + const calls: string[] = []; + + const report = runAdapter([other], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual([]); + expect(report.ok).toBe(true); + expect(report.note).toContain('no Maven target'); + + // The carve-out itself still holds in its reachable shape: when the diff + // ALSO changes module sources, the other platform's wrapper is not a + // resolution input and cannot suppress the dependency carve-out. + const mixed = runAdapter([other, 'core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + expect(mixed.note).toContain('infrastructure evidence'); + expect(mixed.note).toContain('wrapper change itself was not exercised'); + }); + + it('clamps negative report counts to zero', () => { + // A malformed failures="-3" must not cancel legitimate counts when + // totals roll up across reports. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '', + ); + return result(command); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.output).toContain( + '[maven-test-report] core (1 report(s)): tests=1, failures=0, errors=0, skipped=0', + ); + }); + + it('runs a dependency warm-up with its own deadline when installing', () => { + // A review worktree is cold by construction; without the warm-up the + // cold resolve shares the single lifecycle deadline with compilation + // and the tests. The warm-up runs first, narrowed to the same scope, + // and its result is recorded as the report's install. + writeReactor(); + const calls: Array<[string, number]> = []; + + const report = mavenToolchainAdapter.run({ + root, + changedFiles: ['core/src/main/java/example/Core.java'], + timeout: 9, + // A budget above the two 9s deadlines keeps them whole; the budget + // regime itself is pinned by the tests below. + budget: 600, + install: true, + exec: (command, _cwd, timeoutMs) => { + calls.push([command, timeoutMs]); + return result(command); + }, + }); + + expect(calls).toEqual([ + [ + 'mvn --batch-mode --no-transfer-progress -pl core -am dependency:go-offline -q', + 9_000, + ], + ['mvn --batch-mode --no-transfer-progress -pl core -am test', 9_000], + ]); + expect(report.install?.command).toContain('dependency:go-offline'); + expect(report.ok).toBe(true); + expect(report.note).not.toContain('Dependency warm-up'); + }); + + it('keeps module attribution for failing projects past the rollup cap', () => { + // The omitted-failing-rollup marker used to zero the failure counts of + // every project past the cap; when the case-line cap ALSO dropped the + // claimed module's lines, both attribution channels went dark and the + // `-am` carve-out discarded a run that failed inside the claim. Each + // omitted project keeps one module-prefixed failure marker. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + for (let i = 0; i < 101; i++) { + const module = `m${String(i).padStart(3, '0')}`; + const dir = join(root, module, 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Fail.xml'), + '' + + `` + + '', + ); + } + return result(command, { exitCode: 1 }); + }, + }); + + const output = report.test[0]?.output ?? ''; + // m100 sorts last in byte order and sits past the 100-project rollup + // cap — its failure attribution survives as a module-prefixed marker. + expect(output).toContain('[maven-test-failure] m100/target/'); + expect(output).toContain('1 more failing project rollup(s) omitted'); + expect(report.ok).toBe(false); + }, 30_000); + + it('keeps the lifecycle verdict when the warm-up fails or times out', () => { + // The warm-up is best-effort: a partial local repository is + // content-addressed and resumable (unlike a partial node_modules), so + // no warm-up outcome may block the lifecycle run or change its verdict. + writeReactor(); + + const timedOut = mavenToolchainAdapter.run({ + root, + changedFiles: ['core/src/Main.java'], + timeout: 5, + // Keep both 5s deadlines whole despite the warm-up's wall time. + budget: 600, + install: true, + exec: (command, _cwd, timeoutMs) => + command.includes('dependency:go-offline') + ? result(command, { + exitCode: null, + timedOut: true, + seconds: 5, + deadlineMs: timeoutMs, + }) + : result(command), + }); + expect(timedOut.ok).toBe(true); + expect(timedOut.test).toHaveLength(1); + // The report-level list names EVERY command killed by its deadline — + // the warm-up too, like the npm adapter's install command. + expect( + timedOut.timedOut.some((c) => c.includes('dependency:go-offline')), + ).toBe(true); + expect(timedOut.note).toContain('Dependency warm-up'); + expect(timedOut.note).toContain('ran out of time (5s)'); + + const failed = runAdapter(['core/src/Main.java'], { + budget: 600, + install: true, + exec: (command) => + command.includes('dependency:go-offline') + ? result(command, { exitCode: 1 }) + : result(command), + }); + expect(failed.ok).toBe(true); + expect(failed.note).toContain('Dependency warm-up'); + expect(failed.note).toContain('exited 1'); + + // A spawn death without a deadline is its own arm: the note must not + // read "ran out of time" for a warm-up the deadline never touched. + const spawnDied = runAdapter(['core/src/Main.java'], { + budget: 600, + install: true, + exec: (command) => + command.includes('dependency:go-offline') + ? result(command, { exitCode: null }) + : result(command), + }); + expect(spawnDied.ok).toBe(true); + expect(spawnDied.note).toContain('Dependency warm-up'); + expect(spawnDied.note).toContain('ended without an exit code'); + }); + + it('widens to the full reactor when the -pl selector exceeds the launch-safe length', () => { + // A wide diff selects many modules at once; on large reactors the + // comma-joined selector approaches cmd.exe's 8191-character line limit, + // so past the cap the run widens to the full reactor instead of shipping + // a command line the platform may refuse to launch. + const leaves = Array.from( + { length: 100 }, + (_, i) => + `module-with-a-rather-long-directory-name-${String(i).padStart(2, '0')}`, + ); + writeProject('.', ['agg']); + writeProject('agg', leaves); + for (const leaf of leaves) writeProject(`agg/${leaf}`); + const calls: string[] = []; + + const report = mavenToolchainAdapter.run({ + root, + // Source files, not POMs: a POM change is reactor-wide on its own, and + // this case must reach the selector-length guard instead. + changedFiles: leaves.map((leaf) => `agg/${leaf}/src/main/java/Main.java`), + timeout: 5, + install: false, + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.affected).toEqual(['.']); + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.note).toContain('selector exceeded 4096 characters'); + expect(report.note).toContain('full reactor'); + }); + + it('does not launder a test-printed launch diagnostic into infrastructure', () => { + // Unframed launch words count only in the prelude before Maven's own + // output starts: once a Maven-framed line has appeared, a test printing + // `mvn: command not found` in its stdout must not mask a source failure. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: [ + '[INFO] Scanning for projects...', + '[INFO] --- surefire:test ---', + 'sh: 1: mvn: not found', + ].join('\n'), + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it('treats .mvn/wrapper configuration as part of the wrapper', () => { + // maven-wrapper.properties names the distribution ./mvnw downloads and + // executes; a diff touching it controls what the wrapper runs exactly + // as one touching the script does, so the startup failure is the + // diff's to answer for, not the environment's. + writeReactor(); + writeExecutedWrapper(); + + const report = runAdapter(['.mvn/wrapper/maven-wrapper.properties'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + 'Error: The JAVA_HOME environment variable is not defined correctly', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it('builds the owning module for resource-like text files outside doc locations', () => { + // A .txt is only exempted at doc-shaped locations: a resource wired + // into the artifact via maven-resources-plugin (which points at + // arbitrary dirs) must keep the build instead of silently skipping it. + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/config/messages.txt'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.affected).toEqual(['core']); + expect(calls).toEqual([ + 'mvn --batch-mode --no-transfer-progress -pl core -am test', + ]); + }); + + it('still exempts doc-extension files at the module top level and in site/', () => { + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/notes.txt', 'core/site/index.rst'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(report.ok).toBe(true); + expect(report.affected).toEqual([]); + expect(calls).toEqual([]); + }); + + it('does not parse CDATA-wrapped report content as markup', () => { + // `` CDATA is the standard vehicle for test output that + // itself contains XML; scanning it as markup fabricated phantom suites + // and failure evidence for a passing one-test suite. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '' + + '' + + ']]>' + + '', + ); + return result(command); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.output).toContain('tests=1, failures=0'); + expect(report.test[0]?.output).not.toContain('[maven-test-failure]'); + }); + + it('files a resolution failure against a diff that deleted a POM', () => { + // `legacy/pom.xml` exists only in the diff — the shape a deletion leaves. + // Deleting a POM is one of the likeliest ways a diff breaks resolution + // (`Non-resolvable parent POM`, a module Maven can no longer read), and + // deciding WHICH deleted POMs could have caused THIS failure needs the + // effective model this adapter deliberately does not carry. So any + // changed POM withdraws the infrastructure carve-out: over-attributing + // costs a visible failure carrying Maven's own output, while + // under-attributing ships the diff's own breakage as someone else's + // outage. + writeReactor(); + + const report = runAdapter(['core/src/Main.java', 'legacy/pom.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).not.toContain('infrastructure evidence'); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it('parses a report of unterminated openers in linear time', () => { + // The quadratic pre-fix regex scan spent seconds per 256 KiB of + // never-closed ` { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '' + + '` never comes: the interrupted + // header walk discards every later body, so the report is rejected and + // the run fails closed instead of reading the surviving prefix green. + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }, 20_000); + + it('discloses sampling past the fresh-report parse cap instead of failing a green run', () => { + // The mtime freshness filter accepts any writer, so the PR's own + // tests control how many reports exist at parse time. Surefire + // writes one report per test CLASS, so a reactor-wide run on a large + // reactor produces more fresh reports than the parse cap — and the + // parsed reports are still real evidence: failing closed over the + // unread remainder read a fully green run as an uncertified failure + // and ruled every Test Plan claim unchecked. The cap now discloses + // the sampling and the green verdict stands. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + for (let i = 0; i < 1005; i++) { + writeFileSync( + join(dir, `TEST-Case${String(i).padStart(4, '0')}.xml`), + '' + + `` + + '', + ); + } + return result(command); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.evidenceCapped).toBeUndefined(); + expect(report.note).toContain('Evidence sampled: 5 fresh'); + expect(report.note).toContain('1000-report parse cap'); + expect(report.test[0]?.output).toContain( + '5 more fresh report(s) not parsed', + ); + expect(report.test[0]?.output).toContain( + '1000-report evidence cap was reached', + ); + }, 30_000); + + it('caps the failing cases one report accumulates, and counts the drop', () => { + // One report can carry tens of thousands of failing `` + // entries; the parse caps them while building, and the omission + // marker accounts for the drop instead of silently losing it. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + const cases = Array.from( + { length: 250 }, + (_, i) => + ``, + ).join(''); + writeFileSync( + join(dir, 'TEST-Bulk.xml'), + `${cases}`, + ); + return result(command); + }, + }); + + expect(report.ok).toBe(false); + const output = report.test[0]?.output ?? ''; + expect(output).toContain('tests=250, failures=250'); + expect(output).toContain('50 more failing case(s) omitted'); + // The kept case lines stop at the display cap. + expect(output.match(/\[maven-test-failure\] core\/target/g)?.length).toBe( + 200, + ); + }, 30_000); + + it('parses a suite header of unpaired attribute-name runs in linear time', () => { + // `xmlAttributes` backtracked quadratically on a long attribute-name + // run with no `=` — the same denial-of-service class, entering through + // the suite header instead of the testcase walk. + writeReactor(); + const startedAt = Date.now(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + `', + ); + return result(command); + }, + }); + + expect(Date.now() - startedAt).toBeLessThan(5_000); + expect(report.ok).toBe(true); + }, 20_000); + it('walks stacked openers-before-closers reports in linear time', () => { + // Every opener preceding every closer used to re-find the same early + // closing tag for each later opener — quadratic inside the 2 MiB cap, + // and still reporting ok:true while burning the outer deadline. Bodies + // are consumed forward-only now, so the walk stays O(n). + writeReactor(); + const startedAt = Date.now(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + ''.repeat(30_000) + + ''.repeat(30_000) + + '', + ); + return result(command); + }, + }); + + expect(Date.now() - startedAt).toBeLessThan(5_000); + expect(report.ok).toBe(true); + }, 20_000); + + it('treats attached -s settings as dependency inputs too', () => { + // commons-cli accepts the attached short form (`-sci/settings.xml`); + // missing it laundered a PR-caused resolution break into + // infrastructure. + writeReactor(); + mkdirSync(join(root, '.mvn')); + mkdirSync(join(root, 'ci')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-sci/settings.xml\n'); + writeFileSync(join(root, 'ci', 'settings.xml'), '\n'); + + const report = runAdapter(['ci/settings.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('quotes the deadline the lifecycle actually ran under in its timeout note', () => { + // The warm-up spends shared budget first, so the lifecycle fires a + // shorter deadline than the --timeout flag; the note must quote the + // number that fired, not the flag default. + writeReactor(); + let clock = 0; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock); + + try { + const report = runAdapter(['core/src/Main.java'], { + timeout: 300, + budget: 60, + install: true, + exec: (command, _cwd, timeoutMs) => { + clock += 45_000; + return command.includes('dependency:go-offline') + ? result(command, { deadlineMs: timeoutMs }) + : result(command, { + exitCode: null, + timedOut: true, + seconds: 15, + deadlineMs: timeoutMs, + }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.note).toContain('ran out of time (15s)'); + expect(report.note).not.toContain('ran out of time (300s)'); + } finally { + nowSpy.mockRestore(); + } + }); + + it('spends the whole-call budget across warm-up and lifecycle', () => { + // The warm-up and the lifecycle command share one budget: each gets + // the smaller of its own deadline and what remains, and --budget + // shortens the sum (both execs would otherwise take the full 300s). + writeReactor(); + let clock = 0; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock); + const calls: Array<[string, number]> = []; + + try { + const report = runAdapter(['core/src/Main.java'], { + timeout: 300, + budget: 60, + install: true, + exec: (command, _cwd, timeoutMs) => { + calls.push([command, timeoutMs]); + clock += 40_000; + return result(command); + }, + }); + + expect(calls).toEqual([ + [ + 'mvn --batch-mode --no-transfer-progress -pl core -am dependency:go-offline -q', + 60_000, + ], + ['mvn --batch-mode --no-transfer-progress -pl core -am test', 20_000], + ]); + expect(report.ok).toBe(true); + } finally { + nowSpy.mockRestore(); + } + }); + + it('discloses instead of attempting a lifecycle below the attempt floor', () => { + // A warm-up that spends the budget leaves less than the 15s floor for + // the lifecycle: an "attempt" would manufacture a fake timeout, so the + // run discloses that nothing could be built or tested. + writeReactor(); + let clock = 0; + const nowSpy = vi.spyOn(Date, 'now').mockImplementation(() => clock); + const calls: string[] = []; + + try { + const report = runAdapter(['core/src/Main.java'], { + timeout: 300, + budget: 60, + install: true, + exec: (command, _cwd, timeoutMs) => { + calls.push(command); + clock += 50_000; + // A cold reactor's warm-up really does eat the budget by timing + // out; the disclosure must survive the budget early-return. The + // recorded deadline mirrors the production exec: the note must + // quote the 60s that fired, not the 300s flag default. + return command.includes('dependency:go-offline') + ? result(command, { + exitCode: null, + timedOut: true, + seconds: 50, + deadlineMs: timeoutMs, + }) + : result(command); + }, + }); + + expect(calls).toEqual([ + 'mvn --batch-mode --no-transfer-progress -pl core -am dependency:go-offline -q', + ]); + expect(report.ok).toBe(false); + expect(report.test).toEqual([]); + expect(report.install?.command).toContain('dependency:go-offline'); + expect(report.note).toContain('whole-call budget (60s) was spent'); + expect(report.note).toContain('informational'); + expect(report.note).toContain('ran out of time (60s)'); + } finally { + nowSpy.mockRestore(); + } + }); + + it('runs nothing when the budget is below the attempt floor from the start', () => { + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/src/Main.java'], { + timeout: 300, + budget: 5, + install: true, + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual([]); + expect(report.ok).toBe(false); + expect(report.install).toBeNull(); + // Nothing ever ran, so the note must not claim the budget "was + // spent" — it names the floor the grant fell short of instead. + expect(report.note).toContain('granted budget (5s) is below the'); + expect(report.note).toContain('15s minimum'); + expect(report.note).not.toContain('was spent'); + }); + + it('applies the attempt floor to a --no-install run with no warm-up', () => { + // The disclosure check runs unconditionally after the warm-up block: a + // lifecycle-only run below the floor must not execute a sub-floor + // deadline and record the manufactured timeout as a real run. + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['core/src/Main.java'], { + timeout: 300, + budget: 5, + install: false, + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual([]); + expect(report.install).toBeNull(); + expect(report.ok).toBe(false); + expect(report.note).toContain('granted budget (5s) is below the'); + expect(report.note).toContain('15s minimum'); + }); + + it.each([ + ['[ERROR] Non-resolvable import POM for example:bom:1 at line 42'], + ['[ERROR] Failure to find example:core:jar:1 in central was cached'], + ['[ERROR] Could not find artifact example:core:jar:1 in central'], + ['[ERROR] Failed to collect dependencies at example:core:jar:1'], + ['[ERROR] Failed to read artifact descriptor for example:core:jar:1'], + ['[ERROR] Non-resolvable parent POM for example:core:1'], + [ + '[ERROR] org.apache.maven.plugin.dependency.PluginResolutionException: boom', + ], + [ + '[ERROR] org.eclipse.aether.resolution.DependencyResolutionException: boom', + ], + ["[ERROR] No plugin found for prefix 'jetty'"], + ['[ERROR] Unknown host: repo.maven.apache.org'], + ['[ERROR] Name or service not known'], + ['[ERROR] Temporary failure in name resolution'], + ['[ERROR] Connection reset'], + [ + '[ERROR] PKIX path building failed: unable to find valid certification path', + ], + ['[ERROR] status code: 401, reason phrase: Unauthorized'], + ['[ERROR] status code: 403, reason phrase: Forbidden'], + ['[ERROR] status code: 407, reason phrase: Proxy Authentication Required'], + ['[ERROR] status code: 429, reason phrase: Too Many Requests'], + ['[ERROR] status code: 503, reason phrase: Service Unavailable'], + ])('classifies %s as a dependency failure', (line) => { + expect(isDependencyFailureLine(line)).toBe(true); + }); + + it('falls back to mvn for an EMPTY executable wrapper', () => { + // An empty ./mvnw passes the existence/exec-bit gates and exits 0 over + // a build that never started — the run would read green. + writeProject('.'); + writeFileSync(join(root, 'mvnw'), ''); + chmodSync(join(root, 'mvnw'), 0o755); + expect(mavenExecutable(root, 'linux')).toBe('mvn'); + + writeFileSync(join(root, 'mvnw'), '#!/bin/sh\n'); + expect(mavenExecutable(root, 'linux')).toBe('./mvnw'); + + // The win32 branch carries the same size gate on `mvnw.cmd`. + writeFileSync(join(root, 'mvnw.cmd'), ''); + expect(mavenExecutable(root, 'win32')).toBe('mvn'); + writeFileSync(join(root, 'mvnw.cmd'), '@echo off\n'); + expect(mavenExecutable(root, 'win32')).toBe('mvnw.cmd'); + }); + + it('reads a fail-never plugin goal failure as a swallowed failure', () => { + // Under fail-never Maven exits 0 over ANY failed goal, and only the + // compile/dependency/launch classes were recognized before: a + // checkstyle goal failure read green. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-fn\n'); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[INFO] BUILD SUCCESS\n' + + '[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:check (validate) on project core: You have 1 Checkstyle violation.', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.note).toContain('fail-never'); + }); + + it('keeps a swallowed dependency goal failure infrastructure', () => { + // `Failed to execute goal on project …` matches the goal framing too; + // a dependency-class death must keep its acquisition carve-out. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[ERROR] Failed to execute goal on project core: Could not resolve dependencies for project example:core:jar:1', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + expect(report.note).toContain('infrastructure evidence'); + }); + + it('does not read forged framing as swallowed failure over green fresh reports', () => { + // Surefire echoes test stdout into the build output verbatim, so a + // fully green run whose test PRINTS an `[ERROR] Failed to execute goal` + // line used to flip swallowedFailure and read the run as failing. With + // green fresh reports and no fail-never setting, framed lines cannot be + // Maven's own — Maven prints no `[ERROR]` on success. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: + 'some test printed:\n' + + '[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:check on project fixture: boom\n' + + '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.swallowedFailure).toBeUndefined(); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + it('keeps fail-never swallowed failures failing beside green fresh reports', () => { + // The one setting that lets framed `[ERROR]` failures coexist with a + // green exit AND green reports: a multi-module run where an upstream + // module tested green and a later module's goal failure was swallowed. + // Detectable from `.mvn/maven.config`, so the defense stays off there. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-fn\n'); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: + '[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:check on project extension: boom\n' + + '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + }); + + it('folds rescue overflow into evidenceCapped', () => { + // The trim's rescue cap can drop failure-evidence lines before the + // adapter classifies the output: the same epistemic state as the + // fresh-report gaps — refuse to certify, never read green. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode: 0, rescueOverflow: true }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + expect(report.note).toContain('rescue cap'); + }); + + it('classifies a localized system-mvn launch death as infrastructure', () => { + // The shape arm covered only `./mvnw`; a system `mvn` launch death was + // classified exclusively by the English-only wording regexes — under a + // non-English LANG the environmental absence read as a source failure. + // No wrapper in this fixture: the platform falls back to system `mvn`. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 127, + output: 'mvn : commande introuvable', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + expect(report.note).toContain('infrastructure evidence'); + }); + + it('discloses the unscopable npm half of a mixed root', () => { + // npm's gate refused this root package.json (an unmodeled glob), so + // Maven was selected ALONE — the green run must not certify files no + // Maven module owns. + writeReactor(); + writeFileSync( + join(root, 'package.json'), + JSON.stringify({ name: 'frontend', workspaces: ['packages/**'] }), + ); + + const report = runAdapter(['core/src/Main.java']); + + expect(report.ok).toBe(true); + expect(report.note).toContain('files outside the Maven reactor'); + expect(report.note).toContain('were NOT verified'); + }); + + it('treats a changed path outside the worktree as unowned, never as reactor evidence', () => { + // The `../outside` escape the sandbox nesting above exists to contain: + // an out-of-worktree path must not reach owningProject, whose upward + // walk could find a pom.xml ABOVE the worktree and emit an + // out-of-worktree `-pl` selector. + writeReactor(); + writeProject('../outside'); + + expect( + detectMavenOwnership(root, ['../outside/src/main/java/Main.java']), + ).toEqual({ reactorWide: false, modules: [] }); + expect( + detectMavenOwnership(root, [ + 'core/../../outside/src/main/java/Main.java', + ]), + ).toEqual({ reactorWide: false, modules: [] }); + }); + + it('fails closed to reactor-wide for a real module nested under a src/ path', () => { + // The positive control for the root-collapse guard: a reactor can + // aggregate `src/core`, and `-pl .` would compile only + // the root — the changed module untested under a green verdict. + writeProject('.', ['src/core']); + writeProject('src/core'); + const calls: string[] = []; + + const report = runAdapter(['src/core/src/main/java/Foo.java'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.affected).toEqual(['.']); + }); + + it('reads a skip-tests setting as a swallowed failure, never a pass', () => { + // `-DskipTests` exits 0 having run ZERO tests, and Surefire's skip path + // emits no framed error and no XML — without the marker check the run + // was certified green over nothing. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: '[INFO] Tests are skipped.\n[INFO] BUILD SUCCESS', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.note).toContain('Tests are skipped.'); + expect(report.note).toContain('nothing was tested'); + }); + + it('classifies a wrapper distribution-download failure as infrastructure', () => { + // The canonical cold-worktree acquisition failure: the download dies + // before Maven's JVM starts, so the diagnostics are unframed and exit 1 + // — not the 126/127 wrapper-launch shapes. + writeReactor(); + writeWrapper(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + "wget: unable to resolve host address 'repo.maven.apache.org'\n", + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + expect(report.note).toContain('infrastructure evidence'); + }); + + it('classifies colored Maven output exactly like plain output', () => { + // `-Dstyle.color=always` interleaves SGR codes before the framed tokens; + // every classification predicate anchors on the framing, so the strip + // must happen before classification — colored bytes once laundered a + // failed compile under fail-never into a green verdict. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-fn\n'); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '\x1b[1;31m[ERROR]\x1b[m COMPILATION ERROR\n' + + '\x1b[1;31m[ERROR]\x1b[m /repo/core/src/Main.java:[3,5] cannot find symbol\n' + + '\x1b[1;32m[INFO]\x1b[m BUILD SUCCESS', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.note).toContain('fail-never'); + }); + + it('fails closed when the report sweep is truncated by a wide fan-out', () => { + // One directory holding more than MAX_DIR_ENTRIES entries used to cost + // an unbounded Dirent array AND truncated silently — a fresh failing + // report beyond the truncation point would read green. + writeReactor(); + const wide = join(root, 'wide'); + mkdirSync(wide); + for (let i = 0; i < 10_001; i++) { + writeFileSync(join(wide, `f${i}`), ''); + } + + const report = runAdapter(['core/src/Main.java']); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + expect(report.note).toContain('not certified as a pass'); + expect(report.test[0]?.output).toContain('the report sweep was truncated'); + }, 30_000); + + it('does not flag a directory read exactly to the entry cap as truncated', () => { + // The cap check used to run BEFORE the read, so a directory holding + // exactly MAX_DIR_ENTRIES entries — read to exhaustion — still flagged + // truncated, and the flag's propagation to evidenceCapped refused + // certification of a fully-green, fully-read run. + writeReactor(); + const wide = join(root, 'wide'); + mkdirSync(wide); + for (let i = 0; i < 10_000; i++) { + writeFileSync(join(wide, `f${i}`), ''); + } + + expect(reportPaths(root)).toEqual({ paths: [], truncated: false }); + }, 30_000); + + it('fails closed when a fresh report is too large to parse', () => { + // A masked exit 0 over one oversized failing report: the size cap + // rejects the parse, and the rejection must count as unknown evidence — + // not fail open where the count cap fails closed. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Big.xml'), + ``, + ); + return result(command, { + exitCode: 0, + output: '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + expect(report.test[0]?.output).toContain('could not be parsed'); + }); + + it('never follows a symlinked report directory out of the worktree', () => { + // The queue descent skips symlinks via Dirent.isDirectory(), but the + // direct report-dir listing resolves them: a symlinked surefire-reports + // once injected outside stale reports as fresh evidence. + writeReactor(); + const outside = join(sandbox, 'outside-reports'); + mkdirSync(outside); + writeFileSync( + join(outside, 'TEST-Stale.xml'), + '', + ); + mkdirSync(join(root, 'core', 'target'), { recursive: true }); + symlinkSync(outside, join(root, 'core', 'target', 'surefire-reports')); + + const report = runAdapter(['core/src/Main.java']); + + expect(report.ok).toBe(true); + expect(report.test[0]?.output).not.toContain('TEST-Stale.xml'); + }); + + it('ignores a commented-out testsuite in a fresh report', () => { + // The twin of the CDATA case: aggregate writers emit commented-out + // markup, and scanning it fabricated phantom failure evidence. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Real.xml'), + '\n' + + '', + ); + return result(command, { + exitCode: 0, + output: '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.output).not.toContain('Ghost'); + expect(report.test[0]?.output).not.toContain('[maven-test-failure]'); + }); + + it('emits a fallback failure line for a report with failures but no case bodies', () => { + // The invariant test-plan's guards key on: failures>0 ⇒ at least one + // [maven-test-failure] line. A header-only failing report emitted none. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-HeaderOnly.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.output).toContain( + '[maven-test-failure] core/target/surefire-reports/TEST-HeaderOnly.xml: ' + + '1 failure(s), 0 error(s) recorded without case detail', + ); + }); + + it('treats -Dmaven.repo.local locations referenced by .mvn/maven.config as dependency inputs', () => { + // The twin of the settings-inputs case: the launcher injects the + // property into the very command the adapter runs, so a changed local + // repository location must suppress the infrastructure carve-out. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync( + join(root, '.mvn', 'maven.config'), + '-Dmaven.repo.local=local-repo\n', + ); + mkdirSync(join(root, 'local-repo')); + + const report = runAdapter(['local-repo/corrupt.jar'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('treats every -Dmaven.repo.local.tail entry as a dependency input', () => { + // Maven 3.9's chained local repositories: EVERY entry is a resolution + // location the PR can change. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync( + join(root, '.mvn', 'maven.config'), + '-Dmaven.repo.local.tail=repo-a,repo-b\n', + ); + mkdirSync(join(root, 'repo-a')); + mkdirSync(join(root, 'repo-b')); + + const report = runAdapter(['repo-b/corrupt.jar'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('fails closed to reactor-wide for a real module nested under a non-root aggregator src/ path', () => { + // The R1-1 positive control beyond the root: `agg` aggregates + // `src/core`; collapsing to `agg` would run + // `-pl agg -am`, and `-am` adds only UPSTREAM projects — the changed + // module would never compile or test under a green verdict. + writeProject('.', ['agg']); + writeProject('agg', ['src/core']); + writeProject('agg/src/core'); + const calls: string[] = []; + + const report = runAdapter(['agg/src/core/src/main/java/Foo.java'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.affected).toEqual(['.']); + }); + + it('still scopes to the owning module when only test-data-shape fixtures sit under its src/', () => { + // `src/test/` and `src/it/` are the principled fixture shapes: the skip + // there must not cost the module a reactor-wide run. + writeReactor(); + writeProject('core/src/it/projects/sample'); + const calls: string[] = []; + + runAdapter( + ['core/src/main/java/Core.java', 'core/src/it/projects/sample/App.java'], + { + exec: (command) => { + calls.push(command); + return result(command); + }, + }, + ); + + expect(calls).toEqual([ + 'mvn --batch-mode --no-transfer-progress -pl core -am test', + ]); + }); + + it('cross-checks surefire stdout summaries against a relocated report directory', () => { + // Reports written to a non-default `` sit outside the + // sweep; the framed `Tests run:` summary Surefire prints even under + // testFailureIgnore is the cross-check that keeps the run green no + // more. A FAILING summary is `[ERROR]`-framed on real Maven (verified + // 3.8.7 / surefire 3.2.5) — the `[INFO]` twin stays covered because + // the regex admits both. + writeReactor(); + for (const framing of ['[ERROR]', '[INFO]']) { + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + `${framing} Tests run: 5, Failures: 2, Errors: 0, Skipped: 0\n` + + '[INFO] BUILD SUCCESS', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.test[0]?.infrastructure).toBeUndefined(); + expect(report.note).toContain('testFailureIgnore'); + } + }); + + // chmod is the only lever this case has: a root user reads through it, + // so the branch under test is unreachable there — the repo convention. + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'fails closed when a reports directory is unreadable', + () => { + // An unreadable directory is the same epistemic state as the caps: the + // sweep did not see everything. chmod 000 is within what a PR's own + // test/shutdown hook can do — the threat model this file grants. + writeReactor(); + const dir = join(root, 'core', 'target', 'surefire-reports'); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + chmodSync(dir, 0o000); + return result(command); + }, + }); + + // Restore so the sandbox cleanup can remove the tree. + chmodSync(dir, 0o755); + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }, + ); + + it('rejects a report whose comment swallows a later failing suite', () => { + // A raw `` + // sits inside a LATER failing suite: honoring it swallowed the failing + // header. The comment interior closes elements still open where it + // started — the swallowing shape — so the report joins the parser's + // fail-closed rejections instead of reading green. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + 'before ', + ); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }); + + it('keeps CDATA-wrapped system-out with XML samples parseable', () => { + // Surefire's own writer wraps test stdout in CDATA immediately after + // the `` open tag, and that stdout routinely contains XML + // samples closing the very elements open around the section — that one + // shape stays exempt from the swallowing probe the twin below applies. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + ' sample]]>' + + '', + ); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.evidenceCapped).toBeUndefined(); + }); + + it('rejects a report whose CDATA swallows a later failing suite', () => { + // The CDATA twin of the comment swallow: a raw `` text — with OTHER content before it, not the tight + // surefire shape — whose `]]>` sits inside a LATER failing suite + // deletes that suite's evidence. The interior-close probe applies to + // CDATA too, so the report joins the parser's fail-closed rejections + // instead of reading green. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + 'before ' + + '' + + '' + + ' after ]]>', + ); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }); + + it('rejects an exempt stream CDATA whose interior swallows a later suite', () => { + // The surefire-writer exemption tolerates stdout samples that close + // the elements open around the section — but for the section to + // delete a REAL later suite, that suite must open inside the interior + // after the closes of the surrounding elements. That sequence + // rejects the report fail-closed; without the probe the swallowed + // suite's header and failure body parse away to a green read. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '' + + '' + + 'boom' + + ']]>' + + '', + ); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }); + + it('rejects a report holding an unterminated CDATA section', () => { + // Kept verbatim, the opaque text would be scanned as markup by the + // body walk: a planted `` inside it cuts the case body + // before its `` evidence and the failing report parses + // green. An unterminated section therefore joins the parser's + // fail-closed rejections instead of staying in the scan. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '' + + 'boom' + + '', + ); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }); + + it('reads failing case bodies as failures when the header is zeroed', () => { + // A rewritten report: `failures="0" errors="0"` attributes over a live + // `` body. The parsed proof of failure is authoritative — the + // green-wash this adapter's threat model exists to catch. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '', + ); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.output).toContain( + '[maven-test-failure] core/target/surefire-reports/TEST-Core.xml: example.T#fails', + ); + }); + + it('does not cut a testcase body on a quoted attribute value', () => { + // The close-tag walk is quote-aware like the header walk: a literal + // `` inside a quoted attribute value is content, not + // markup. Cutting the body there silently lost the `` after + // it and read a failing report green. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '' + + 'boom' + + '', + ); + return result(command); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.output).toContain( + '[maven-test-failure] core/target/surefire-reports/TEST-Core.xml: example.CoreTest#t', + ); + }); + + it('rejects a report nesting a testcase inside a quoted attribute', () => { + // A `` body lands in no body the evidence floor scans, reading + // the failing case away. The raw-opener count sees the hidden header + // and rejects the report like the other unreadable shapes. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + 'boom\'>' + + '', + ); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }); + + it('refuses a report whose last testcase never closes', () => { + // A file truncated mid-case has no closing tag to attribute a body to; + // returning the partially-parsed prefix read the recorded failure body + // away into a green verdict. Fail closed like the interrupted header + // walk — the rejection counts as unknown evidence. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + 'boom' + + '', + ); + return result(command); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + expect(report.note).toContain('failure status is unknown'); + }); + + it('renders an errors-only report rollup with the errors folded into failures', () => { + // Every rollup emitter hardcodes errors=0 and folds errors into + // failures=; pin the real emitter path end-to-end. The fixture carries + // its error ONLY in the header (no failing case body): dropping + // `summary.errors` from failedCount would render failures=0 here while + // every other assertion stayed green, and the rollup would read clean. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command); + }, + }); + + expect(report.test[0]?.output).toContain( + '[maven-test-report] core (1 failing report(s)): tests=1, failures=1, errors=0, skipped=0', + ); + expect(report.ok).toBe(false); + }); + + it('treats a fully-read zero-suite report as known-empty, not unknown', () => { + // A small suite-less XML a PR's own tests write (failsafe-summary.xml is + // the same shape) contributes no evidence and no gap — it must not hold + // the whole run uncertified. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync(join(dir, 'failsafe-summary.xml'), ''); + return result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.evidenceCapped).toBeUndefined(); + }); + + it('classifies a wrapper SHA-256 validation failure as infrastructure', () => { + // apache/maven-wrapper prints this verbatim on a checksum mismatch; the + // pinning fixture uses the wording a real wrapper emits. + writeReactor(); + writeWrapper(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + 'Error: Failed to validate Maven distribution SHA-256, ' + + 'your Maven distribution might be compromised.', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + }); + + it('classifies a silent bootstrap download death as infrastructure', () => { + // Both wrapper generations try wget before curl, and the distribution + // download runs it quiet: a DNS failure dies exit 4 with EMPTY output — + // no wording to match. The curl fallback dies the same way on its own + // codes (resolve, connect, HTTP error, timeout). The absence of any + // Maven-framed line pins the death to bootstrap. + writeReactor(); + writeExecutedWrapper(); + + for (const exitCode of [4, 6, 7, 8, 22, 28]) { + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode, output: '' }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + expect(report.note).toContain('infrastructure evidence'); + } + }); + + it('classifies the curl bootstrap die message as infrastructure', () => { + // Hosts without wget fall back to `curl --silent`, which suppresses + // curl's own `curl: (N)` line; the wrapper's die wording is all the + // output the death leaves. + writeReactor(); + writeWrapper(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + 'curl: Failed to fetch https://archive.apache.org/dist/maven/' + + 'maven-3/3.9.9/binaries/apache-maven-3.9.9-bin.zip', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + }); + + it('classifies the wrapper-jar SHA-256 wording as infrastructure', () => { + // The jar-mode wrapper names the WRAPPER where the distribution mode + // names the distribution; both are checksum verdicts this run's + // launcher printed before Maven existed. + writeReactor(); + writeWrapper(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + 'Error: Failed to validate Maven wrapper SHA-256, your Maven ' + + 'wrapper might be compromised.', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + }); + + it('classifies the missing checksum-tool message as infrastructure', () => { + // Both wrapper generations print this verbatim and exit 1 when a + // checksum was requested and neither sha256sum nor shasum exists. + writeReactor(); + writeWrapper(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + 'Checksum validation was requested but neither ' + + "'sha256sum' or 'shasum' are available.", + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + }); + + it.skipIf(process.platform === 'win32')( + 'classifies a localized wrapper launch failure as infrastructure', + () => { + // bash/dash localize the 126/127 diagnostics under a non-English + // LANG; the classification keys on the shape — an unmodified wrapper + // dying at a launch exit code with no Maven-framed output — not the + // wording. + writeReactor(); + writeWrapper(); + + for (const [output, exitCode] of [ + ['/bin/sh: 1: ./mvnw: Keine Berechtigung', 126], + ['sh: ./mvnw: Datei oder Verzeichnis nicht gefunden', 127], + ] as const) { + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode, output }), + }); + expect(report.note).toContain('infrastructure evidence'); + } + }, + ); + + it('reads a PR-modified wrapper with no fresh reports as never run, whatever it echoes', () => { + // A wrapper the PR modifies CONTROLS both evidence channels: a stub + // `#!/bin/sh` edit keeps the exec bit, echoes a framed line, and + // exits 0. With zero fresh reports, framed output proves nothing the + // stub could not forge, so the run reads unverified whether it echoes + // or stays silent. A modified wrapper that surfaces fresh reports is + // the sibling test's case — reports are the one evidence channel a + // bare echo-stub does not produce on its own terms. + writeReactor(); + writeExecutedWrapper(); + + const framed = runAdapter([executedWrapperName, 'core/src/Main.java'], { + exec: (command) => + result(command, { exitCode: 0, output: '[INFO] BUILD SUCCESS' }), + }); + expect(framed.ok).toBe(false); + expect(framed.test[0]?.neverRan).toBe(true); + expect(framed.note).toContain('changed by the diff'); + + // The silent stub twin lands the same way, and the note names the + // diff's part in it. + const silent = runAdapter([executedWrapperName, 'core/src/Main.java'], { + exec: (command) => result(command, { exitCode: 0, output: '' }), + }); + expect(silent.ok).toBe(false); + expect(silent.test[0]?.neverRan).toBe(true); + expect(silent.note).toContain('changed by the diff'); + }); + + it('reads a PR-modified wrapper writing fresh green reports as a real run', () => { + // The sibling twin: fresh reports plus framed output are the evidence + // of a build that ran — classifying the run "never ran" over them + // asserted a false contradiction for the ordinary wrapper-bump case. + writeReactor(); + writeExecutedWrapper(); + + const report = runAdapter([executedWrapperName, 'core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.ok).toBe(true); + expect(report.test[0]?.neverRan).toBeUndefined(); + expect(report.note).toContain('Maven test passed'); + }); + + it('does not fail a green run when a test echoes a failing Surefire summary', () => { + // Surefire echoes test stdout verbatim: a plugin-integration test + // that prints a child build's failing summary records it in the + // captured output of a fully green run. With visible green fresh + // reports the echoed summary is test output, not Maven's verdict — + // positive failure evidence it becomes only where no reports are + // visible at all (the relocated-`` shape). + writeReactor(); + + const green = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: + '[INFO] BUILD SUCCESS\n' + + '[ERROR] Tests run: 3, Failures: 1, Errors: 0, Skipped: 0', + }); + }, + }); + expect(green.ok).toBe(true); + expect(green.test[0]?.swallowedFailure).toBeUndefined(); + expect(green.note).toContain('Maven test passed'); + + // The relocated-reports twin keeps its defense: with no visible + // reports, the stdout summary is the only signal. + const relocated = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[INFO] BUILD SUCCESS\n' + + '[ERROR] Tests run: 3, Failures: 1, Errors: 0, Skipped: 0', + }), + }); + expect(relocated.ok).toBe(false); + expect(relocated.test[0]?.swallowedFailure).toBe(true); + }); + + it('detects single-dash long fail-never and quiet spellings in maven.config', () => { + // commons-cli accepts `-fail-never`/`-quiet` exactly like the `--` + // twins; missing them silently disarmed the exit-0 green-wash defense + // for a spelling the PR-writable config can carry. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-fail-never\n'); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[INFO] BUILD SUCCESS\n' + + '[ERROR] Failed to execute goal org.apache.maven.plugins:maven-checkstyle-plugin:3.3.1:check on project core: boom', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.note).toContain('fail-never'); + }); + + it('refuses certification when maven.config redirects output to a log file', () => { + // `-l`/`--log-file` sends the ENTIRE build output to the named file: + // every stdout failure scan reads nothing while a green sibling report + // still blocks neverRan — the certified green-wash the quiet and + // fail-never detectors exist to prevent. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-l\nbuild.log\n'); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + expect(report.note).toContain('log file'); + expect(report.note).not.toContain('Maven test passed'); + }); + + it('reads post-test-phase disk and dependency deaths at a failing exit as infrastructure', () => { + // `-pl -am` builds AND tests the upstream modules first, so the + // first `[INFO] Running` line prints long before the changed module + // resolves — a dependency-resolution or disk death AFTER it is still + // the run's own death, and cutting the scan at the first test phase + // filed a transient outage (and a mid-command ENOSPC) as a defect in + // the PR. A failing exit carries no exit-0 forgery premise, so the + // acquisition scans read the whole output. + writeReactor(); + + const afterTests = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[INFO] Running com.example.EchoTest\n' + + '[ERROR] simulated ENOSPC: No space left on device', + }), + }); + expect(afterTests.test[0]?.infrastructure).toBe(true); + expect(afterTests.note).toContain('infrastructure evidence'); + + // The same wording BEFORE any test phase is Maven's own, unchanged. + const genuine = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: '[ERROR] simulated ENOSPC: No space left on device', + }), + }); + expect(genuine.test[0]?.infrastructure).toBe(true); + expect(genuine.note).toContain('infrastructure evidence'); + }); + + it('does not launder a compile failure into infrastructure when upstream test stdout echoes infrastructure words', () => { + // `-pl -am` runs the upstream modules' tests first, and a + // passing upstream test can echo dependency- or disk-wording lines in + // its own stdout. The acquisition arms read the whole output, so the + // echo matches — but a genuine framed compile failure later in the + // same output is the run's real verdict and must stay PR-attributed: + // a compile failure writes no Surefire XML for freshFailures to see, + // so only the source-failure suppression keeps the echo from + // laundering it into an infrastructure result. + writeReactor(); + + const echoedDependency = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[INFO] Running com.example.EchoTest\n' + + '[ERROR] Could not resolve dependencies for project example:upstream\n' + + '[ERROR] COMPILATION ERROR :\n' + + '[ERROR] /tmp/x/core/src/main/java/Main.java:[12,5] cannot find symbol', + }), + }); + expect(echoedDependency.test[0]?.infrastructure).toBeUndefined(); + expect(echoedDependency.note).toContain( + 'Correlate compiler or test errors', + ); + expect(echoedDependency.note).not.toContain('infrastructure evidence'); + + // The disk arm's twin: an upstream test exercising an ENOSPC path + // prints the framed disk wording; the changed module's genuine + // compile failure still stays PR-attributed. + const echoedDisk = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[INFO] Running com.example.EchoTest\n' + + '[ERROR] simulated ENOSPC: No space left on device\n' + + '[ERROR] COMPILATION ERROR :\n' + + '[ERROR] /tmp/x/core/src/main/java/Main.java:[12,5] cannot find symbol', + }), + }); + expect(echoedDisk.test[0]?.infrastructure).toBeUndefined(); + expect(echoedDisk.note).toContain('Correlate compiler or test errors'); + expect(echoedDisk.note).not.toContain('infrastructure evidence'); + }); + + it('does not discard a green run on a forged selector rejection', () => { + // Exit-0 + green fresh reports + no fail-never: Maven prints no + // `[ERROR]`, so a framed selector-rejection line is test stdout — the + // passing run must survive exactly like every other exit-0 framing + // scan. + writeProject('.', ['core']); + writeProject('core'); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: + '[INFO] BUILD SUCCESS\n' + + '[ERROR] Could not find the selected project in the reactor: core', + }); + }, + }); + + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(true); + expect(report.note).toContain('Maven test passed'); + }); + + it('keeps fresh failing reports on a run with forged selector-rejection wording', () => { + // Exit 0 + fresh reports + rejection wording is always forgery: a + // genuine rejection fail-fasts non-zero before any test runs, so it + // never coexists with fresh reports — FAILING ones included. The + // forged line must not discard the run into `unsupported` and hide + // the captured genuine failures (the green twin is the test above). + writeProject('.', ['core']); + writeProject('core'); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: + '[INFO] BUILD SUCCESS\n' + + '[ERROR] Could not find the selected project in the reactor: core', + }); + }, + }); + + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(false); + expect(report.test).toHaveLength(1); + expect(report.test[0]?.output).toContain('[maven-test-failure]'); + expect(report.note).toContain('test failures, not a pass'); + }); + + it('rejects a section that opens a verdict element it does not close', () => { + // The mirror of the swallowing shape: an interior OPEN whose close + // sits after the section erases the element's header and failure body + // without rejection. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '', + ); + return result(command, { exitCode: 1, output: '[INFO] BUILD FAILURE' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }); + + it('keeps stray unclosed fragments of other names out of the mirror probe', () => { + // Test output wrapped in CDATA routinely carries unclosed markup + // fragments (a printed generic type, an HTML log); only the names the + // parse reads may reject the report. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + 'boom ]]>' + + '', + ); + return result(command, { exitCode: 1, output: '[INFO] BUILD FAILURE' }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBeUndefined(); + }); + + it('treats detached -D maven.repo.local spellings in maven.config as dependency inputs', () => { + // The config reader hands Maven one argument per line; commons-cli + // pairs a value-less `-D` with the next line exactly like the attached + // `-Dmaven.repo.local=…` spelling. + writeReactor(); + mkdirSync(join(root, '.mvn')); + mkdirSync(join(root, 'custom-repo')); + writeFileSync( + join(root, '.mvn', 'maven.config'), + '-D\nmaven.repo.local=custom-repo\n', + ); + + const report = runAdapter(['custom-repo/org/example/lib.jar'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.test[0]?.infrastructure).toBeUndefined(); + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it.skipIf(process.platform === 'win32')( + 'sanitizes control characters out of marker line names', + () => { + // A report path is PR-controlled text: a newline in a directory name + // split the appended marker and forged a second line inside the + // classified output (win32 forbids control chars in names, so the + // vector is POSIX-only). + writeReactor(); + const forgedName = + 'evil\n[ERROR] Could not resolve dependencies for project example:core:jar:1'; + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, forgedName, 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Evil.xml'), + '', + ); + return result(command, { + exitCode: 1, + output: + '[ERROR] Failed to execute goal org.example:plugin:1:check on project core: boom', + }); + }, + }); + + // The forged `[ERROR]` line never lands in the classified output: no + // infrastructure classification off it, and the marker carries the + // sanitized single-line name. + expect(report.test[0]?.infrastructure).toBeUndefined(); + const output = report.test[0]?.output ?? ''; + expect(output).not.toContain('\n[ERROR] Could not resolve dependencies'); + expect(output).toContain('evil_[ERROR] Could not resolve dependencies'); + }, + ); + + it('treats single-dash long settings spellings in maven.config as dependency inputs', () => { + // commons-cli accepts `-settings ` exactly like `--settings`; + // reading the token through the `-s` prefix regex recorded `ettings` + // and let the PR's own breakage launder into infrastructure. + writeReactor(); + mkdirSync(join(root, '.mvn')); + mkdirSync(join(root, 'ci')); + writeFileSync(join(root, 'ci', 'conf.xml'), '\n'); + + const withConfig = (config: string) => { + writeFileSync(join(root, '.mvn', 'maven.config'), config); + return runAdapter(['ci/conf.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + }; + + const paired = withConfig('-settings\nci/conf.xml\n'); + expect(paired.note).toContain('Correlate compiler or test errors'); + expect(paired.note).not.toContain('infrastructure evidence'); + + const attached = withConfig('-settings=ci/conf.xml\n'); + expect(attached.note).toContain('Correlate compiler or test errors'); + expect(attached.note).not.toContain('infrastructure evidence'); + }); + + it('splits -Dmaven.repo.local.tail on comma only, never on |', () => { + // Maven parses the chain with `split(",")`: a `|` is part of a path. + // Reading it as a separator recorded a phantom input that could + // withdraw the infrastructure carve-out for an unrelated outage. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync( + join(root, '.mvn', 'maven.config'), + '-Dmaven.repo.local.tail=cache|warm\n', + ); + mkdirSync(join(root, 'cache|warm')); + + const own = runAdapter(['cache|warm/corrupt.jar'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + expect(own.note).toContain('Correlate compiler or test errors'); + expect(own.note).not.toContain('infrastructure evidence'); + + // The phantom half of the old split must NOT record as an input: a + // change under `cache/` alone keeps the carve-out. + mkdirSync(join(root, 'cache')); + const phantom = runAdapter(['cache/corrupt.jar'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + expect(phantom.note).toContain('infrastructure evidence'); + }); + + it('widens the run to the full reactor for a root-level miscellaneous file', () => { + // A root file no exemption claims is the unknown-input case: it must + // widen the run, not drop it. + writeReactor(); + const calls: string[] = []; + + const report = runAdapter(['checkstyle.xml'], { + exec: (command) => { + calls.push(command); + return result(command); + }, + }); + + expect(calls).toEqual(['mvn --batch-mode --no-transfer-progress test']); + expect(report.affected).toEqual(['.']); + }); + + it('parses a failing case whose classname carries İ through the fallback scan', () => { + // `İ`.toLowerCase() lengthens UTF-16 text, which switches + // xmlOpenTagHeaders to its case-insensitive fallback scan; the parse + // must still attribute the failure body to its case. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Ilk.xml'), + '' + + '' + + 'boom' + + '' + + '', + ); + return result(command, { + exitCode: 1, + output: 'There are test failures.', + }); + }, + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.output).toContain('İlk#fails'); + }); + + it('rejects a report whose unclosed attribute quote hides failure bodies', () => { + // A greenwash shape: the suite header says zero failures, an opener's + // quote never closes, and every `` body sits after the hole. + // The interrupted walk discards those bodies, so the report must be + // rejected — reading the surviving prefix would greenwash the run. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + ''); + } + + const sweep = reportPaths(root, 3); + expect(sweep.truncated).toBe(true); + expect(sweep.paths).toHaveLength(2); + }); + + it('reads an unframed selector-rejection wording from test stdout as evidence, not rejection', () => { + // The classifier anchors on Maven's framing: a PR test echoing the + // wording must not discard the run's fresh evidence into `unsupported`. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + return result(command, { + exitCode: 0, + output: + 'some test printed: Could not find the selected project in the reactor: core\n' + + '[INFO] BUILD SUCCESS', + }); + }, + }); + + expect(report.toolchain).toBe('maven'); + expect(report.ok).toBe(true); + }); + + it('falls back to mvn when the wrapper name is a directory', () => { + // A directory named `mvnw` is searchable (passes X_OK) but dies exit 126 + // on execution; the isFile() gate treats it as absent on both platforms. + writeProject('.'); + mkdirSync(join(root, 'mvnw')); + chmodSync(join(root, 'mvnw'), 0o755); + expect(mavenExecutable(root, 'linux')).toBe('mvn'); + + mkdirSync(join(root, 'mvnw.cmd')); + expect(mavenExecutable(root, 'win32')).toBe('mvn'); + }); + + it('reads a non-empty stub wrapper that exits 0 as never run, not as tested nothing', () => { + // Trimming the wrapper to `#!/bin/sh` keeps the exec bit and passes the + // size gate: exit 0, zero reports, zero Maven output. Enumerating + // wrapper shapes misses it; classifying the run does not. The fixture + // writes the wrapper this platform EXECUTES so the neverRan path is + // pinned through its executed-wrapper variant on both platforms. + writeReactor(); + writeExecutedWrapper(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode: 0, output: '' }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.neverRan).toBe(true); + expect(report.note).toContain('without starting Maven'); + }); + + it('mirrors Maven line-by-line maven.config reading for spaced arguments', () => { + // Maven reads one argument per line: `-s` and a spaced path are two + // lines. Whitespace tokenizing recorded the truncated path and let the + // PR's own breakage launder into infrastructure. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync( + join(root, '.mvn', 'maven.config'), + '-s\nci/my settings.xml\n', + ); + mkdirSync(join(root, 'ci')); + writeFileSync(join(root, 'ci', 'my settings.xml'), '\n'); + + const report = runAdapter(['ci/my settings.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('does not read maven.config comment lines as arguments', () => { + // A `#` line naming flags must not record a spurious input that could + // withdraw the carve-out for an unrelated environmental failure. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '# -s ci/real.xml\n'); + mkdirSync(join(root, 'ci')); + writeFileSync(join(root, 'ci', 'real.xml'), '\n'); + + const report = runAdapter(['ci/real.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).toContain('infrastructure evidence'); + }); + + it.each([ + '-s=ci/settings.xml\n', + // Maven reads one argument PER LINE: the paired forms are two lines. + '--settings\nci/settings.xml\n', + '--settings=ci/settings.xml\n', + '-gs\nci/settings.xml\n', + '-gs=ci/settings.xml\n', + ])( + 'treats the %j maven.config spelling as a settings dependency input', + (config) => { + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), config); + mkdirSync(join(root, 'ci')); + writeFileSync(join(root, 'ci', 'settings.xml'), '\n'); + + const report = runAdapter(['ci/settings.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] Could not resolve dependencies for project example:core', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }, + ); + + it('treats a POM nested under a bare src/ module path as a resolution input', () => { + // A reactor can aggregate `src/core`: that POM feeds + // resolution like any other. Excluding EVERY src/-nested POM laundered + // the PR's own resolution breakage into an infrastructure outage. + writeProject('.', ['src/core']); + writeProject('src/core'); + + const report = runAdapter(['src/core/pom.xml'], { + exec: (command) => + result(command, { + exitCode: 1, + output: '[ERROR] Non-resolvable parent POM for example:core', + }), + }); + + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it.skipIf(process.platform === 'win32')( + 'treats a symlinked .mvn/maven.config as unreadable instead of hanging on it', + () => { + // The isFile() gate: a symlink to a character device reports size 0 + // and passes any size cap — readFileSync would block forever. + writeReactor(); + mkdirSync(join(root, '.mvn')); + symlinkSync('/dev/zero', join(root, '.mvn', 'maven.config')); + + const report = runAdapter(['core/src/Main.java']); + + expect(report.ok).toBe(true); + }, + ); + + it('exercises the wrapper-skip arm through the platform parameter', () => { + // The arm that skips the OTHER platform's wrapper, reachable without + // depending on the host's process.platform. + writeReactor(); + expect(detectMavenOwnership(root, ['mvnw'], 'win32')).toEqual({ + reactorWide: false, + modules: [], + }); + expect(detectMavenOwnership(root, ['mvnw.cmd'], 'linux')).toEqual({ + reactorWide: false, + modules: [], + }); + // The SAME platform's wrapper is still reactor-wide evidence. + expect(detectMavenOwnership(root, ['mvnw'], 'linux').reactorWide).toBe( + true, + ); + }); + + it('decodes XML entities in failing case identities', () => { + // Parameterized Surefire names escape `<` / `&`; the decoded identity is + // what Agent 7 correlates against the changed files. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + '' + + '', + ); + return result(command, { exitCode: 1, output: '[ERROR] Tests failed' }); + }, + }); + + expect(report.test[0]?.output).toContain( + '[maven-test-failure] core/target/surefire-reports/TEST-Core.xml: example.T#a & b < c', + ); + }); + + it('pins the Scala and Groovy source-failure alternants', () => { + // For Scala/Groovy repos these alternants are the ONLY source-failure + // signal; a regex rewrite breaking just them must not ship green. + expect( + isSourceFailureLine( + '[ERROR] /tmp/x/core/src/main/scala/Main.scala:12: not found: value foo', + ), + ).toBe(true); + expect( + isSourceFailureLine( + '[ERROR] /tmp/x/core/src/main/groovy/Main.groovy: 3: unable to resolve class', + ), + ).toBe(true); + }); + + it('keys the entry disk floor on the install flag', () => { + // 2 GiB free sits inside the [1 GiB build, 3 GiB install) window: the + // install floor skips a warm-cache run it should admit. + writeReactor(); + statfsSyncMock.mockReturnValue({ bavail: 2 * 1024 ** 3, bsize: 1 }); + + const installing = runAdapter(['core/src/Main.java'], { install: true }); + expect(installing.ok).toBe(false); + expect(installing.note).toContain('Insufficient disk space'); + + const warmCache = runAdapter(['core/src/Main.java'], { install: false }); + expect(warmCache.ok).toBe(true); + expect(warmCache.note).not.toContain('Insufficient disk space'); + }); + + it('does not blame the warm-up in the second preflight note when none ran', () => { + // A --no-install run passes the first preflight, then free space falls + // below the build floor: the note must not assert a download that never + // happened. + writeReactor(); + statfsSyncMock.mockReturnValueOnce({ bavail: 16 * 1024 ** 3, bsize: 1 }); + statfsSyncMock.mockReturnValue({ bavail: 0.5 * 1024 ** 3, bsize: 1 }); + + const report = runAdapter(['core/src/Main.java'], { install: false }); + + expect(report.ok).toBe(false); + expect(report.note).toContain( + 'free space fell below the build floor between the preflight and the lifecycle command', + ); + expect(report.note).not.toContain('warm-up consumed'); + }); + + it('keeps a wrapper-config-only diff from withdrawing the mvn launch carve-out', () => { + // `.mvn/wrapper/**` feeds the wrapper scripts — which never ran here (no + // wrapper in the tree; the system `mvn` launch died). The config cannot + // have caused that death, so the outage stays infrastructure. + writeReactor(); + const report = runAdapter(['.mvn/wrapper/maven-wrapper.properties'], { + exec: (command) => + result(command, { exitCode: 127, output: 'sh: 1: mvn: not found' }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBe(true); + }); + + it('attributes a timed-out reactor-wide run to the selector the diff made unexpressible', () => { + // The timedOut note must carry the TRUE widening cause: a selectorUnsafe + // widening is not "inputs every module inherits". + writeProject('.', ['od,d']); + writeProject('od,d'); + + const report = runAdapter(['od,d/src/main/java/Main.java'], { + exec: (command) => result(command, { exitCode: null, timedOut: true }), + }); + + expect(report.ok).toBe(false); + expect(report.note).toContain('ran out of time'); + expect(report.note).toContain('cannot express'); + expect(report.note).not.toContain('inputs every module inherits'); + }); + + it('fails closed when the report sweep exceeds the scanned-directory cap', () => { + // The scan-count cap is the same fail-closed state as the fan-out bound; + // the cap is a parameter so the test reaches it without building 20,000 + // directories. + writeReactor(); + for (let i = 0; i < 10; i++) { + mkdirSync(join(root, `d${i}`, 'nested'), { recursive: true }); + } + + expect(reportPaths(root, 5).truncated).toBe(true); + expect(reportPaths(root, 100).truncated).toBe(false); + }); + + it('caps the accumulated report paths when the per-dimension product blows up', () => { + // Every cap bounded ONE dimension; nothing bounded their product — two + // modules x both report dirs x 5,100 XMLs respects the per-dir entry + // cap and the scanned-dir cap yet accumulates past the path cap. The + // sweep must stop collecting instead of retaining and statSync-ing + // hundreds of thousands of paths. + writeReactor(); + for (const module of ['core', 'extension']) { + for (const reportDir of ['surefire-reports', 'failsafe-reports']) { + const dir = join(root, module, 'target', reportDir); + mkdirSync(dir, { recursive: true }); + for (let i = 0; i < 5_100; i++) { + writeFileSync(join(dir, `t${reportDir[0]}${i}.xml`), ''); + } + } + } + + const { paths, truncated } = reportPaths(root); + + expect(truncated).toBe(true); + expect(paths.length).toBe(20_000); + }, 60_000); + + it('matches unmatched closers against a deep open stack in linear time', () => { + // The closeTag membership scan went quadratic on PR-controlled bytes: + // k never-closed openers plus k unmatched closers is k full stack + // scans — measured seconds at 20k pairs through the real adapter + // (extrapolating to tens of minutes at the report cap), the same + // denial-of-service class the other linear pins in this suite. The + // trailing comment is load-bearing: without a CDATA/comment marker + // stripOpaqueSections returns before scanning at all. + writeReactor(); + const startedAt = Date.now(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '' + + ''.repeat(50_000) + + ''.repeat(50_000) + + '', + ); + return result(command); + }, + }); + + expect(Date.now() - startedAt).toBeLessThan(5_000); + expect(report.ok).toBe(true); + }, 20_000); + + it('scans comment-interior close tokens against a deep stack in linear time', () => { + // The swallow check ran `.some(...)` over the whole open stack for + // EVERY close token inside a comment — the second quadratic site, + // reachable through a single `' + + '', + ); + return result(command); + }, + }); + + expect(Date.now() - startedAt).toBeLessThan(5_000); + expect(report.ok).toBe(true); + }, 20_000); + + it('prints per-report clamped totals on the failing rollup', () => { + // Surefire does not guarantee tests >= failures + skipped within one + // report, and test-plan clamps per parsed LINE: raw pre-aggregated + // totals would let one anomalous report cancel its batchmates' passed + // counts (report A below parses to -3 passed without the per-report + // clamp). The failing rollup must emit the same clamped shape the + // clean rollup does. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(root, 'core', 'target', 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Anomalous.xml'), + '', + ); + writeFileSync( + join(dir, 'TEST-Normal.xml'), + '' + + '' + + '', + ); + return result(command, { exitCode: 1, output: '[ERROR] Tests failed' }); + }, + }); + + // Per-report truth: A passed max(0, 2-2-3)=0, B passed max(0, 10-1-0)=9 + // — so tests = 9 passed + 3 failed with skipped zeroed, and the line + // clamp parses back to 9 instead of the old wash-down to 6. + expect(report.test[0]?.output).toContain( + '[maven-test-report] core (2 failing report(s)): ' + + 'tests=12, failures=3, errors=0, skipped=0', + ); + }); + + it('reads [ERROR]-framed stdout test failures as source-side, not infrastructure', () => { + // Real Maven frames a failing module's stdout summary `[ERROR]`, and a + // test throwing ConnectException prints dependency-flavored wording the + // dependency matcher claims. With the failing reports relocated out of + // the sweep, the stdout summary is the only evidence that the run + // executed failing tests — that is source-side, never an acquisition + // outage. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 1, + output: + '[ERROR] ConnTest.connects:7 \u00bb Connect Connection refused\n' + + '[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0\n' + + '[INFO] BUILD FAILURE', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.infrastructure).toBeUndefined(); + expect(report.test[0]?.swallowedFailure).toBeUndefined(); + expect(report.note).toContain('Correlate compiler or test errors'); + expect(report.note).not.toContain('infrastructure evidence'); + }); + + it('does not launder a swallowed stdout test failure into infrastructure', () => { + // The exit-0 twin of the ConnectException wash: a fail-never run whose + // stdout records executed failing tests beside dependency-flavored + // wording is a swallowed test failure, not an acquisition outage. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: 0, + output: + '[ERROR] Could not transfer artifact org.example:lib:pom:1 from central: Connection timed out\n' + + '[ERROR] Tests run: 1, Failures: 1, Errors: 0, Skipped: 0\n' + + '[INFO] BUILD SUCCESS', + }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.swallowedFailure).toBe(true); + expect(report.test[0]?.infrastructure).toBeUndefined(); + }); + + // chmod is the only lever this case has; the repo convention for it. + it.skipIf(process.platform === 'win32' || process.getuid?.() === 0)( + 'fails closed when a target directory is unreadable', + () => { + // The lstat gate one level ABOVE the report dir: an unreadable + // `target` used to read as 'no reports dir here', certifying green a + // run whose fresh failing reports the sweep could not see. chmod 000 + // is within the threat model this file grants. + writeReactor(); + const target = join(root, 'core', 'target'); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => { + const dir = join(target, 'surefire-reports'); + mkdirSync(dir, { recursive: true }); + writeFileSync( + join(dir, 'TEST-Core.xml'), + '', + ); + chmodSync(target, 0o000); + return result(command); + }, + }); + + // Restore so the sandbox cleanup can remove the tree. + chmodSync(target, 0o755); + expect(report.ok).toBe(false); + expect(report.test[0]?.evidenceCapped).toBe(true); + }, + ); + + it('names a quiet maven.config as the never-ran alternative cause', () => { + // `-q`/`--quiet` in the PR-writable config strips every framed line the + // neverRan check keys on: a quiet run that skipped its tests exits 0 + // with empty output, indistinguishable there from a wrapper that never + // started — the note must name the real alternative. + writeReactor(); + mkdirSync(join(root, '.mvn')); + writeFileSync(join(root, '.mvn', 'maven.config'), '-q\n-DskipTests\n'); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode: 0, output: '' }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.neverRan).toBe(true); + expect(report.note).toContain('`-q`/`--quiet`'); + }); + + it('does not name the quiet setting when the config has none', () => { + // Control for the note above: the same empty-output shape without the + // flag points at the wrapper only. + writeReactor(); + + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => result(command, { exitCode: 0, output: '' }), + }); + + expect(report.ok).toBe(false); + expect(report.test[0]?.neverRan).toBe(true); + expect(report.note).toContain('empty or'); + expect(report.note).not.toContain('--quiet'); + }); + + it('names stdout-recorded failures in a timed-out run', () => { + // The deadline kill is infrastructure, but the captured framed `Tests + // run:` summaries are failures Surefire already recorded — the note + // must not assert a purely informational result over them. + writeReactor(); + const report = runAdapter(['core/src/Main.java'], { + exec: (command) => + result(command, { + exitCode: null, + timedOut: true, + output: '[ERROR] Tests run: 5, Failures: 2, Errors: 0, Skipped: 0', + }), + }); + + expect(report.ok).toBe(false); + expect(report.note).toContain('ran out of time'); + expect(report.note).toContain('treat those as test failures'); + }); + + it('omits the mixed-root caveat when package.json is a directory', () => { + // npm's applies() fails closed on a DIRECTORY named package.json + // (EISDIR swallowed to no manifests), so Maven runs alone with no npm + // half — the caveat would be false. + writeReactor(); + mkdirSync(join(root, 'package.json')); + + const report = runAdapter(['core/src/Main.java']); + + expect(report.ok).toBe(true); + expect(report.note).not.toContain('Mixed root'); + }); + + it('keeps the mixed-root caveat for a real root package.json', () => { + writeReactor(); + writeFileSync( + join(root, 'package.json'), + JSON.stringify({ scripts: { build: 'tsc' } }), + ); + + const report = runAdapter(['core/src/Main.java']); + + expect(report.ok).toBe(true); + expect(report.note).toContain('Mixed root'); + }); + + it('applies() requires a REGULAR pom.xml file', () => { + // A DIRECTORY named pom.xml passes existsSync but selects Maven over a + // shape `mvn` refuses to build — the same isFile() gate mavenExecutable + // and mavenConfigDependencyInputs apply. This is also what keeps a + // polyglot base selecting npm instead of falling unsupported. + const plain = join(sandbox, 'applies-plain'); + mkdirSync(plain); + expect(mavenToolchainAdapter.applies(plain)).toBe(false); + + writeFileSync(join(plain, 'pom.xml'), pom()); + expect(mavenToolchainAdapter.applies(plain)).toBe(true); + + const dirPom = join(sandbox, 'applies-dir'); + mkdirSync(join(dirPom, 'pom.xml'), { recursive: true }); + expect(mavenToolchainAdapter.applies(dirPom)).toBe(false); + }); +}); diff --git a/packages/cli/src/commands/review/lib/maven-toolchain.ts b/packages/cli/src/commands/review/lib/maven-toolchain.ts new file mode 100644 index 00000000000..6c2982ad52f --- /dev/null +++ b/packages/cli/src/commands/review/lib/maven-toolchain.ts @@ -0,0 +1,2572 @@ +/** + * @license + * Copyright 2026 Qwen Team + * SPDX-License-Identifier: Apache-2.0 + */ + +import { + accessSync, + constants, + existsSync, + lstatSync, + opendirSync, + readFileSync, + statSync, +} from 'node:fs'; +import type { Dirent } from 'node:fs'; +import { dirname, isAbsolute, join, relative, resolve, sep } from 'node:path'; +import type { + BuildTestReport, + CommandResult, + MavenCommandFacts, +} from '../build-test.js'; +import { + BUILD_MIN_FREE_BYTES, + INSTALL_MIN_FREE_BYTES, + freeDiskBytes, + gib, +} from './disk.js'; +import { shellQuotePath } from './shell-quote.js'; +import type { ReviewToolchainAdapter, ToolchainRunArgs } from './toolchain.js'; + +export interface MavenOwnership { + reactorWide: boolean; + modules: string[]; +} + +/** + * SGR color sequences. Every classification regex in this file anchors on + * Maven's `[INFO]`/`[ERROR]` framing, and a `-Dstyle.color=always` config + * interleaves these codes BEFORE and BETWEEN tokens, defeating every + * anchored predicate — so the executed output is stripped ONCE at + * ingestion, before any classification or marker mining reads it. + * `build-test`'s trim rescue shares this constant: same bytes, same answer. + */ +// eslint-disable-next-line no-control-regex -- ESC is the character under test +export const ANSI_SGR_RE = /\x1b\[[0-9;]*m/g; + +const REACTOR_WIDE_FILES = new Set(['pom.xml', 'mvnw', 'mvnw.cmd']); +/** + * `failsafe-reports` is forward-looking today: this adapter only ever runs + * `test` and `test-compile`, and Failsafe binds to `integration-test` / + * `verify`, so any XML found there is filtered out as stale. The scan stays + * — one readdir per project per snapshot — so the evidence is picked up if a + * later change ever runs a Failsafe phase. + */ +const REPORT_DIRS = ['surefire-reports', 'failsafe-reports']; + +/** + * Surefire writes one XML per test class, so a green full-reactor run yields + * thousands of reports. Clean AND failing reports therefore roll up per + * project dir, and the rollup lines are capped: this block is appended AFTER + * the command output was trimmed, so it carries its own bound. + */ +const MAX_FAILING_REPORT_LINES = 100; +const MAX_FAILURE_CASE_LINES = 200; +const MAX_CLEAN_ROLLUP_LINES = 100; + +/** + * cmd.exe refuses command lines past 8191 characters, and containerized + * execve enforces ARG_MAX. A POM change at a mid-level aggregator closes + * over every aggregation AND inheritance descendant, and on the 200-400 + * module reactors this adapter targets the comma-joined `-pl` selector can + * approach those limits — a command line the platform refuses to launch is + * not a scope. Past the cap the run widens to the full reactor instead. + * 4096 leaves headroom for the executable, flags, and environment. + */ +const MAX_SELECTOR_CHARS = 4096; + +/** + * Cap evidence files before reading them: Surefire/Failsafe XML is + * PR-controlled (the PR's own tests can write into `target/surefire-reports/` + * during the run, and the mtime freshness filter accepts any writer), so an + * uncapped read of a multi-gigabyte file is this harness's own denial-of- + * service surface. 2 MiB is far beyond any realistic per-class report; an + * oversized file simply contributes no evidence. + */ +const MAX_REPORT_BYTES = 2 * 1024 * 1024; + +/** + * `.mvn/maven.config` carries the same class of cap: it is split on + * whitespace, and an uncapped multi-megabyte config a PR commits is this + * harness's own denial-of-service surface — measured at 37 MB the split cost + * seconds of synchronous CPU and hundreds of MB of transient heap, scaling + * linearly to GitHub's 100 MB per-file limit. An oversized config contributes + * no settings inputs. + */ +const MAX_CONFIG_BYTES = 2 * 1024 * 1024; + +/** + * Cap the report sweep: it walks the worktree for `target/` + * directories, and a PR controls how many directories exist. Past the cap the + * sweep stops and reports the truncation — a truncated sweep can miss failure + * evidence, so the run refuses to certify a pass (see `evidenceCapped`). + */ +const MAX_SCANNED_DIRS = 20_000; + +/** + * Bound the enumeration of ONE directory as well as the sweep's scan count: + * `readdirSync` materializes the full Dirent array at once, and a PR- + * controlled wide fan-out (a single directory holding hundreds of thousands + * of children) made that array the sweep's dominant memory cost. Entries are + * streamed through `opendirSync` and reading stops past this bound; the + * directory then counts as truncation, failing closed like the scan cap. + */ +const MAX_DIR_ENTRIES = 10_000; + +/** + * Cap how many fresh reports one run parses: the parse is synchronous, + * outside any deadline, on files the PR's own tests can write during the + * run (the mtime freshness filter accepts any writer). `MAX_REPORT_BYTES` + * bounds each file, but nothing else bounded the COUNT — thousands of + * 2 MiB reports are multi-GB of live strings and minutes of CPU past the + * outer tool timeout. Past the cap the run's note discloses the sampling + * (`sampledEvidence`) instead of refusing certification: the parsed + * reports are still real evidence, and a green-but-huge reactor run must + * not read as an uncertified failure. + */ +const MAX_FRESH_REPORTS = 1_000; + +/** + * Cap the sweep's PATH accumulation itself: every other cap bounds ONE + * dimension (scanned dirs, entries per dir, parsed reports, report bytes), + * but nothing bounded their product — 20k scanned dirs x both report dirs x + * 10k entries each accumulates hundreds of millions of paths, and + * snapshotReports + freshTestSummaries statSync and retain every one before + * the MAX_FRESH_REPORTS slice ever applies. A PR controls how many + * directories and report files exist, so the product is this harness's own + * denial-of-service surface. Past the cap the sweep stops collecting and + * reports truncation, failing closed like the other caps. + */ +const MAX_REPORT_PATHS = 20_000; + +/** + * Cap the failing cases one report accumulates, while building it: the + * display caps in appendTestSummaries apply after every report was + * materialized, and one report can carry tens of thousands of failing + * `` entries. The dropped count still joins the omission + * marker, so count adjudication sees the truncation. + */ +const MAX_FAILURE_CASES_PER_REPORT = 200; + +/** + * Below this much remaining whole-call budget a Maven command is NOT + * attempted — the same floor as the npm adapter, for the same reason: Maven + * cannot boot and produce signal in a few hundred milliseconds, so an + * "attempt" would manufacture a fake timeout where an honest disclosure says + * exactly what happened. + */ +const BUDGET_MIN_ATTEMPT_MS = 15_000; + +function toPosix(path: string): string { + return path.split(sep).join('/'); +} + +function isInside(root: string, path: string): boolean { + const rel = relative(root, path); + return ( + rel === '' || + (!rel.startsWith(`..${sep}`) && rel !== '..' && !isAbsolute(rel)) + ); +} + +function normalizedChangedPath( + root: string, + changedFile: string, +): string | null { + const absolute = resolve(root, changedFile); + if (!isInside(root, absolute)) return null; + return toPosix(relative(root, absolute)); +} + +function isDocumentationPath(path: string): boolean { + // Anchored to the WHOLE relative path: a DIRECTORY named `README` must + // not exempt its entire subtree — files of any extension — from + // verification. + if (/^README(?:\.[^/]*)?$/i.test(path)) return true; + // The `src/` guard alone would skip a compilable file under a module's + // `doc/` tree; a documentation path is a documentation EXTENSION first. + if (path.startsWith('src/')) return false; + if (!/\.(?:md|mdx|adoc|rst|txt)$/i.test(path)) return false; + // Outside `src/`, the extension alone is not enough: a `.txt` can be a + // resource wired into the artifact (maven-resources-plugin points at + // arbitrary dirs), and skipping the build on it would be a fail-open in an + // otherwise fail-closed design. Exempt only doc-shaped locations: a + // `docs?/` or `site/` tree, or the module/root top level itself. + const dir = dirname(path); + return dir === '.' || /^(?:docs?|site)$/i.test(dir.split('/')[0]); +} + +/** + * Repository metadata that cannot change what Maven builds: VCS/CI config, + * licenses, editor rules. Anything NOT recognized here still runs the reactor + * — a root `checkstyle.xml` or build script affects the build, and failing + * closed costs time while failing open ships an unverified diff. + */ +function isRepoMetadataPath(path: string): boolean { + // `[^/]*` keeps the LICENSE/NOTICE extension run inside the final + // segment: a DIRECTORY with one of those names must not exempt its + // subtree from verification. + return ( + /^(?:\.git(?:ignore|attributes|modules)|\.editorconfig|CODEOWNERS|LICENSE(?:\.[^/]*)?|NOTICE(?:\.[^/]*)?)$/.test( + path, + ) || path.startsWith('.github/') + ); +} + +/** + * The Maven project directory that owns a path: the nearest ancestor holding a + * `pom.xml`. + * + * Directories strictly beneath a `src/` tree are skipped. A POM there is OFTEN + * test data — maven-invoker ITs, archetype fixtures, + * `src/test/resources/projects/*` — but a reactor can also aggregate a real + * module there (`src/core`). The skip is principled only + * for the test-data shapes (`src/test/`, `src/it/`); when the walk collapses + * onto a POM it skipped, it fails closed — to the ROOT always (`-pl .` + * compiles only the root), and elsewhere unless every skipped POM was a + * test-data shape — because `-pl -am` adds only UPSTREAM projects, + * so a mis-collapsed target leaves the changed module untested under a green + * verdict. Returning null escalates the path to a reactor-wide run instead. + * + * Whether the project this returns is ACTIVE under the current profiles, JDK, + * and `` inheritance is deliberately NOT decided here: Maven decides + * it, by accepting or rejecting the `-pl` selector this ownership produces + * (see SELECTOR_REJECTED_RE). Approximating that answer from the POM text + * means shipping a second, weaker model of the thing the very next command + * evaluates for real. + */ +function owningProject(root: string, path: string): string | null { + let dir = dirname(join(root, path)); + let skippedPomBeneathSrc = false; + let skippedTestDataOnly = true; + while (isInside(root, dir)) { + const rel = toPosix(relative(root, dir)) || '.'; + // Strictly BENEATH `src/`: a real project located exactly AT a `src` path + // is not test data. + if (/(?:^|\/)src\//.test(rel)) { + if (existsSync(join(dir, 'pom.xml'))) { + skippedPomBeneathSrc = true; + // `src/test/` and `src/it/` trees are the principled fixture shapes + // (invoker ITs, archetype test projects); any OTHER src/-nested POM + // can be a real module a reactor aggregates (`src/core`). + if (!/(?:^|\/)src\/(?:test|it)\//.test(rel)) { + skippedTestDataOnly = false; + } + } + } else if (existsSync(join(dir, 'pom.xml'))) { + // Fail closed when the collapse cannot be trusted: at the ROOT a + // skipped POM may be a real `src/…` and `-pl .` + // would compile only the root, and anywhere else a skipped POM that + // is not a test-data shape is the same risk — `-pl -am` + // adds only UPSTREAM projects, so the changed module would go + // untested under a green verdict. Null escalates to reactor-wide. + if (skippedPomBeneathSrc && (rel === '.' || !skippedTestDataOnly)) { + return null; + } + return rel; + } + if (dir === root) break; + dir = dirname(dir); + } + return null; +} + +export function detectMavenOwnership( + root: string, + changedFiles: readonly string[], + platform: string = process.platform, +): MavenOwnership { + const modules = new Set(); + let reactorWide = false; + // Every wrapper repo ships both platform variants, but only one is ever + // executed: a change confined to the OTHER platform's wrapper cannot affect + // this platform's run, so it neither escalates to reactor-wide nor falls + // into the unowned catch-all (which would run the whole reactor to verify + // nothing). + const otherPlatformWrapper = platform === 'win32' ? 'mvnw' : 'mvnw.cmd'; + + for (const changedFile of changedFiles) { + const path = normalizedChangedPath(root, changedFile); + if (path === null) continue; + if (path === otherPlatformWrapper) continue; + if (REACTOR_WIDE_FILES.has(path) || path.startsWith('.mvn/')) { + reactorWide = true; + continue; + } + const owner = owningProject(root, path); + if (owner === null) { + // Outside every Maven project. Documentation and repository metadata + // cannot change what Maven builds; anything else can. + if (!isDocumentationPath(path) && !isRepoMetadataPath(path)) { + reactorWide = true; + } + continue; + } + // A project's own POM is parent config: Maven merges it into every project + // that aggregates it or declares it as ``, and this adapter models + // none of those edges — `-pl -am` would compile the aggregator and + // test nothing that actually changed. A POM change runs the reactor and + // lets Maven apply the real inheritance. + if (path === (owner === '.' ? 'pom.xml' : `${owner}/pom.xml`)) { + reactorWide = true; + continue; + } + if (owner === '.') { + if (isDocumentationPath(path) || isRepoMetadataPath(path)) continue; + // Source owned by the root project scopes to `-pl . -am`: no other + // module compiles the root artifact's own `src/`, and on the large + // reactors this adapter targets a reactor-wide run can spend its whole + // deadline proving nothing. Anything ELSE at the root (a build script, a + // checkstyle config) can affect every module. + if (path === 'src' || path.startsWith('src/')) modules.add('.'); + else reactorWide = true; + continue; + } + // Documentation and metadata are judged MODULE-relatively, so the `src/` + // guard means the module's own source tree: `core/README.md` is a no-op + // run, but `core/src/test/resources/expected.txt` is test data and must + // keep building. + const inModule = path.slice(owner.length + 1); + if (isDocumentationPath(inModule) || isRepoMetadataPath(inModule)) continue; + modules.add(owner); + } + + return { reactorWide, modules: [...modules].sort() }; +} + +interface ReportSnapshot { + mtimes: Map; + /** The pre-run sweep stopped early: the freshness baseline is incomplete. */ + truncated: boolean; +} + +interface MavenTestSummary { + report: string; + tests: number; + failures: number; + errors: number; + skipped: number; + failedCases: string[]; + /** Failing cases dropped by MAX_FAILURE_CASES_PER_REPORT while parsing. */ + droppedCases: number; +} + +/** + * A directory's entries, streamed one at a time so a PR-controlled wide + * fan-out cannot materialize an unbounded Dirent array. Reading stops past + * MAX_DIR_ENTRIES and reports the truncation; an unreadable directory + * returns null. + */ +function readDirBounded( + dir: string, +): { entries: Dirent[]; truncated: boolean } | null { + let handle; + try { + handle = opendirSync(dir); + } catch { + return null; + } + const entries: Dirent[] = []; + let truncated = false; + try { + for (;;) { + // Read BEFORE the cap check: a directory holding exactly + // MAX_DIR_ENTRIES entries is read to exhaustion and must not report + // truncation — the flag propagates to evidenceCapped and would + // refuse certification of a fully-read green run. + const entry = handle.readSync(); + if (entry === null) break; + if (entries.length >= MAX_DIR_ENTRIES) { + truncated = true; + break; + } + entries.push(entry); + } + } catch { + // A mid-read throw (EIO/ESTALE on a network-backed worktree) is the + // same epistemic state as an unreadable directory: join the fail-closed + // truncation path instead of escaping the sweep. + return null; + } finally { + // A failing close must not throw past both the truncation logic and the + // return-null rescue — the fail-closed guarantee covers it too. + try { + handle.closeSync(); + } catch { + truncated = true; + } + } + return { entries, truncated }; +} + +/** + * Every `/target//*.xml` in the worktree. + * + * The sweep walks the tree rather than a list of reactor projects: which + * projects are active is Maven's answer, not this adapter's, and a report + * directory only exists where Maven actually ran. Symlinks are never + * followed: a Dirent's `isDirectory()` is false for one, and the report-dir + * read itself is gated on `lstatSync` (which does not resolve the link), so + * neither the descent nor the direct listing can escape the worktree. + * + * `truncated` reports that the sweep stopped early — the scanned-directory + * cap, the per-directory fan-out bound, an unreadable directory, or a queue + * that outgrew the scan budget. A truncated sweep can miss failure evidence, + * so the caller fails closed on it exactly like the fresh-report cap. + */ +export function reportPaths( + root: string, + maxScannedDirs: number = MAX_SCANNED_DIRS, +): { paths: string[]; truncated: boolean } { + const paths: string[] = []; + const queue: string[] = [root]; + let scanned = 0; + let truncated = false; + let pathsCapped = false; + while (queue.length > 0 && scanned < maxScannedDirs && !pathsCapped) { + const dir = queue.pop() as string; + scanned += 1; + const listing = readDirBounded(dir); + // An unreadable directory is the same epistemic state as the caps: the + // sweep did not see everything, so it fails closed instead of skipping on. + if (listing === null) { + truncated = true; + continue; + } + if (listing.truncated) truncated = true; + for (const entry of listing.entries) { + if (!entry.isDirectory()) continue; + if (entry.name === '.git' || entry.name === 'node_modules') continue; + const child = join(dir, entry.name); + if (entry.name !== 'target') { + // A wide fan-out can enqueue far more directories than the scan + // budget will ever pop; the backlog itself is the memory cost, so + // stop enqueuing and count it as truncation. Not pinnable by a + // behavior test: the end-of-loop `queue.length > 0` check sets the + // same flag whenever this guard fires, and which paths survive the + // LIFO pop depends on the filesystem's entry order — the bound is + // the point, not the observable outcome. + if (queue.length >= maxScannedDirs) { + truncated = true; + continue; + } + queue.push(child); + continue; + } + // Never descend INTO `target`: it holds unpacked dependencies and + // generated sources, and the only paths of interest sit one level down. + for (const reportDir of REPORT_DIRS) { + const reports = join(child, reportDir); + // lstat does NOT follow a symlink: a symlinked report dir would + // resolve outside the worktree and inject its stale reports as + // fresh evidence. + try { + if (!lstatSync(reports).isDirectory()) continue; + } catch (error) { + // Absence (ENOENT/ENOTDIR) is 'no reports dir here'; any OTHER + // error — EACCES on an unreadable `target`, chmod 000 within the + // threat model this file grants — means the sweep did not see + // everything and must fail closed like the sibling caps. + if ( + (error as NodeJS.ErrnoException).code !== 'ENOENT' && + (error as NodeJS.ErrnoException).code !== 'ENOTDIR' + ) { + truncated = true; + } + continue; + } + const files = readDirBounded(reports); + if (files === null) { + truncated = true; + continue; + } + if (files.truncated) truncated = true; + for (const file of files.entries) { + if (paths.length >= MAX_REPORT_PATHS) { + pathsCapped = true; + break; + } + if (file.isFile() && file.name.endsWith('.xml')) { + paths.push(join(reports, file.name)); + } + } + if (pathsCapped) break; + } + if (pathsCapped) break; + } + } + if (pathsCapped) truncated = true; + if (queue.length > 0) truncated = true; + return { paths, truncated }; +} + +function snapshotReports(root: string): ReportSnapshot { + // Freshness is an mtime comparison, and some filesystems resolve mtimes at + // 1s granularity: a report rewritten inside the same tick reads as stale and + // is dropped. That degrades in the safe direction — absent test-count + // evidence, never a wrong verdict — so no sub-second workaround is worth it. + const { paths, truncated } = reportPaths(root); + const mtimes = new Map(); + for (const path of paths) { + try { + mtimes.set(path, statSync(path).mtimeMs); + } catch { + // The report disappeared while the snapshot was being taken. + } + } + return { mtimes, truncated }; +} + +function xmlAttributes(source: string): Map { + const attributes = new Map(); + // The lookbehind pins each name to a maximal word run: without it, a long + // attribute-name run with no `=` backtracked the greedy name from every + // start position — quadratic on PR-controlled report bytes. + const re = /(?') + .replace(/&/g, '&'); +} + +function numberAttribute( + attributes: Map, + name: string, +): number { + const value = Number.parseInt(attributes.get(name) ?? '0', 10); + if (!Number.isFinite(value)) return 0; + // A malformed report's negative count must not cancel legitimate counts + // from its neighbours when totals roll up across reports. + return Math.max(0, value); +} + +/** A start tag located by `xmlOpenTagHeaders`. */ +interface XmlOpenTagHeader { + /** Attribute run between the tag name and the closing `>`. */ + attributes: string; + /** Offset of the opening `<` in the scanned text. */ + index: number; + /** The full tag text; a self-closing tag ends `/>`. */ + text: string; +} + +const XML_WORD_CHAR = /[A-Za-z0-9_]/; + +/** + * Quote-aware linear scan for `` start tags. A `>` is legal + * unescaped inside a quoted attribute value (parameterized-test and + * @DisplayName suite/case names carry them). The regex header walk this + * replaces went quadratic on PR-controlled reports: one never-closed opener + * made every later tag start scan to EOF (a 2 MiB report of `` outside quotes. + * An opener with no `>` before EOF ends the scan and reports truncation: + * every later header — and every ``/`` body after it — was + * discarded, so parseTestReport fails closed on such a report instead of + * reading the surviving prefix as the whole truth. + */ +function xmlOpenTagHeaders( + xml: string, + name: string, +): { headers: XmlOpenTagHeader[]; truncated: boolean } { + const tag = `<${name.toLowerCase()}`; + // toLowerCase() can lengthen UTF-16 text (`İ` → `i` + U+0307), so offsets + // located in a lowercased copy would misindex the original xml past the + // first such character. Use the copy only while it stayed the same length; + // otherwise scan the original case-insensitively. + const lower = xml.toLowerCase(); + const indexOfTag = + lower.length === xml.length + ? (from: number): number => lower.indexOf(tag, from) + : (from: number): number => { + for (let i = from; i + tag.length <= xml.length; i += 1) { + let matched = true; + for (let j = 0; j < tag.length; j += 1) { + if (xml[i + j].toLowerCase() !== tag[j]) { + matched = false; + break; + } + } + if (matched) return i; + } + return -1; + }; + const headers: XmlOpenTagHeader[] = []; + let from = 0; + for (;;) { + const start = indexOfTag(from); + if (start === -1) return { headers, truncated: false }; + from = start + 1; + // `\b` semantics: `') { + end = i; + break; + } + } + if (end === -1) return { headers, truncated: true }; + headers.push({ + attributes: xml.slice(start + tag.length, end), + index: start, + text: xml.slice(start, end + 1), + }); + from = end + 1; + } +} + +/** + * Quote-aware forward scan for the next `` close. A literal + * `` inside a quoted attribute value is content, not markup: + * cutting a body there silently loses every ``/`` element + * after it — the anti-greenwash body floor this walk exists to provide. + * Quote state only matters INSIDE tags; body text between tags carries + * apostrophes freely. + */ +function findTestcaseClose( + xml: string, + from: number, +): { start: number; end: number } | null { + let inTag = false; + let quote: '"' | "'" | null = null; + for (let i = from; i < xml.length; i += 1) { + const char = xml[i]; + if (inTag) { + if (quote !== null) { + if (char === quote) quote = null; + } else if (char === '"' || char === "'") { + quote = char; + } else if (char === '>') { + inTag = false; + } + continue; + } + if (char !== '<') continue; + if (isTestcaseCloseAt(xml, i)) { + let end = i; + while (xml[end] !== '>') end += 1; + return { start: i, end: end + 1 }; + } + inTag = true; + } + return null; +} + +function isTestcaseCloseAt(xml: string, i: number): boolean { + let j = i + 1; + for (const char of '/testcase') { + if ((xml[j] ?? '').toLowerCase() !== char) return false; + j += 1; + } + while (xml[j] !== undefined && /^\s$/.test(xml[j])) j += 1; + return xml[j] === '>'; +} + +const XML_NAME_CHAR = /[A-Za-z0-9:_.-]/; + +/** + * Drop terminated `` sections and `` comments in + * one linear pass: both are opaque text, never markup, and scanning a + * commented-out or CDATA-wrapped suite (aggregate writers like jest-junit + * and karma emit both) fabricated phantom suites and failure evidence. The + * earlier marker wins — a marker inside the other kind is literal content, + * consumed with it. An unterminated section rejects the report: kept + * verbatim, its opaque text is scanned as markup by the body walk, and a + * planted `` inside it cuts a testcase body before its + * `` evidence — a green read instead of a fail-closed one. + * + * The pass tracks tag/quote state so markers are honored only in genuine + * markup position. A malformed aggregate-writer report can carry a RAW `` sits inside a + * LATER suite — honoring it swallows that suite's failing header and reads a + * failed run green. Two COMMENT shapes therefore reject the report (null), + * joining the parser's other fail-closed rejections: a marker inside a tag + * or quoted attribute is never markup, and a comment whose interior closes + * an element still open where the comment started spanned across that + * element's boundary — the swallowing shape — rather than commenting out + * self-contained phantom markup, whose open/close pairs both sit inside the + * comment. CDATA carries the same probe, with the one legitimate shape + * narrowed: surefire's own writer wraps ``/`` test + * stdout in CDATA immediately after the open tag, and that stdout + * routinely contains XML samples closing the very elements open around the + * section or pairing their own opens and closes — both stay exempt. The + * swallowing shape is the sequence neither covers: an interior close of an + * element open at the marker FOLLOWED BY an interior open of verdict + * markup — markup after the section that the section deletes must open + * inside it — and rejects the report. A raw CDATA marker anywhere else + * (even after OTHER content inside the stream element) carries the full + * probe and rejects exactly like its comment twin. + */ +function stripOpaqueSections(xml: string): string | null { + if (!xml.includes('(); + const pushOpen = (name: string): void => { + openElements.push(name); + const lower = name.toLowerCase(); + openCounts.set(lower, (openCounts.get(lower) ?? 0) + 1); + }; + // The tag currently being scanned (`-1` = content position), its name, and + // whether it is a closing tag. + let tagStart = -1; + let tagName = ''; + let tagClosing = false; + let quote: '"' | "'" | null = null; + // Non-whitespace content seen since the innermost element's open tag: + // the CDATA exemption models surefire's own writer, whose CDATA starts + // IMMEDIATELY after the stream open tag — a marker with content before + // it is the swallowing shape even inside a stream element. + let contentSinceOpen = true; + const closeTag = (selfClosing: boolean): void => { + if (tagClosing) { + const lower = tagName.toLowerCase(); + if ((openCounts.get(lower) ?? 0) > 0) { + for (let stack = openElements.length - 1; stack >= 0; stack -= 1) { + const name = openElements[stack].toLowerCase(); + const count = (openCounts.get(name) ?? 1) - 1; + if (count === 0) openCounts.delete(name); + else openCounts.set(name, count); + if (name === lower) { + openElements.length = stack; + break; + } + } + } + contentSinceOpen = true; + } else if (!selfClosing && tagName !== '') { + pushOpen(tagName); + contentSinceOpen = false; + } else { + contentSinceOpen = true; + } + tagStart = -1; + tagName = ''; + tagClosing = false; + }; + while (i < xml.length) { + if (tagStart === -1) { + if (xml.startsWith('' : ']]>'; + const end = xml.indexOf(closer, i + (comment ? 4 : 9)); + // Unterminated: rejected fail-closed (see the doc comment) rather + // than kept verbatim, where the body walk would scan it as markup. + if (end === -1) return null; + // The swallowing-shape probe: an interior close of an element open + // at the marker spans across that element's boundary. Applied to + // CDATA too — except the shape surefire's own writer emits, a + // section immediately after an open ``/`` + // tag with no content before it. + const innermost = openElements.at(-1)?.toLowerCase() ?? ''; + const exempt = + !comment && + !contentSinceOpen && + (innermost === 'system-out' || innermost === 'system-err'); + const interior = xml.slice(i + (comment ? 4 : 9), end); + if (exempt) { + // The exempt shape keeps the probe for the one direction the + // surefire-stdout model cannot cover. Legitimate stdout samples + // close the elements open around the section and open-and-close + // self-contained phantom markup — both stay exempt. But markup + // the section SWALLOWS must open inside it AFTER the closes of + // the surrounding elements: a later suite, or a later case of + // this suite. Reject exactly + // that sequence — an interior close of an element open at the + // marker followed by an interior OPEN — pairing interior closes + // against earlier interior opens first, so a self-contained + // sample never trips it. + const interiorToken = + /<(\/)?\s*(testsuite|testcase|failure|error)\b[^<>]*?(\/?)\s*>/gi; + const interiorOpenCounts = new Map(); + let closesSurrounding = false; + let match: RegExpExecArray | null; + while ((match = interiorToken.exec(interior)) !== null) { + const name = match[2].toLowerCase(); + if (match[1]) { + const open = interiorOpenCounts.get(name) ?? 0; + if (open > 0) { + interiorOpenCounts.set(name, open - 1); + } else if ((openCounts.get(name) ?? 0) > 0) { + closesSurrounding = true; + } + } else if (match[3] !== '/') { + if (closesSurrounding) return null; + interiorOpenCounts.set( + name, + (interiorOpenCounts.get(name) ?? 0) + 1, + ); + } + } + } else { + const interiorClose = /<\/\s*([A-Za-z0-9:_.-]+)/gi; + let match: RegExpExecArray | null; + const interiorCloses = new Map(); + while ((match = interiorClose.exec(interior)) !== null) { + const name = match[1].toLowerCase(); + if ((openCounts.get(name) ?? 0) > 0) { + return null; + } + interiorCloses.set(name, (interiorCloses.get(name) ?? 0) + 1); + } + // The mirror probe: an interior OPEN of a verdict-bearing element + // whose close sits after the section straddles the boundary the + // other way — the section deletes the element's header and its + // failure body. Restricted to the names the parse reads: stray + // unclosed fragments of test output (a printed generic type, an + // HTML log) must not reject the report. + const interiorOpen = + /<(testsuite|testcase|failure|error)\b[^<>]*?(\/?)\s*>/gi; + const interiorOpens = new Map(); + while ((match = interiorOpen.exec(interior)) !== null) { + if (match[2] === '/') continue; + const name = match[1].toLowerCase(); + interiorOpens.set(name, (interiorOpens.get(name) ?? 0) + 1); + } + for (const [name, opens] of interiorOpens) { + if (opens > (interiorCloses.get(name) ?? 0)) { + return null; + } + } + } + chunks.push(xml.slice(chunkStart, i)); + i = end + closer.length; + chunkStart = i; + continue; + } + if (xml[i] === '<') { + tagStart = i; + tagClosing = xml[i + 1] === '/'; + tagName = ''; + let nameEnd = i + (tagClosing ? 2 : 1); + while (nameEnd < xml.length && XML_NAME_CHAR.test(xml[nameEnd])) { + tagName += xml[nameEnd]; + nameEnd += 1; + } + i = nameEnd; + continue; + } + if (!/^\s$/.test(xml[i])) contentSinceOpen = true; + i += 1; + continue; + } + const char = xml[i]; + if (quote !== null) { + if (char === quote) quote = null; + i += 1; + continue; + } + if (char === '"' || char === "'") { + quote = char; + i += 1; + continue; + } + if (xml.startsWith('