Skip to content

FE-1093: Harden orchestrator agent harness testing diagnostics#269

Open
kostandinang wants to merge 1 commit into
mainfrom
ka/fe-1093-harness-diagnostics
Open

FE-1093: Harden orchestrator agent harness testing diagnostics#269
kostandinang wants to merge 1 commit into
mainfrom
ka/fe-1093-harness-diagnostics

Conversation

@kostandinang

@kostandinang kostandinang commented Jun 26, 2026

Copy link
Copy Markdown
Contributor

What

Adds profile-owned runner diagnostics for cook test execution.

Why

Runner/toolchain load failures, like Vitest being denied under confinement, should route as infra instead of product test failures.

Verification

  • npx vitest run src/orchestrator/src/project-profile.test.ts src/orchestrator/src/test-runner.test.ts
  • npm run verify reached the full test suite; unrelated graph type-contract tests timed out.

@kostandinang kostandinang changed the title FE-1093: add profile-owned runner diagnostics FE-1093: Harden orchestrator agent harness testing diagnostics Jun 26, 2026

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@kostandinang kostandinang marked this pull request as ready for review June 26, 2026 21:51
@cursor

cursor Bot commented Jun 26, 2026

Copy link
Copy Markdown

PR Summary

Medium Risk
Changes how verify failures are routed (infra vs test vs absent); misclassification could skip real reds or send logic-fix agents at toolchain issues, but behavior is conservative and covered by new unit tests.

Overview
Adds profile-owned RunnerDiagnostics on each cook toolchain (runnerPackages, noTestsPatterns, runnerName) so failure classification is no longer hardcoded to Vitest wording.

classifyTestFailure now takes the active profile’s diagnostics: “no tests matched” uses per-runner regexes (e.g. Deno vs Vitest), and EPERM/EACCES on paths under /node_modules/{runnerPackage}/ is treated as infra (sandbox can’t load the runner). Permission errors on other packages (e.g. Playwright) still count as test failures.

isInfraSpawnError also treats spawn EACCES/EPERM as infra. ToolchainTestRunner passes toolchain.diagnostics when stamping failureKind.

Reviewed by Cursor Bugbot for commit be3c011. Bugbot is set up for automated code reviews on this repo. Configure here.

}

function isRunnerPackageDenied(output: string, diagnostics: RunnerDiagnostics): boolean {
if (!/\b(?:EACCES|EPERM)\b|operation not permitted|permission denied/i.test(output)) return false;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Semgrep identified an issue in your code:
Ensure that the regex used to compare with user supplied input is safe from regular expression denial of service.

To resolve this comment:

🔧 No guidance has been designated for this issue. Fix according to your organization's approved methods.

💬 Ignore this finding

Reply with Semgrep commands to ignore this finding.

  • /fp <comment> for false positive
  • /ar <comment> for acceptable risk
  • /other <comment> for all other reasons

Alternatively, triage in Semgrep AppSec Platform to ignore the finding created by regex_dos.

You can view more details about this finding in the Semgrep AppSec Platform.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant