Skip to content

Wave 11: checks - #13

Closed
ThePlenkov wants to merge 0 commit into
wave-10-clifrom
wave-11-checks
Closed

ThePlenkov wants to merge 0 commit into
wave-10-clifrom
wave-11-checks

Conversation

@ThePlenkov

@ThePlenkov ThePlenkov commented Aug 10, 2026 •

Copy link
Copy Markdown
Contributor

User description

Summary

  • @sverka/checks package: CheckResolver.resolve() maps ProposedCheck + ProjectContext → ResolvedCheck (OperationSpec + CheckOutput[]). createBuiltinResolver() covers 6 checkIds (typecheck, lint, test, clippy, vet, fmt-check) across Node/Python/Rust/Go. extractFindings() reads SARIF artifacts via @sverka/findings.normalizeSarif.
  • CheckError with 2 codes (RESOLUTION_FAILED, EXTRACTION_FAILED), override readonly cause: unknown per noImplicitOverride.
  • SDK integration: doPlan/doExecute resolve proposed checks into operations, extract findings from SARIF output artifacts after execution. Re-exports added to @sverka/sdk.
  • Spec 11 trimmed 383→215 lines (12 amendments: CheckProvider interface cut, YAML plugins cut, 10 built-in providers → single table resolver, error codes 6→2).

Test plan

  • checks vitest: 32 pass (errors 3, resolver 19, extract 5, public-api 5)
  • SDK vitest: 63 pass (was 60, +3 new: plan-mode auto-discovery, re-exports checks)
  • typecheck: 0 errors (16 projects)
  • lint: 0 errors (16 projects)
  • build: dist/index.mjs + index.d.mts emitted (16 projects)
  • full monorepo: test/typecheck/lint/build green (fresh, --skip-nx-cache)
  • no any types (impl or tests)
  • exports match spec 1:1
  • spec test plan items 1-17 all covered

Generated with Devin


Summary by cubic

Adds @sverka/checks to turn discovered checks into IR operations and extract SARIF findings. Integrates with @sverka/sdk so auto‑discovery plan() returns check ops and execute() loads findings.

  • New Features

    • @sverka/checks: CheckResolver.resolve() → ResolvedCheck (IR OperationSpec + CheckOutput[]); createBuiltinResolver() maps typecheck, lint, test, clippy, vet, fmt-check across Node (bun/npm/yarn/pnpm), Python (ruff/pytest), Rust (cargo), and Go (go).
    • extractFindings() reads SARIF via @sverka/findings.normalizeSarif; skips missing files; invalid SARIF throws CheckError("EXTRACTION_FAILED").
    • SDK: plan() resolves proposed checks into ops; execute() extracts findings from the artifact dir; throws CONFIG_NOT_FOUND when no resolvable checks; re‑exports createBuiltinResolver, extractFindings, CheckError, and types from @sverka/checks.
  • Refactors

    • Spec 11 simplified: removed provider registry and YAML plugins; single table‑driven resolver; error codes reduced to RESOLUTION_FAILED and EXTRACTION_FAILED.
    • Packaging: @sverka/checks ships ESM (.mjs/.d.mts); added deps @sverka/core, @sverka/planner, @sverka/findings; @sverka/sdk now depends on @sverka/checks.

Written for commit 7e38213. Summary will update on new commits.

Review in cubic


CodeAnt-AI Description

Resolve discovered checks into executable operations and extract SARIF findings

What Changed

  • Auto-discovered checks now become runnable operations during planning and execution across Node, Python, Rust, and Go projects
  • Added built-in mappings for typecheck, lint, test, clippy, vet, and format checks, using the detected package manager or tool
  • Execution now collects findings from declared SARIF output files and reports clear extraction errors for invalid files
  • Missing output files and unsupported formats are skipped without failing the run
  • Exposed custom check resolvers, findings extraction, and check errors through the checks package and SDK
  • Added coverage for resolver mappings, SARIF handling, error behavior, public exports, and plan-mode auto-discovery

Impact

✅ Runnable checks in auto-discovered plans
✅ SARIF findings available after check execution
✅ Clearer errors for invalid check results

🔄 Retrigger CodeAnt AI Review

💡 Usage Guide

Checking Your Pull Request

Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.

Talking to CodeAnt AI

Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:

@codeant-ai ask: Your question here

This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.

Example

@codeant-ai ask: Can you suggest a safer alternative to storing this secret?

Preserve Org Learnings with CodeAnt

You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:

@codeant-ai: Your feedback here

