diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 904d523f8..334164528 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -290,14 +290,19 @@ jobs: - if: needs.changes.outputs.code == 'true' && success() run: npm ci - uses: actions/download-artifact@d3f86a106a0bac45b974a628896c90dbdf5c8093 # v4.3.0 - if: needs.changes.outputs.code == 'true' && success() + if: >- + !cancelled() && needs.changes.outputs.code == 'true' && + (needs.test-shard.result == 'success' || needs.test-shard.result == 'failure') with: path: .vitest/blob pattern: vitest-report-* merge-multiple: true - name: Merge coverage, JUnit, and timing reports id: merge - if: needs.changes.outputs.code == 'true' && needs.changes.outputs.tier == 'full' && success() + if: >- + !cancelled() && needs.changes.outputs.code == 'true' && + needs.changes.outputs.tier == 'full' && + (needs.test-shard.result == 'success' || needs.test-shard.result == 'failure') run: >- npm run test:ci:merge -- --coverage.reportsDirectory=coverage @@ -312,7 +317,10 @@ jobs: --outputFile.json=test-results.json - name: Merge fast-suite JUnit and timing reports id: merge-fast - if: needs.changes.outputs.code == 'true' && needs.changes.outputs.tier == 'fast' && success() + if: >- + !cancelled() && needs.changes.outputs.code == 'true' && + needs.changes.outputs.tier == 'fast' && + (needs.test-shard.result == 'success' || needs.test-shard.result == 'failure') run: >- npm exec -- vitest --merge-reports --reporter=default diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 96d8010ee..e80d903a3 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -182,8 +182,7 @@ The separate GitHub Pages host uses only the manual ## Development feedback and PR verification For routine merges, wait for the required checks to pass for the current PR -revision against the current `main`. Branch protection requires up-to-date -branches; administrators retain an override for deliberate maintainer recovery. +revision. Administrators retain an override for deliberate maintainer recovery. If `main` is broken, prioritize a focused repair before updating affected PRs. Select checks for the behavior changed. For a source edit: diff --git a/src/application/wasm/WasmArtifactService.ts b/src/application/wasm/WasmArtifactService.ts index 25b844e54..fc68acc90 100644 --- a/src/application/wasm/WasmArtifactService.ts +++ b/src/application/wasm/WasmArtifactService.ts @@ -6,6 +6,7 @@ import { AnalysisCancelledError, AnalysisInputError, AnalysisOutputError, + type AnalysisInputIssue, } from "../../domain/analysisErrorCore.js"; import { createEvidence, type Evidence } from "../../domain/evidence.js"; import { @@ -41,22 +42,29 @@ export class WasmArtifactService { cause: input.error, }), ); - if ( - ![ - input.data.path, - ...input.data.glue_paths, - ...input.data.candidate_paths, - ].every(isAbsolute) - ) - return err( - new AnalysisInputError(OPERATION, undefined, [ - { - path: ["path"], - reason: "invalid_format", - message: "Expected an absolute filesystem path on this host.", - }, - ]), - ); + const pathIssues = [ + { value: input.data.path, path: ["path"] }, + ...input.data.glue_paths.map((value, index) => ({ + value, + path: ["glue_paths", index], + })), + ...input.data.candidate_paths.map((value, index) => ({ + value, + path: ["candidate_paths", index], + })), + ].flatMap(({ value, path }) => + isAbsolute(value) + ? [] + : [ + { + path, + reason: "invalid_format", + message: "Expected an absolute filesystem path on this host.", + }, + ], + ); + if (pathIssues.length > 0) + return err(new AnalysisInputError(OPERATION, undefined, pathIssues)); const inspected = await this.provider.inspect(input.data, options); if (!inspected.ok) return inspected; if (options?.signal?.aborted) diff --git a/src/domain/reconstructionVerification.functionSources.test.ts b/src/domain/reconstructionVerification.functionSources.test.ts index e23d37d74..8f25ccb36 100644 --- a/src/domain/reconstructionVerification.functionSources.test.ts +++ b/src/domain/reconstructionVerification.functionSources.test.ts @@ -44,6 +44,16 @@ const dossier = jsonValueSchema.parse( referenced_names: [], native_api: null, native_value_flow: null, + facet_availability: { + pseudocode: { available: true }, + assembly: { available: true }, + comments: { available: true }, + callers: { available: true }, + callees: { available: true }, + references: { available: true }, + strings_names: { available: true }, + cfg: { available: true }, + }, basic_blocks: [{ start: "0x1000", end: "0x1001", successors: [] }], limitations: [], }), diff --git a/tests/boundary/filesystem/ciTestReports.test.ts b/tests/boundary/filesystem/ciTestReports.test.ts index 06212ad07..3e2aadcd4 100644 --- a/tests/boundary/filesystem/ciTestReports.test.ts +++ b/tests/boundary/filesystem/ciTestReports.test.ts @@ -45,23 +45,34 @@ const workflow = async () => ), ).jobs; -it("collects failed-shard diagnostics before enforcing the shard result", async () => { +it("merges available failed-shard diagnostics and skips unavailable shard reports", async () => { const steps = (await workflow()).test!.steps; + const download = steps.find( + (step) => + step.uses?.includes("actions/download-artifact") && + step.with?.pattern === "vitest-report-*", + ); const merge = steps.findIndex((step) => step.run?.includes("test:ci:merge")); const upload = steps.findIndex((step) => step.with?.name === "test-results"); const gate = steps.findIndex( (step) => step.env?.SHARD_RESULT && step.run?.includes("test "), ); expect(merge).toBeGreaterThan(0); + expect(download?.if).toContain("!cancelled()"); + expect(download?.if).toContain("needs.test-shard.result == 'success'"); + expect(download?.if).toContain("needs.test-shard.result == 'failure'"); expect(upload).toBeGreaterThan(merge); expect(gate).toBeGreaterThan(upload); - expect(steps[merge]?.if).not.toContain("needs.test-shard.result"); const merges = steps.filter((step) => step.run?.includes("--reporter=json")); expect(merges).toHaveLength(2); - for (const step of merges) + for (const step of merges) { expect(step.run).toContain( "--reporter=./scripts/ci/test-timing-reporter.mjs", ); + expect(step.if).toContain("!cancelled()"); + expect(step.if).toContain("needs.test-shard.result == 'success'"); + expect(step.if).toContain("needs.test-shard.result == 'failure'"); + } expect(steps[upload]?.with?.path).toContain("test-timings.json"); expect(steps[upload]?.with).toMatchObject({ "include-hidden-files": true }); expect(steps[upload]?.with?.path).toContain( diff --git a/tests/boundary/mcp/wasmArtifactMcp.test.ts b/tests/boundary/mcp/wasmArtifactMcp.test.ts index 04c5e59da..91c63fc8e 100644 --- a/tests/boundary/mcp/wasmArtifactMcp.test.ts +++ b/tests/boundary/mcp/wasmArtifactMcp.test.ts @@ -215,6 +215,97 @@ it("advertises exact valid schemas and records inline WASM interface evidence wi expect(close).toHaveBeenCalled(); }); +it("attributes rejected WASM paths to their selected fields before provider work", async () => { + const value = wasmArtifactFixture(); + value.glue = [ + { + artifact: { path: "/glue.js", sha256: "b".repeat(64), bytes: 0 }, + parse_status: "complete", + references: [], + }, + ]; + value.candidates.push({ + path: "/other.wasm", + sha256: "c".repeat(64), + bytes: 8, + }); + const validInput = { + path: value.artifact.path, + glue_paths: ["/glue.js"], + candidate_paths: ["/other.wasm"], + }; + const inspect = vi.fn(() => + Promise.resolve(ok(wasmArtifactExecution(value))), + ); + const harness = await createApplicationMcpHarness({ + serverOptions: { + wasmArtifact: new WasmArtifactService({ + close: () => Promise.resolve(), + inspect, + }), + }, + }); + onTestFinished(harness.close); + for (const [input, paths] of [ + [{ path: "relative.wasm" }, [["path"]]], + [ + { path: value.artifact.path, glue_paths: ["relative.js"] }, + [["glue_paths", 0]], + ], + [ + { path: value.artifact.path, candidate_paths: ["relative.wasm"] }, + [["candidate_paths", 0]], + ], + [ + { + path: value.artifact.path, + glue_paths: ["/glue.js", "relative.js"], + candidate_paths: ["/other.wasm", "relative.wasm"], + }, + [ + ["glue_paths", 1], + ["candidate_paths", 1], + ], + ], + [ + { + path: "relative.wasm", + glue_paths: ["relative.js"], + candidate_paths: ["relative-candidate.wasm"], + }, + [["path"], ["glue_paths", 0], ["candidate_paths", 0]], + ], + ] as const) { + const response = await harness.client.callTool({ + name: "inspect_wasm_artifact", + arguments: input, + }); + const failure = parseMcpToolError(response).error; + expect(failure.code).toBe("invalid_request"); + const { issues } = z + .object({ issues: z.array(z.unknown()) }) + .parse(failure.details); + expect(issues).toEqual( + paths.map((path) => ({ + path, + reason: "invalid_format", + message: "Expected an absolute filesystem path on this host.", + })), + ); + } + expect(inspect).not.toHaveBeenCalled(); + const valid = await harness.client.callTool({ + name: "inspect_wasm_artifact", + arguments: validInput, + }); + expect(valid.isError).not.toBe(true); + expect(inspect).toHaveBeenCalledExactlyOnceWith( + validInput, + expect.objectContaining({ signal: expect.any(AbortSignal) }), + ); + await harness.client.ping(); +}); + it("rejects a WASM provider response with a non-file subject before recording Evidence", async () => { const value = wasmArtifactFixture(); const execution = wasmArtifactExecution(value);