From 0ad4918b42713f38aecc20f8e6e69fc738906128 Mon Sep 17 00:00:00 2001 From: Petr Plenkov Date: Thu, 13 Aug 2026 02:40:58 +0200 Subject: [PATCH 1/2] v0 Wave J: checks integration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adapted @sverka/checks to work with the new Definition Graph + Run Plan: - Resolver now produces StepDefinition (with shell operations) instead of the old OperationSpec - New synthesizeCheckSteps: converts ProposedChecks → StepDefinition[] for inclusion in a Definition Graph - extractFindings reused unchanged (SARIF normalization) - Check steps use ID pattern checks/, runtime mode host - Deduplicates by checkId, skips unresolved checks 42 checks tests pass (7 new synthesize + 35 existing). No any types. Generated with [Devin](https://devin.ai) Co-Authored-By: Devin <158243242+devin-ai-integration[bot]@users.noreply.github.com> --- .../checks/src/__tests__/public-api.test.ts | 6 +- .../checks/src/__tests__/resolver.test.ts | 68 ++++------ .../checks/src/__tests__/synthesize.test.ts | 80 ++++++++++++ packages/checks/src/index.ts | 3 +- packages/checks/src/resolver.ts | 62 +++------ packages/checks/src/synthesize.ts | 31 +++++ specs/14-checks/spec.md | 121 ++++++++++++++++-- 7 files changed, 275 insertions(+), 96 deletions(-) create mode 100644 packages/checks/src/__tests__/synthesize.test.ts create mode 100644 packages/checks/src/synthesize.ts diff --git a/packages/checks/src/__tests__/public-api.test.ts b/packages/checks/src/__tests__/public-api.test.ts index 02fcb9ab9..6e157699b 100644 --- a/packages/checks/src/__tests__/public-api.test.ts +++ b/packages/checks/src/__tests__/public-api.test.ts @@ -12,6 +12,10 @@ describe("public API", () => { expect(typeof api.createBuiltinResolver).toBe("function"); }); + it("exports synthesizeCheckSteps function", () => { + expect(typeof api.synthesizeCheckSteps).toBe("function"); + }); + it("exports extractFindings function", () => { expect(typeof api.extractFindings).toBe("function"); }); @@ -23,6 +27,6 @@ describe("public API", () => { it("does not export unexpected runtime values", () => { const runtimeKeys = Object.keys(api).sort(); - expect(runtimeKeys).toEqual(["CheckError", "createBuiltinResolver", "extractFindings"]); + expect(runtimeKeys).toEqual(["CheckError", "createBuiltinResolver", "extractFindings", "synthesizeCheckSteps"]); }); }); diff --git a/packages/checks/src/__tests__/resolver.test.ts b/packages/checks/src/__tests__/resolver.test.ts index a3e2d5238..ebb5337ed 100644 --- a/packages/checks/src/__tests__/resolver.test.ts +++ b/packages/checks/src/__tests__/resolver.test.ts @@ -11,26 +11,23 @@ describe("createBuiltinResolver — Node (bun)", () => { it("resolves typecheck to bun run typecheck", () => { const r = resolver.resolve(makeCheck("typecheck"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("bun"); - expect(r!.operation.args).toEqual(["run", "typecheck"]); - expect(r!.operation.kind).toBe("check"); - expect(r!.operation.id).toBe("prop-typecheck"); - expect(r!.operation.name).toBe("typecheck"); - expect(r!.operation.description).toBe("test"); + expect(r!.step.id).toBe("checks/typecheck"); + expect(r!.step.operations).toHaveLength(1); + expect(r!.step.operations[0]!.kind).toBe("shell"); + expect((r!.step.operations[0] as { command: string }).command).toBe("bun run typecheck"); + expect(r!.step.runtime.mode).toBe("host"); }); it("resolves lint to bun run lint", () => { const r = resolver.resolve(makeCheck("lint"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("bun"); - expect(r!.operation.args).toEqual(["run", "lint"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("bun run lint"); }); it("resolves test to bun run test", () => { const r = resolver.resolve(makeCheck("test"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("bun"); - expect(r!.operation.args).toEqual(["run", "test"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("bun run test"); }); }); @@ -38,22 +35,19 @@ describe("createBuiltinResolver — Node (npm/yarn/pnpm)", () => { it("resolves typecheck with npm", () => { const r = resolver.resolve(makeCheck("typecheck"), makeContext(["npm"])); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("npm"); - expect(r!.operation.args).toEqual(["run", "typecheck"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("npm run typecheck"); }); it("resolves lint with yarn", () => { const r = resolver.resolve(makeCheck("lint"), makeContext(["yarn"])); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("yarn"); - expect(r!.operation.args).toEqual(["run", "lint"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("yarn run lint"); }); it("resolves test with pnpm", () => { const r = resolver.resolve(makeCheck("test"), makeContext(["pnpm"])); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("pnpm"); - expect(r!.operation.args).toEqual(["run", "test"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("pnpm run test"); }); }); @@ -61,15 +55,13 @@ describe("createBuiltinResolver — Python", () => { it("resolves lint to ruff check", () => { const r = resolver.resolve(makeCheck("lint"), makeContext(["poetry"])); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("ruff"); - expect(r!.operation.args).toEqual(["check"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("ruff check"); }); it("resolves test to pytest", () => { const r = resolver.resolve(makeCheck("test"), makeContext(["pip"])); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("pytest"); - expect(r!.operation.args).toEqual([]); + expect((r!.step.operations[0] as { command: string }).command).toBe("pytest"); }); }); @@ -79,22 +71,19 @@ describe("createBuiltinResolver — Rust", () => { it("resolves clippy to cargo clippy", () => { const r = resolver.resolve(makeCheck("clippy"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("cargo"); - expect(r!.operation.args).toEqual(["clippy"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("cargo clippy"); }); it("resolves fmt-check to cargo fmt --check", () => { const r = resolver.resolve(makeCheck("fmt-check"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("cargo"); - expect(r!.operation.args).toEqual(["fmt", "--check"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("cargo fmt --check"); }); it("resolves test to cargo test", () => { const r = resolver.resolve(makeCheck("test"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("cargo"); - expect(r!.operation.args).toEqual(["test"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("cargo test"); }); }); @@ -104,15 +93,13 @@ describe("createBuiltinResolver — Go", () => { it("resolves vet to go vet ./...", () => { const r = resolver.resolve(makeCheck("vet"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("go"); - expect(r!.operation.args).toEqual(["vet", "./..."]); + expect((r!.step.operations[0] as { command: string }).command).toBe("go vet ./..."); }); it("resolves test to go test ./...", () => { const r = resolver.resolve(makeCheck("test"), ctx); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("go"); - expect(r!.operation.args).toEqual(["test", "./..."]); + expect((r!.step.operations[0] as { command: string }).command).toBe("go test ./..."); }); }); @@ -133,16 +120,14 @@ describe("createBuiltinResolver — multiple package managers", () => { const r = resolver.resolve(makeCheck("test"), makeContext(["cargo", "bun"])); expect(r).not.toBeNull(); // Node entries come before cargo in table order, so bun wins. - expect(r!.operation.command).toBe("bun"); - expect(r!.operation.args).toEqual(["run", "test"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("bun run test"); }); it("honours proposal reason over table order in polyglot projects", () => { const rustCheck = makeCheck("test", "Rust project defaults"); const r = resolver.resolve(rustCheck, makeContext(["cargo", "bun"])); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("cargo"); - expect(r!.operation.args).toEqual(["test"]); + expect((r!.step.operations[0] as { command: string }).command).toBe("cargo test"); }); }); @@ -173,12 +158,13 @@ describe("custom CheckResolver", () => { resolve() { return { checkId: "custom", - operation: { - id: "op-1", - kind: "check", - name: "custom", - command: "my-tool", - args: ["--sarif", "out.sarif"], + step: { + id: "checks/custom", + runtime: { mode: "host" }, + operations: [{ kind: "shell", command: "my-tool --sarif out.sarif" }], + inputs: [], + outputs: [], + dependencies: [], }, outputs: [{ path: "out.sarif", format: "sarif" }], }; @@ -186,7 +172,7 @@ describe("custom CheckResolver", () => { }; const r = custom.resolve(makeCheck("custom"), makeContext([])); expect(r).not.toBeNull(); - expect(r!.operation.command).toBe("my-tool"); + expect((r!.step.operations[0] as { command: string }).command).toBe("my-tool --sarif out.sarif"); expect(r!.outputs).toHaveLength(1); expect(r!.outputs[0]!.format).toBe("sarif"); }); diff --git a/packages/checks/src/__tests__/synthesize.test.ts b/packages/checks/src/__tests__/synthesize.test.ts new file mode 100644 index 000000000..d87a893d7 --- /dev/null +++ b/packages/checks/src/__tests__/synthesize.test.ts @@ -0,0 +1,80 @@ +import { describe, it, expect } from "vitest"; +import { synthesizeCheckSteps } from "../synthesize.js"; +import { createBuiltinResolver } from "../resolver.js"; +import type { CheckResolver } from "../resolver.js"; +import { makeCheck, makeContext } from "./helpers/fixtures.js"; + +describe("synthesizeCheckSteps", () => { + it("converts proposed checks to StepDefinitions", () => { + const ctx = makeContext(["bun"]); + const checks = [makeCheck("typecheck"), makeCheck("lint"), makeCheck("test")]; + const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(steps).toHaveLength(3); + expect(steps[0]!.id).toBe("checks/typecheck"); + expect(steps[1]!.id).toBe("checks/lint"); + expect(steps[2]!.id).toBe("checks/test"); + }); + + it("skips checks that fail resolution", () => { + const ctx = makeContext(["bun"]); + const checks = [makeCheck("typecheck"), makeCheck("clippy"), makeCheck("lint")]; + // clippy requires cargo, not bun — should be skipped. + const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(steps).toHaveLength(2); + expect(steps.map((s) => s.id)).toEqual(["checks/typecheck", "checks/lint"]); + }); + + it("deduplicates by checkId", () => { + const ctx = makeContext(["bun"]); + const checks = [makeCheck("typecheck"), makeCheck("typecheck"), makeCheck("lint")]; + const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(steps).toHaveLength(2); + expect(steps.map((s) => s.id)).toEqual(["checks/typecheck", "checks/lint"]); + }); + + it("step IDs follow checks/ pattern", () => { + const ctx = makeContext(["bun"]); + const checks = [makeCheck("typecheck")]; + const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(steps[0]!.id).toMatch(/^checks\//); + }); + + it("steps have runtime.mode === host", () => { + const ctx = makeContext(["bun"]); + const checks = [makeCheck("typecheck"), makeCheck("lint"), makeCheck("test")]; + const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + for (const step of steps) { + expect(step.runtime.mode).toBe("host"); + } + }); + + it("returns empty array for empty input", () => { + const ctx = makeContext(["bun"]); + const steps = synthesizeCheckSteps([], ctx, createBuiltinResolver()); + expect(steps).toHaveLength(0); + }); + + it("works with custom resolver", () => { + const custom: CheckResolver = { + resolve(check) { + return { + checkId: check.checkId, + step: { + id: `checks/${check.checkId}`, + runtime: { mode: "host" }, + operations: [{ kind: "shell", command: "echo hello" }], + inputs: [], + outputs: [], + dependencies: [], + }, + outputs: [], + }; + }, + }; + const ctx = makeContext([]); + const checks = [makeCheck("custom1"), makeCheck("custom2")]; + const steps = synthesizeCheckSteps(checks, ctx, custom); + expect(steps).toHaveLength(2); + expect(steps[0]!.id).toBe("checks/custom1"); + }); +}); diff --git a/packages/checks/src/index.ts b/packages/checks/src/index.ts index 468802d5d..0619e94f2 100644 --- a/packages/checks/src/index.ts +++ b/packages/checks/src/index.ts @@ -1,6 +1,7 @@ -// @sverka/checks — public API +// @sverka/checks — public API. Spec 14. export type { CheckResolver, ResolvedCheck, CheckOutput } from "./resolver.js"; export { createBuiltinResolver } from "./resolver.js"; +export { synthesizeCheckSteps } from "./synthesize.js"; export { extractFindings } from "./extract.js"; export { CheckError, type CheckErrorCode } from "./errors.js"; diff --git a/packages/checks/src/resolver.ts b/packages/checks/src/resolver.ts index 75806a218..99f1510ea 100644 --- a/packages/checks/src/resolver.ts +++ b/packages/checks/src/resolver.ts @@ -1,6 +1,10 @@ +// Check resolver — resolves ProposedCheck into StepDefinition. +// Spec 14 — §24, §25. Reuses the existing resolution table but produces +// StepDefinition (new graph model) instead of the old OperationSpec. + import { existsSync, readFileSync } from "node:fs"; import { resolve } from "node:path"; -import type { OperationSpec } from "@sverka/core"; +import type { StepDefinition } from "@sverka/core"; import type { ProposedCheck, ProjectContext, @@ -8,21 +12,20 @@ import type { } from "@sverka/planner"; /** - * Resolves a ProposedCheck into an executable OperationSpec with output - * declarations. Returns null when the resolver has no mapping for the - * given check + context (the caller skips the check). + * Resolves a ProposedCheck into a ResolvedCheck containing a StepDefinition. + * Returns null when the resolver has no mapping for the given check + context. */ export interface CheckResolver { resolve(check: ProposedCheck, ctx: ProjectContext): ResolvedCheck | null; } /** - * A fully-resolved check: an OperationSpec for the IR plus output - * declarations for findings extraction. + * A fully-resolved check: a StepDefinition for the Definition Graph plus + * output declarations for findings extraction. */ export interface ResolvedCheck { readonly checkId: string; - readonly operation: OperationSpec; + readonly step: StepDefinition; readonly outputs: readonly CheckOutput[]; } @@ -30,7 +33,6 @@ export interface ResolvedCheck { * An output file a check produces, used for findings extraction. */ export interface CheckOutput { - /** Relative path within the artifact directory. */ readonly path: string; readonly format: "sarif" | "json" | "junit" | "text"; } @@ -39,19 +41,12 @@ type PmName = DetectedPackageManager["name"]; interface TableEntry { readonly checkId: string; - /** Proposal reason this entry matches (e.g. "Node project defaults"). */ readonly reason: string; readonly packageManagers: readonly PmName[]; readonly command: string; readonly args: readonly string[]; } -/** - * Built-in resolution table. Order matters: the first matching entry - * (by checkId + packageManager) wins. Node entries come before Python, - * Rust, and Go so that Node checks take precedence when multiple - * package managers are present. - */ const NODE_REASON = "Node project defaults"; const PYTHON_REASON = "Python project defaults"; const RUST_REASON = "Rust project defaults"; @@ -59,42 +54,28 @@ const GO_REASON = "Go project defaults"; const KNOWN_REASONS = new Set([NODE_REASON, PYTHON_REASON, RUST_REASON, GO_REASON]); const TABLE: readonly TableEntry[] = [ - // Node — typecheck { checkId: "typecheck", reason: NODE_REASON, packageManagers: ["bun"], command: "bun", args: ["run", "typecheck"] }, { checkId: "typecheck", reason: NODE_REASON, packageManagers: ["npm"], command: "npm", args: ["run", "typecheck"] }, { checkId: "typecheck", reason: NODE_REASON, packageManagers: ["yarn"], command: "yarn", args: ["run", "typecheck"] }, { checkId: "typecheck", reason: NODE_REASON, packageManagers: ["pnpm"], command: "pnpm", args: ["run", "typecheck"] }, - // Node — lint { checkId: "lint", reason: NODE_REASON, packageManagers: ["bun"], command: "bun", args: ["run", "lint"] }, { checkId: "lint", reason: NODE_REASON, packageManagers: ["npm"], command: "npm", args: ["run", "lint"] }, { checkId: "lint", reason: NODE_REASON, packageManagers: ["yarn"], command: "yarn", args: ["run", "lint"] }, { checkId: "lint", reason: NODE_REASON, packageManagers: ["pnpm"], command: "pnpm", args: ["run", "lint"] }, - // Node — test { checkId: "test", reason: NODE_REASON, packageManagers: ["bun"], command: "bun", args: ["run", "test"] }, { checkId: "test", reason: NODE_REASON, packageManagers: ["npm"], command: "npm", args: ["run", "test"] }, { checkId: "test", reason: NODE_REASON, packageManagers: ["yarn"], command: "yarn", args: ["run", "test"] }, { checkId: "test", reason: NODE_REASON, packageManagers: ["pnpm"], command: "pnpm", args: ["run", "test"] }, - // Python — lint { checkId: "lint", reason: PYTHON_REASON, packageManagers: ["pip", "poetry", "uv", "pipenv"], command: "ruff", args: ["check"] }, - // Python — test { checkId: "test", reason: PYTHON_REASON, packageManagers: ["pip", "poetry", "uv", "pipenv"], command: "pytest", args: [] }, - // Rust { checkId: "clippy", reason: RUST_REASON, packageManagers: ["cargo"], command: "cargo", args: ["clippy"] }, { checkId: "fmt-check", reason: RUST_REASON, packageManagers: ["cargo"], command: "cargo", args: ["fmt", "--check"] }, { checkId: "test", reason: RUST_REASON, packageManagers: ["cargo"], command: "cargo", args: ["test"] }, - // Go { checkId: "vet", reason: GO_REASON, packageManagers: ["go"], command: "go", args: ["vet", "./..."] }, { checkId: "test", reason: GO_REASON, packageManagers: ["go"], command: "go", args: ["test", "./..."] }, ]; -/** - * Built-in resolver backed by a (checkId, packageManager) → command table. - * Covers the 6 checkIds the planner proposes across Node/Python/Rust/Go. - * Never throws — returns null for unknown mappings. - * - * Table order determines precedence: Node entries come before Python/Rust/Go, - * so when multiple package managers are present the first matching entry wins. - */ +/** Create the built-in check resolver backed by the resolution table. */ export function createBuiltinResolver(): CheckResolver { return { resolve(check, ctx) { @@ -113,25 +94,24 @@ function findEntry( ): ResolvedCheck | null { for (const entry of TABLE) { if (entry.checkId !== check.checkId) continue; - // When the planner supplies a known ecosystem reason, require an exact match - // so polyglot projects resolve to the correct tool instead of table order. if (KNOWN_REASONS.has(check.reason) && entry.reason !== check.reason) continue; if (!entry.packageManagers.some((pm) => pmNames.includes(pm))) continue; if (!isEntryApplicable(entry, ctx.root, rootPkg)) continue; - const operation: OperationSpec = { - id: check.id, - kind: "check", - name: check.checkId, - description: check.reason, - command: entry.command, - args: entry.args, + + const command = [entry.command, ...entry.args].join(" "); + const step: StepDefinition = { + id: `checks/${check.checkId}`, + runtime: { mode: "host" }, + operations: [{ kind: "shell", command }], + inputs: [], + outputs: [], + dependencies: [], }; - return { checkId: check.checkId, operation, outputs: [] }; + return { checkId: check.checkId, step, outputs: [] }; } return null; } -/** Validate Node entries against the root package.json (packageManager field and scripts). */ function isEntryApplicable( entry: TableEntry, root: string, diff --git a/packages/checks/src/synthesize.ts b/packages/checks/src/synthesize.ts new file mode 100644 index 000000000..14e342733 --- /dev/null +++ b/packages/checks/src/synthesize.ts @@ -0,0 +1,31 @@ +// Check step synthesis — converts ProposedChecks into StepDefinitions. +// Spec 14 — §24, §25. + +import type { StepDefinition } from "@sverka/core"; +import type { ProposedCheck, ProjectContext } from "@sverka/planner"; +import type { CheckResolver } from "./resolver.js"; + +/** + * Convert proposed checks into StepDefinitions for inclusion in a + * Definition Graph. Checks that fail resolution (resolver returns null) + * are skipped. Duplicate checkIds are deduplicated — only the first + * resolved check for each checkId is included. + */ +export function synthesizeCheckSteps( + checks: readonly ProposedCheck[], + ctx: ProjectContext, + resolver: CheckResolver, +): readonly StepDefinition[] { + const steps: StepDefinition[] = []; + const seen = new Set(); + + for (const check of checks) { + if (seen.has(check.checkId)) continue; + const resolved = resolver.resolve(check, ctx); + if (!resolved) continue; + seen.add(resolved.checkId); + steps.push(resolved.step); + } + + return steps; +} diff --git a/specs/14-checks/spec.md b/specs/14-checks/spec.md index 3f925cf3e..c7b4e737b 100644 --- a/specs/14-checks/spec.md +++ b/specs/14-checks/spec.md @@ -1,34 +1,131 @@ -# Spec 14 — Checks +# Spec 14 — Checks Integration -**Status:** Stub — to be written by architect during the corresponding wave. -**Source:** specs/architecture-spec.md (authoritative) +**Status:** Active +**Source:** specs/architecture-spec.md §24, §25 +**Package:** `@sverka/checks` (adapted) ## Overview -TODO — architect fills in during wave design phase. Reference the architecture -spec section(s) listed in the reconciliation plan -(engdocs/architecture/v0-architecture-spec-reconciliation.md). +The checks package bridges the planner's `ProposedCheck` output and the +Definition Graph. It resolves proposed checks into `StepDefinition` +objects with shell operations, and extracts findings from check output +files (SARIF). It integrates with the planner (discovery → proposed +checks) and the engine (Run Plan execution → findings extraction). ## Goals -TODO +- `CheckResolver`: resolves a `ProposedCheck` into a `ResolvedCheck` + containing a `StepDefinition` with shell operations +- `createBuiltinResolver()`: resolver backed by the existing resolution + table (checkId + packageManager → command) +- `synthesizeCheckSteps(proposedChecks, ctx, resolver)`: converts + proposed checks into `StepDefinition[]` for inclusion in a Definition + Graph +- `extractFindings(outputs, artifactDir, checkId)`: extracts findings + from SARIF output files (reused from existing implementation) +- Integration with planner: `ProposedCheck` → `ResolvedCheck` → `StepDefinition` +- Integration with engine: check runs as shell step, outputs extracted + post-execution ## Non-goals -TODO +- Plugin-based custom resolvers (Wave E — Plugin model) +- Non-SARIF output formats (deferred — only SARIF in v0) +- Check caching (§32 — deferred) +- Check retry policy (§32 — deferred) +- Capability manifest analysis (Wave E) +- Provider-native check actions (deferred) ## Interfaces -TODO +```ts +import type { StepDefinition } from "@sverka/core"; +import type { ProposedCheck, ProjectContext } from "@sverka/planner"; +import type { Finding } from "@sverka/findings"; + +interface CheckResolver { + resolve(check: ProposedCheck, ctx: ProjectContext): ResolvedCheck | null; +} + +interface ResolvedCheck { + readonly checkId: string; + readonly step: StepDefinition; + readonly outputs: readonly CheckOutput[]; +} + +interface CheckOutput { + readonly path: string; + readonly format: "sarif" | "json" | "junit" | "text"; +} + +function createBuiltinResolver(): CheckResolver; +function synthesizeCheckSteps( + checks: readonly ProposedCheck[], + ctx: ProjectContext, + resolver: CheckResolver, +): readonly StepDefinition[]; +async function extractFindings( + outputs: readonly CheckOutput[], + artifactDir: string, + checkId: string, +): Promise; +``` + +### Exports + +```ts +export type { CheckResolver, ResolvedCheck, CheckOutput }; +export { createBuiltinResolver, synthesizeCheckSteps, extractFindings }; +export { CheckError }; +export type { CheckErrorCode }; +``` ## Data models -TODO +**Resolution**: The resolver table maps `(checkId, packageManager)` → +`(command, args)`. The resolver creates a `StepDefinition` with: +- `id`: `checks/` (e.g., `checks/typecheck`) +- `runtime`: `{ mode: "host" }` (checks run on host in v0) +- `operations`: a single `shell` operation with `command` and `args` + joined as a shell command string +- `inputs`: `[]` (no inputs in v0) +- `outputs`: `[]` (findings extracted post-execution, not via graph outputs) +- `dependencies`: `[]` (checks are independent by default) + +**Synthesis**: `synthesizeCheckSteps` iterates proposed checks, resolves +each via the resolver, collects the resulting `StepDefinition[]`. Checks +that fail resolution (resolver returns null) are skipped. + +**Step ID generation**: Check steps use the ID pattern `checks/`. +If multiple checks have the same checkId (e.g., from different ecosystems), +they are deduplicated — only the first resolved check for each checkId +is included. + +**Findings extraction**: Reused unchanged from existing implementation. +Reads SARIF files from the artifact directory, normalizes via +`@sverka/findings.normalizeSarif`, returns `Finding[]`. ## Error handling -TODO +Reuses existing `CheckError` with codes: +- `RESOLUTION_FAILED`: check resolution failed +- `EXTRACTION_FAILED`: SARIF extraction or normalization failed ## Test plan -TODO +1. `createBuiltinResolver`: returns a CheckResolver. +2. Resolver resolves typecheck for Node/bun → StepDefinition with shell command. +3. Resolver resolves lint for Node/npm → StepDefinition. +4. Resolver returns null for unknown checkId. +5. Resolver validates package.json scripts (Node entries). +6. `synthesizeCheckSteps`: converts proposed checks to StepDefinition[]. +7. `synthesizeCheckSteps`: skips checks that fail resolution. +8. `synthesizeCheckSteps`: deduplicates by checkId. +9. `synthesizeCheckSteps`: step IDs follow `checks/` pattern. +10. `synthesizeCheckSteps`: steps have `runtime.mode === "host"`. +11. `extractFindings`: extracts findings from SARIF file (reused). +12. `extractFindings`: missing file → skipped (no findings). +13. `extractFindings`: invalid SARIF → throws CheckError. +14. `CheckError`: extends Error with code and cause. +15. Public API: all exports present, no any types. +16. Regression: existing resolver table entries still work. From 110ccbb41d3d8f4b70d672a82357e0254413c34f Mon Sep 17 00:00:00 2001 From: Petr Plenkov Date: Thu, 13 Aug 2026 10:31:49 +0000 Subject: [PATCH 2/2] fix(checks): address v0-j-checks review threads - Set runtime.workingDir to ctx.root in resolved check steps. - Shell-quote each argument before joining the command string. - Return ResolvedCheck[] from synthesizeCheckSteps and preserve resolver outputs. - Use resolved.checkId for both deduplication and generated step id. - Update synthesize/resolver tests and spec 14. Co-Authored-By: Petr Plenkov --- .act-replies-43.tsv | 7 +++ .../checks/src/__tests__/resolver.test.ts | 1 + .../checks/src/__tests__/synthesize.test.ts | 47 ++++++++++--------- packages/checks/src/resolver.ts | 11 ++++- packages/checks/src/synthesize.ts | 24 ++++++---- specs/14-checks/spec.md | 17 +++---- 6 files changed, 65 insertions(+), 42 deletions(-) create mode 100644 .act-replies-43.tsv diff --git a/.act-replies-43.tsv b/.act-replies-43.tsv new file mode 100644 index 000000000..a1de9c968 --- /dev/null +++ b/.act-replies-43.tsv @@ -0,0 +1,7 @@ +PRRT_kwDOTyoI9s6YxeZ4 Fixed: resolved steps now set runtime.workingDir to ctx.root so checks execute in the project directory. +PRRT_kwDOTyoI9s6Yxe3t Fixed: synthesizeCheckSteps now returns ResolvedCheck[] and preserves resolver outputs for findings extraction. +PRRT_kwDOTyoI9s6Yxgj9 Fixed: shell command arguments are individually quoted before joining, preventing unsafe space-join behavior. +PRRT_kwDOTyoI9s6YxgkA Fixed: synthesis now preserves ResolvedCheck.outputs instead of discarding them. +PRRT_kwDOTyoI9s6YxgkD Fixed: deduplication and the generated step ID both use resolved.checkId consistently (checks/${resolved.checkId}). +PRRT_kwDOTyoI9s6YxkDh Fixed: shell command args are safely quoted and runtime.workingDir is set to ctx.root. +PRRT_kwDOTyoI9s6YxkDs Fixed: synthesizeCheckSteps preserves outputs and uses the same resolved.checkId for dedup and step id. diff --git a/packages/checks/src/__tests__/resolver.test.ts b/packages/checks/src/__tests__/resolver.test.ts index ebb5337ed..bacfd642f 100644 --- a/packages/checks/src/__tests__/resolver.test.ts +++ b/packages/checks/src/__tests__/resolver.test.ts @@ -16,6 +16,7 @@ describe("createBuiltinResolver — Node (bun)", () => { expect(r!.step.operations[0]!.kind).toBe("shell"); expect((r!.step.operations[0] as { command: string }).command).toBe("bun run typecheck"); expect(r!.step.runtime.mode).toBe("host"); + expect(r!.step.runtime.workingDir).toBe(ctx.root); }); it("resolves lint to bun run lint", () => { diff --git a/packages/checks/src/__tests__/synthesize.test.ts b/packages/checks/src/__tests__/synthesize.test.ts index d87a893d7..deb3e5de2 100644 --- a/packages/checks/src/__tests__/synthesize.test.ts +++ b/packages/checks/src/__tests__/synthesize.test.ts @@ -8,53 +8,53 @@ describe("synthesizeCheckSteps", () => { it("converts proposed checks to StepDefinitions", () => { const ctx = makeContext(["bun"]); const checks = [makeCheck("typecheck"), makeCheck("lint"), makeCheck("test")]; - const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); - expect(steps).toHaveLength(3); - expect(steps[0]!.id).toBe("checks/typecheck"); - expect(steps[1]!.id).toBe("checks/lint"); - expect(steps[2]!.id).toBe("checks/test"); + const resolved = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(resolved).toHaveLength(3); + expect(resolved[0]!.step.id).toBe("checks/typecheck"); + expect(resolved[1]!.step.id).toBe("checks/lint"); + expect(resolved[2]!.step.id).toBe("checks/test"); }); it("skips checks that fail resolution", () => { const ctx = makeContext(["bun"]); const checks = [makeCheck("typecheck"), makeCheck("clippy"), makeCheck("lint")]; // clippy requires cargo, not bun — should be skipped. - const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); - expect(steps).toHaveLength(2); - expect(steps.map((s) => s.id)).toEqual(["checks/typecheck", "checks/lint"]); + const resolved = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(resolved).toHaveLength(2); + expect(resolved.map((r) => r.step.id)).toEqual(["checks/typecheck", "checks/lint"]); }); it("deduplicates by checkId", () => { const ctx = makeContext(["bun"]); const checks = [makeCheck("typecheck"), makeCheck("typecheck"), makeCheck("lint")]; - const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); - expect(steps).toHaveLength(2); - expect(steps.map((s) => s.id)).toEqual(["checks/typecheck", "checks/lint"]); + const resolved = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(resolved).toHaveLength(2); + expect(resolved.map((r) => r.step.id)).toEqual(["checks/typecheck", "checks/lint"]); }); it("step IDs follow checks/ pattern", () => { const ctx = makeContext(["bun"]); const checks = [makeCheck("typecheck")]; - const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); - expect(steps[0]!.id).toMatch(/^checks\//); + const resolved = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + expect(resolved[0]!.step.id).toMatch(/^checks\//); }); it("steps have runtime.mode === host", () => { const ctx = makeContext(["bun"]); const checks = [makeCheck("typecheck"), makeCheck("lint"), makeCheck("test")]; - const steps = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); - for (const step of steps) { - expect(step.runtime.mode).toBe("host"); + const resolved = synthesizeCheckSteps(checks, ctx, createBuiltinResolver()); + for (const r of resolved) { + expect(r.step.runtime.mode).toBe("host"); } }); it("returns empty array for empty input", () => { const ctx = makeContext(["bun"]); - const steps = synthesizeCheckSteps([], ctx, createBuiltinResolver()); - expect(steps).toHaveLength(0); + const resolved = synthesizeCheckSteps([], ctx, createBuiltinResolver()); + expect(resolved).toHaveLength(0); }); - it("works with custom resolver", () => { + it("preserves outputs from custom resolvers", () => { const custom: CheckResolver = { resolve(check) { return { @@ -67,14 +67,15 @@ describe("synthesizeCheckSteps", () => { outputs: [], dependencies: [], }, - outputs: [], + outputs: [{ path: "out.sarif", format: "sarif" }], }; }, }; const ctx = makeContext([]); const checks = [makeCheck("custom1"), makeCheck("custom2")]; - const steps = synthesizeCheckSteps(checks, ctx, custom); - expect(steps).toHaveLength(2); - expect(steps[0]!.id).toBe("checks/custom1"); + const resolved = synthesizeCheckSteps(checks, ctx, custom); + expect(resolved).toHaveLength(2); + expect(resolved[0]!.step.id).toBe("checks/custom1"); + expect(resolved[0]!.outputs).toEqual([{ path: "out.sarif", format: "sarif" }]); }); }); diff --git a/packages/checks/src/resolver.ts b/packages/checks/src/resolver.ts index 99f1510ea..d9ed85af5 100644 --- a/packages/checks/src/resolver.ts +++ b/packages/checks/src/resolver.ts @@ -53,6 +53,13 @@ const RUST_REASON = "Rust project defaults"; const GO_REASON = "Go project defaults"; const KNOWN_REASONS = new Set([NODE_REASON, PYTHON_REASON, RUST_REASON, GO_REASON]); +const SAFE_SHELL_ARG = /^[\w.\/:@=-]+$/; + +function quoteShellArg(arg: string): string { + if (SAFE_SHELL_ARG.test(arg)) return arg; + return `"${arg.replace(/\\/g, "\\\\").replace(/"/g, '\\"')}"`; +} + const TABLE: readonly TableEntry[] = [ { checkId: "typecheck", reason: NODE_REASON, packageManagers: ["bun"], command: "bun", args: ["run", "typecheck"] }, { checkId: "typecheck", reason: NODE_REASON, packageManagers: ["npm"], command: "npm", args: ["run", "typecheck"] }, @@ -98,10 +105,10 @@ function findEntry( if (!entry.packageManagers.some((pm) => pmNames.includes(pm))) continue; if (!isEntryApplicable(entry, ctx.root, rootPkg)) continue; - const command = [entry.command, ...entry.args].join(" "); + const command = [entry.command, ...entry.args.map(quoteShellArg)].join(" "); const step: StepDefinition = { id: `checks/${check.checkId}`, - runtime: { mode: "host" }, + runtime: { mode: "host", workingDir: ctx.root }, operations: [{ kind: "shell", command }], inputs: [], outputs: [], diff --git a/packages/checks/src/synthesize.ts b/packages/checks/src/synthesize.ts index 14e342733..ca872d482 100644 --- a/packages/checks/src/synthesize.ts +++ b/packages/checks/src/synthesize.ts @@ -1,31 +1,37 @@ -// Check step synthesis — converts ProposedChecks into StepDefinitions. +// Check step synthesis — converts ProposedChecks into ResolvedChecks. // Spec 14 — §24, §25. import type { StepDefinition } from "@sverka/core"; import type { ProposedCheck, ProjectContext } from "@sverka/planner"; -import type { CheckResolver } from "./resolver.js"; +import type { CheckResolver, ResolvedCheck } from "./resolver.js"; /** - * Convert proposed checks into StepDefinitions for inclusion in a + * Convert proposed checks into ResolvedChecks for inclusion in a * Definition Graph. Checks that fail resolution (resolver returns null) * are skipped. Duplicate checkIds are deduplicated — only the first - * resolved check for each checkId is included. + * resolved check for each checkId is included. The resolver's + * `outputs` metadata is preserved alongside the generated step. */ export function synthesizeCheckSteps( checks: readonly ProposedCheck[], ctx: ProjectContext, resolver: CheckResolver, -): readonly StepDefinition[] { - const steps: StepDefinition[] = []; +): readonly ResolvedCheck[] { + const result: ResolvedCheck[] = []; const seen = new Set(); for (const check of checks) { - if (seen.has(check.checkId)) continue; const resolved = resolver.resolve(check, ctx); if (!resolved) continue; + if (seen.has(resolved.checkId)) continue; seen.add(resolved.checkId); - steps.push(resolved.step); + + const step: StepDefinition = { + ...resolved.step, + id: `checks/${resolved.checkId}`, + }; + result.push({ ...resolved, step }); } - return steps; + return result; } diff --git a/specs/14-checks/spec.md b/specs/14-checks/spec.md index c7b4e737b..3e3cc5d90 100644 --- a/specs/14-checks/spec.md +++ b/specs/14-checks/spec.md @@ -19,8 +19,8 @@ checks) and the engine (Run Plan execution → findings extraction). - `createBuiltinResolver()`: resolver backed by the existing resolution table (checkId + packageManager → command) - `synthesizeCheckSteps(proposedChecks, ctx, resolver)`: converts - proposed checks into `StepDefinition[]` for inclusion in a Definition - Graph + proposed checks into `ResolvedCheck[]` for inclusion in a Definition + Graph, preserving each resolver's `outputs` metadata - `extractFindings(outputs, artifactDir, checkId)`: extracts findings from SARIF output files (reused from existing implementation) - Integration with planner: `ProposedCheck` → `ResolvedCheck` → `StepDefinition` @@ -63,7 +63,7 @@ function synthesizeCheckSteps( checks: readonly ProposedCheck[], ctx: ProjectContext, resolver: CheckResolver, -): readonly StepDefinition[]; +): readonly ResolvedCheck[]; async function extractFindings( outputs: readonly CheckOutput[], artifactDir: string, @@ -85,16 +85,17 @@ export type { CheckErrorCode }; **Resolution**: The resolver table maps `(checkId, packageManager)` → `(command, args)`. The resolver creates a `StepDefinition` with: - `id`: `checks/` (e.g., `checks/typecheck`) -- `runtime`: `{ mode: "host" }` (checks run on host in v0) +- `runtime`: `{ mode: "host", workingDir: }` (checks run on host in v0) - `operations`: a single `shell` operation with `command` and `args` - joined as a shell command string + joined as a safely-quoted shell command string - `inputs`: `[]` (no inputs in v0) - `outputs`: `[]` (findings extracted post-execution, not via graph outputs) - `dependencies`: `[]` (checks are independent by default) **Synthesis**: `synthesizeCheckSteps` iterates proposed checks, resolves -each via the resolver, collects the resulting `StepDefinition[]`. Checks -that fail resolution (resolver returns null) are skipped. +each via the resolver, collects the resulting `ResolvedCheck[]` while +preserving resolver `outputs`. Checks that fail resolution (resolver returns +null) are skipped. **Step ID generation**: Check steps use the ID pattern `checks/`. If multiple checks have the same checkId (e.g., from different ecosystems), @@ -118,7 +119,7 @@ Reuses existing `CheckError` with codes: 3. Resolver resolves lint for Node/npm → StepDefinition. 4. Resolver returns null for unknown checkId. 5. Resolver validates package.json scripts (Node entries). -6. `synthesizeCheckSteps`: converts proposed checks to StepDefinition[]. +6. `synthesizeCheckSteps`: converts proposed checks to ResolvedCheck[], preserving `outputs`. 7. `synthesizeCheckSteps`: skips checks that fail resolution. 8. `synthesizeCheckSteps`: deduplicates by checkId. 9. `synthesizeCheckSteps`: step IDs follow `checks/` pattern.