This helps CodeAnt AI learn and adapt to your team's coding style and standards.

Example

@codeant-ai: Do not flag unused imports.

Retrigger review

Ask CodeAnt AI to review the PR again, by typing:

@codeant-ai: review

Check Your Repository Health

To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.

@codeant-ai

codeant-ai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

🤖 CodeAnt AI — Review Status

Status Commit Started (UTC) Finished (UTC)
✅ Incremental review completed 7e38213 Aug 11, 2026 · 06:24 06:24
✅ Incremental review completed 0ce578f Aug 11, 2026 · 00:50 00:51
✅ Reviewed your PR a9292a1 Aug 10, 2026 · 08:06 08:09

@coderabbitai

coderabbitai Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Organization UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 8781639d-67d8-4d07-ab86-920159e0b32b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

Comment @coderabbitai help to get the list of available commands.

@codeant-ai codeant-ai Bot added the size:XXL This PR changes 1000+ lines, ignoring generated files label Aug 10, 2026
@baz-reviewer

baz-reviewer Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Merger

Needs Review

The shipped extractFindings path joins untrusted CheckOutput.path values without constraining them to artifactDir, leaving a concrete path-traversal file-read vulnerability despite the resolved discussion. CI also did not run for this non-trivial SDK/execution change.

Commit 3b2803d · Evaluated 2026-08-11 01:19 UTC

Review this PR on Baz | Customize your next review

@amazon-q-developer amazon-q-developer Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Summary

This PR introduces the @sverka/checks package with check resolution and findings extraction. The implementation is well-structured with comprehensive test coverage (32 passing tests). However, there are critical security issues that must be addressed before merge.

Critical Issues

Path Traversal Vulnerability (extract.ts): The file path handling in extractFindings allows arbitrary file access outside the artifact directory through path traversal attacks. This enables reading sensitive files anywhere on the filesystem.

Missing Imports (extract.ts): The path traversal fix requires additional imports (resolve, sep) from "node:path" that are currently missing.

Test Coverage

The PR demonstrates solid test coverage including edge cases (missing files, invalid SARIF, multiple package managers). All 32 tests pass and the implementation correctly handles the documented 6 check types across 4 languages.


You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.

Comment thread packages/checks/src/extract.ts Outdated
Comment thread packages/checks/src/extract.ts Outdated
Comment thread packages/sdk/src/sverka.ts Outdated
@qodo-code-review

Copy link
Copy Markdown

PR Summary by Qodo

Wave 11: add checks resolver, SARIF findings extraction, and SDK wiring

✨ Enhancement 🧪 Tests 📝 Documentation ⚙️ Configuration changes 🕐 40+ Minutes

Grey Divider

AI Description

• Introduce @sverka/checks to resolve ProposedCheck into executable OperationSpec.
• Add SARIF artifact findings extraction via @sverka/findings.normalizeSarif.
• Wire auto-discovery in SDK plan/execute and re-export checks API from @sverka/sdk.
Diagram

graph TD
  sdk["SDK (plan/execute)"] --> planner["Planner"] --> resolver["Checks resolver"] --> ops["OperationSpec[]"] --> runtime["Runtime scheduler"] --> artifacts[("Artifacts dir")] --> extract["extractFindings"] --> findings["Finding[]"]
Loading
High-Level Assessment

The following are alternative approaches to this PR:

1. Prefer ctx.packageManagers order over fixed table order
  • ➕ Matches the planner’s detection priority more directly
  • ➕ Avoids surprising precedence when multiple ecosystems exist in a repo
  • ➖ Requires clearer definition of how to break ties per checkId
  • ➖ May reduce determinism across planner versions unless tightly specified
2. Declare default outputs for built-in checks (optional conventions)
  • ➕ Enables findings extraction without requiring custom resolvers
  • ➕ Makes wave-11 end-to-end findings flow visible sooner
  • ➖ High risk of incorrect paths/formats across real-world toolchains
  • ➖ Encourages implicit behavior that may be hard to support long-term
3. Introduce a small registry for resolvers (no YAML) from day one
  • ➕ Makes extensibility explicit and composable (multiple resolvers)
  • ➕ Allows layering (org resolver overrides builtin)
  • ➖ Adds API surface/complexity beyond the immediate wave scope
  • ➖ May be premature until real custom resolver needs emerge

Recommendation: The PR’s approach (a deterministic built-in (checkId, packageManager) table + a narrow CheckResolver interface) is a good Wave-11 baseline: small surface area, easy to test, and aligns with the planner/findings separation. Consider a follow-up to clarify/adjust precedence semantics (table vs detected order) once multi-ecosystem repos become common, and keep built-in outputs empty until there is a stable cross-tool SARIF convention.

Files changed (20) +1312 / -356

Enhancement (6) +223 / -9
errors.tsIntroduce CheckError with two error codes and override cause +15/-0

Introduce CheckError with two error codes and override cause

• Adds CheckError and CheckErrorCode (RESOLUTION_FAILED, EXTRACTION_FAILED) with 'override readonly cause: unknown' for noImplicitOverride compliance.

packages/checks/src/errors.ts

extract.tsAdd SARIF-based findings extraction via @sverka/findings +61/-0

Add SARIF-based findings extraction via @sverka/findings

• Implements extractFindings to read declared SARIF outputs from an artifact directory, parse JSON, normalize via normalizeSarif, and wrap failures as CheckError(EXTRACTION_FAILED).

packages/checks/src/extract.ts

index.tsExport checks public API (resolver, extract, errors) +5/-0

Export checks public API (resolver, extract, errors)

• Defines the package entrypoint exports for resolver types/functions, extractFindings, and CheckError/CheckErrorCode.

packages/checks/src/index.ts

resolver.tsAdd CheckResolver interface and built-in (checkId, PM) table +105/-0

Add CheckResolver interface and built-in (checkId, PM) table

• Defines CheckResolver/ResolvedCheck/CheckOutput and implements createBuiltinResolver using an ordered lookup table mapping checkId and detected package managers to command/args, returning null for unknown mappings.

packages/checks/src/resolver.ts

index.tsRe-export checks types and helpers from @sverka/sdk +9/-0

Re-export checks types and helpers from @sverka/sdk

• Re-exports CheckResolver/ResolvedCheck/CheckOutput types and createBuiltinResolver/extractFindings/CheckError for SDK consumers.

packages/sdk/src/index.ts

sverka.tsResolve ProposedChecks into operations and extract SARIF findings +28/-9

Resolve ProposedChecks into operations and extract SARIF findings

• Updates doPlan to resolve proposed checks into OperationSpec[] in auto-discovery mode. Updates doExecute to resolve checks into executable operations when no config exists, and extracts findings from resolved check outputs post-execution.

packages/sdk/src/sverka.ts

Tests (8) +457 / -1
errors.test.tsAdd tests for CheckError fields and codes +27/-0

Add tests for CheckError fields and codes

• Verifies CheckError sets name/message/code and supports optional cause, and that all CheckErrorCode values are accepted.

packages/checks/src/tests/errors.test.ts

extract.test.tsAdd extractFindings SARIF tests and error wrapping coverage +67/-0

Add extractFindings SARIF tests and error wrapping coverage

• Covers SARIF extraction happy path, missing files, non-SARIF outputs, invalid SARIF raising CheckError(EXTRACTION_FAILED), and empty output lists.

packages/checks/src/tests/extract.test.ts

fixtures.tsAdd ProposedCheck/ProjectContext builders and SARIF fixture +88/-0

Add ProposedCheck/ProjectContext builders and SARIF fixture

• Introduces helper constructors for ProposedCheck and ProjectContext (with package manager lists) and a minimal valid SARIF 2.1.0 fixture for tests.

packages/checks/src/tests/helpers/fixtures.ts

public-api.test.tsAdd public API surface test for @sverka/checks +28/-0

Add public API surface test for @sverka/checks

• Asserts expected runtime exports exist and ensures no unexpected runtime values are exported from the checks entrypoint.

packages/checks/src/tests/public-api.test.ts

resolver.test.tsAdd built-in resolver mapping and determinism tests +185/-0

Add built-in resolver mapping and determinism tests

• Validates command/args mappings across Node/Python/Rust/Go, unknown/unmatched behavior, multi-PM precedence, empty outputs, determinism, and custom resolver usage with SARIF outputs.

packages/checks/src/tests/resolver.test.ts

fixtures.tsAdd temp repo fixture that triggers Node check proposals +24/-0

Add temp repo fixture that triggers Node check proposals

• Adds makeTempGitRepoWithPackageJson to create a minimal bun-managed repo so the planner detects bun and proposes Node checks for auto-discovery tests.

packages/sdk/src/tests/helpers/fixtures.ts

plan-mode.test.tsTest plan-mode auto-discovery now yields check operations +20/-1

Test plan-mode auto-discovery now yields check operations

• Extends plan mode tests to assert that detected package managers lead to ProposedChecks being resolved into non-empty OperationSpec[] with kind=check.

packages/sdk/src/tests/plan-mode.test.ts

re-exports.test.tsVerify SDK re-exports checks API symbols +18/-0

Verify SDK re-exports checks API symbols

• Adds assertions that createBuiltinResolver, extractFindings, and CheckError are exported from @sverka/sdk and have expected runtime shapes.

packages/sdk/src/tests/re-exports.test.ts

Documentation (2) +613 / -339
wave-11-checks-plan.mdAdd Wave 11 checks implementation plan and sequencing +419/-0

Add Wave 11 checks implementation plan and sequencing

• Adds an architecture/implementation plan detailing spec amendments, file layout, TDD steps, and SDK integration guidance for checks resolution and findings extraction.

engdocs/architecture/wave-11-checks-plan.md

spec.mdRewrite Spec 11 around resolver + SARIF extraction (trimmed scope) +194/-339

Rewrite Spec 11 around resolver + SARIF extraction (trimmed scope)

• Replaces provider/plugin-based design with a minimal CheckResolver + extractFindings contract, reduces error codes, documents the built-in resolution table and updated test plan.

specs/11-checks/spec.md

Other (4) +19 / -7
bun.lockAdd workspace deps for checks and link checks into SDK +6/-0

Add workspace deps for checks and link checks into SDK

• Registers @sverka/checks dependencies on core/planner/findings and adds @sverka/checks as a dependency of @sverka/sdk in the lockfile.

bun.lock

package.jsonFix ESM outputs and declare runtime deps for @sverka/checks +10/-5

Fix ESM outputs and declare runtime deps for @sverka/checks

• Updates main/module/types/exports to .mjs/.d.mts and adds workspace dependencies on @sverka/core, @sverka/planner, and @sverka/findings.

packages/checks/package.json

project.jsonUpdate checks lint command for ESLint 9 flat config +1/-1

Update checks lint command for ESLint 9 flat config

• Removes the deprecated --ext .ts flag from the nx lint target command.

packages/checks/project.json

package.jsonDepend on @sverka/checks from the SDK +2/-1

Depend on @sverka/checks from the SDK

• Adds @sverka/checks as a workspace dependency so SDK can resolve checks and extract findings.

packages/sdk/package.json

@codacy-production

codacy-production Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Up to standards ✅

🟢 Issues 0 issues

Results:
0 new issues

View in Codacy

🟢 Metrics 20 complexity · 0 duplication

Metric Results
Complexity 20
Duplication 0

View in Codacy

AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.

Run reviewer

TIP This summary will be updated as you push new changes.

Comment thread packages/checks/src/resolver.ts Outdated
Comment thread packages/sdk/src/sverka.ts Outdated
Comment thread packages/sdk/src/sverka.ts Outdated
@qodo-code-review

qodo-code-review Bot commented Aug 10, 2026 •

Copy link
Copy Markdown

Code Review by Qodo

🐞 Bugs (4) 📘 Rule violations (0) 📜 Skill insights (0)

Grey Divider


Action required

1. Docker checks cannot validate 🐞 Bug ≡ Correctness
Description
Auto-discovered operations omit image and imageDigest regardless of the selected executor, so
execute({ executor: "docker" }) deterministically fails IR validation before running any check.
The supported Docker option is therefore unusable with the new auto-discovery path.
Code

packages/checks/src/resolver.ts[R92-95]

+        const operation: OperationSpec = {
+          id: check.id,
+          kind: "check",
+          name: check.checkId,
Evidence
The resolver creates operations without image metadata, while SDK conversion assigns the globally
selected Docker executor. IR validation explicitly rejects Docker operations without a SHA-256 image
digest, and the SDK converts that validation result into EXECUTION_FAILED.

packages/checks/src/resolver.ts[85-100]
packages/sdk/src/sverka.ts[94-143]
packages/sdk/src/convert.ts[59-73]
packages/ir/src/validate.ts[188-200]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Make auto-discovered checks valid when the SDK Docker executor is selected.

## Issue Context
Docker IR operations require a pinned SHA-256 image digest, but built-in resolutions are host commands with no image metadata. Either reject/override Docker for host-only auto-checks or add immutable image mappings to resolution.

## Fix Focus Areas
- packages/checks/src/resolver.ts[85-100]
- packages/sdk/src/sverka.ts[94-134]
- packages/sdk/src/convert.ts[59-73]
- packages/ir/src/validate.ts[188-200]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


2. Custom resolver cannot integrate ⊘ Outdated 🐞 Bug ≡ Correctness
Description
Both SDK paths hard-code createBuiltinResolver(), and SverkaOptions exposes no resolver field,
so consumers cannot use the exported CheckResolver extension point in plan() or execute().
Because built-ins always declare empty outputs, real SDK executions cannot exercise output-bearing
custom checks or SARIF extraction.
Code

packages/sdk/src/sverka.ts[R82-85]

+  const resolver = createBuiltinResolver();
+  const operations = proposal.checks
+    .map((check) => resolver.resolve(check, context))
+    .filter((r): r is ResolvedCheck => r !== null)
Evidence
The SDK creates the built-in resolver directly in both planning and execution, while its public
options contain no injection point. The only output-bearing custom resolver is exercised in an
isolated checks-package unit test, and every built-in resolution returns outputs: [].

packages/sdk/src/sverka.ts[80-87]
packages/sdk/src/sverka.ts[114-120]
packages/sdk/src/types.ts[20-34]
packages/checks/src/resolver.ts[85-104]
packages/checks/src/tests/resolver.test.ts[162-179]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Allow callers to provide a `CheckResolver` to both SDK planning and execution, defaulting to the built-in resolver.

## Issue Context
The package publicly exports `CheckResolver`, but the SDK always instantiates its own built-in resolver. Add an optional resolver to SDK options and apply it consistently in both paths, including `createSverka` defaults and per-call overrides.

## Fix Focus Areas
- packages/sdk/src/types.ts[20-34]
- packages/sdk/src/sverka.ts[65-87]
- packages/sdk/src/sverka.ts[94-127]
- packages/sdk/src/__tests__/plan-mode.test.ts[59-71]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


3. Outputs never reach artifacts 🐞 Bug ≡ Correctness
Description
The SDK drops ResolvedCheck.outputs when building operations and never maps them to
OperationSpec.artifacts, so executors do not copy declared SARIF files into artifactDir.
extractFindings() then silently skips those missing files, producing false-negative empty
findings.
Code

packages/sdk/src/sverka.ts[R117-120]

+    resolvedChecks = proposal.checks
+      .map((check) => resolver.resolve(check, context))
+      .filter((r): r is ResolvedCheck => r !== null);
+    operations = resolvedChecks.map((r) => r.operation);
Evidence
Plan conversion derives runtime artifacts only from OperationSpec.artifacts, and both executors
copy only those declarations. The new custom-resolver example declares out.sarif only in
ResolvedCheck.outputs; extraction subsequently looks for that path under artifactDir and
silently skips it when absent.

packages/sdk/src/convert.ts[86-88]
packages/runtime-host/src/host-executor.ts[280-299]
packages/runtime-docker/src/docker-executor.ts[267-286]
packages/checks/src/extract.ts[29-33]
packages/checks/src/tests/resolver.test.ts[162-179]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Ensure every declared check output is collected into the artifact directory before findings extraction.

## Issue Context
Runtime executors collect only `operation.artifacts`, but `CheckOutput` is currently stored separately and discarded when operations are formed. Merge outputs into artifact declarations with matching destination names, and add an execution-level SARIF integration test.

## Fix Focus Areas
- packages/sdk/src/sverka.ts[114-120]
- packages/sdk/src/sverka.ts[175-184]
- packages/sdk/src/convert.ts[86-88]
- packages/checks/src/__tests__/resolver.test.ts[162-179]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


View action required (1)
4. Missing scripts always execute 🐞 Bug ≡ Correctness
Description
The built-in resolver treats package-manager detection as proof that generic Node scripts exist, so
auto-execution runs typecheck, lint, and test even when package.json declares none. Ordinary
detected projects therefore fail with missing-script errors instead of skipping unsupported checks.
Code

packages/checks/src/resolver.ts[R53-56]

+  { checkId: "typecheck", packageManagers: ["bun"], command: "bun", args: ["run", "typecheck"] },
+  { checkId: "typecheck", packageManagers: ["npm"], command: "npm", args: ["run", "typecheck"] },
+  { checkId: "typecheck", packageManagers: ["yarn"], command: "yarn", args: ["run", "typecheck"] },
+  { checkId: "typecheck", packageManagers: ["pnpm"], command: "pnpm", args: ["run", "typecheck"] },
Evidence
The planner proposes all three Node checks solely from language and package-manager matches, while
package-manager discovery can come from package.json#packageManager without inspecting scripts.
The new SDK fixture demonstrates this exact state: it declares Bun and TypeScript files but no
scripts, yet the resolver maps every proposal to bun run <checkId>.

packages/planner/src/planner.ts[268-303]
packages/planner/src/detect.ts[153-210]
packages/sdk/src/tests/helpers/fixtures.ts[117-135]
packages/sdk/src/sverka.ts[114-120]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
Prevent built-in checks from resolving commands that the detected project does not actually provide.

## Issue Context
Planner proposals are based only on language and package-manager presence; package-manager detection does not verify `package.json` scripts. Resolution must inspect applicable manifests/tool configuration or return `null` when availability is unconfirmed.

## Fix Focus Areas
- packages/checks/src/resolver.ts[51-77]
- packages/planner/src/planner.ts[268-303]
- packages/planner/src/detect.ts[153-210]
- packages/sdk/src/__tests__/helpers/fixtures.ts[117-135]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools



Informational

5. Synchronous fs I/O inside async extractFindings 🐞 Bug ➹ Performance
Description
extractFindings() is declared async but uses synchronous existsSync/readFileSync from node:fs
instead of the async fs/promises API, blocking the Node.js event loop while checking and reading
each SARIF file. It is also invoked sequentially in a for-loop over resolvedChecks in doExecute,
compounding the blocking effect when multiple checks declare outputs.
Code

packages/checks/src/extract.ts[R30-33]

+    if (output.format !== "sarif") continue;
+    const filePath = join(artifactDir, output.path);
+    if (!existsSync(filePath)) continue;
+    const raw = readFileSync(filePath, "utf8");
Evidence
extract.ts imports existsSync/readFileSync from node:fs (synchronous API) and calls them inside an
async function; sverka.ts then awaits extractFindings sequentially per resolved check in a for-loop,
so any blocking read serializes and stalls the event loop during execution. Impact is currently
limited since the built-in resolver always returns outputs: [], but it will affect custom resolvers
and future built-in checks that declare SARIF outputs.

packages/checks/src/extract.ts[1-33]
packages/sdk/src/sverka.ts[177-184]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

## Issue description
extractFindings() is marked `async` but performs synchronous, blocking filesystem I/O (`existsSync`, `readFileSync`) internally, which blocks the Node.js event loop during what is otherwise an async execution pipeline.

## Issue Context
The built-in resolver currently always returns `outputs: []`, so this code path is not exercised by default checks today, but it will be exercised by any custom `CheckResolver` that declares SARIF outputs, and by future built-in checks. The function is called in a sequential loop in the SDK's `doExecute`.

## Fix Focus Areas
- packages/checks/src/extract.ts[1-2]
- packages/checks/src/extract.ts[30-33]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

Context used
Review mode: 🧠 Deep: This introduces a new checks package plus SDK execution/planning integration across 20 files, with 27 independent edit sites and multiple resolver, artifact-extraction, and execution paths where subtle defects could be missed in one pass.

Grey Divider

Tip of the day
💡 Did you know, you can group findings by type and pick your Finding display, from Minimal to Full

More tips ↗ | Customize Qodo ↗ | Qodo docs ↗

Grey Divider

Qodo Logo

Comment thread packages/checks/src/resolver.ts Outdated
Comment thread packages/sdk/src/sverka.ts Outdated
Comment thread packages/sdk/src/sverka.ts Outdated
Comment thread packages/checks/src/resolver.ts Outdated
Comment thread packages/checks/src/extract.ts Outdated
@codeant-ai

codeant-ai Bot commented Aug 10, 2026

Copy link
Copy Markdown

Skipping CodeAnt AI review — this PR changes more than 100 files, which usually means a migration, codemod, or vendored drop. Line-level review on diffs this large produces duplicate findings on the same rewrite pattern and drowns out anything that actually matters.

If you still want a review, comment @codeant-ai : review. For better signal, consider splitting the PR into smaller chunks.

@codeant-ai codeant-ai Bot added size:XXL This PR changes 1000+ lines, ignoring generated files and removed size:XXL This PR changes 1000+ lines, ignoring generated files labels Aug 11, 2026
@ThePlenkov
ThePlenkov force-pushed the wave-11-checks branch 2 times, most recently from 535e2ea to 3b2803d Compare August 11, 2026 01:18
@ThePlenkov ThePlenkov closed this Aug 11, 2026
@sonarqubecloud

Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

baz: needs review size:XXL This PR changes 1000+ lines, ignoring generated files

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant