From d00df4f3271a44aa8aac790e7e6a6e6da516133e Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 02:19:34 -0400 Subject: [PATCH 1/4] fix(ci): execute coherence tests and propagate failures --- .github/workflows/coherence.yml | 4 +++- package.json | 2 +- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/.github/workflows/coherence.yml b/.github/workflows/coherence.yml index 5fa04e74a..13398d04e 100644 --- a/.github/workflows/coherence.yml +++ b/.github/workflows/coherence.yml @@ -12,12 +12,15 @@ on: paths: - 'src/**' - 'tests/**' + - 'package.json' - '.github/workflows/coherence.yml' pull_request: branches: [main] paths: - 'src/**' - 'tests/**' + - 'package.json' + - '.github/workflows/coherence.yml' concurrency: group: ${{ github.workflow }}-${{ github.ref }} @@ -90,7 +93,6 @@ jobs: - name: Run coherence tests run: npm run test:safe -- tests/integrations/coherence/ tests/learning/coherence-integration.test.ts --reporter=verbose - continue-on-error: true coherence-status: name: Coherence Status diff --git a/package.json b/package.json index 64ad371c0..a63ea75e2 100644 --- a/package.json +++ b/package.json @@ -83,7 +83,7 @@ "test:unit:mcp": "vitest run tests/unit/mcp --exclude='**/mcp/handlers/domain-handlers.test.ts' --fileParallelism=false", "test:ci": "vitest run --exclude='**/browser/**' --exclude='**/*.e2e.test.ts' --exclude='**/vibium/**' --exclude='**/integration/browser/**' --exclude='**/browser-swarm-coordinator.test.ts' --exclude='**/mcp/handlers/domain-handlers.test.ts' --exclude='**/fixtures/init-corpus/**'", "test:e2e": "vitest run tests/integration/browser --testTimeout=120000", - "test:safe": "NODE_OPTIONS='--max-old-space-size=768 --expose-gc' vitest run --maxForks=1", + "test:safe": "NODE_OPTIONS='--max-old-space-size=768 --expose-gc' vitest run --maxWorkers=1", "test:dev": "npm run test:unit:fast", "test:all": "npm test -- --run", "test:mcp": "npm run test:unit:mcp", From 594da0350837a155eb13d9dda395c33782b55ff0 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 01:46:07 -0400 Subject: [PATCH 2/4] fix(ci): skip report comments for fork pull requests (cherry picked from commit 0686b7cf5185f6f855c33f629bcdcbaf2c7fb832) --- .github/workflows/mcp-tools-test.yml | 3 +- .github/workflows/optimized-ci.yml | 3 +- .github/workflows/skill-validation.yml | 2 + tests/unit/scripts/fork-pr-comments.test.ts | 49 +++++++++++++++++++++ 4 files changed, 55 insertions(+), 2 deletions(-) create mode 100644 tests/unit/scripts/fork-pr-comments.test.ts diff --git a/.github/workflows/mcp-tools-test.yml b/.github/workflows/mcp-tools-test.yml index a0a409e7d..64c0abced 100644 --- a/.github/workflows/mcp-tools-test.yml +++ b/.github/workflows/mcp-tools-test.yml @@ -229,7 +229,8 @@ jobs: - name: Create summary comment uses: actions/github-script@v9 - if: github.event_name == 'pull_request' + # Fork tokens cannot write PR comments; keep reports and test gates running. + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository with: script: | const fs = require('fs'); diff --git a/.github/workflows/optimized-ci.yml b/.github/workflows/optimized-ci.yml index ab346fb95..ffd0d52bd 100644 --- a/.github/workflows/optimized-ci.yml +++ b/.github/workflows/optimized-ci.yml @@ -412,7 +412,8 @@ jobs: path: ci-metrics.md retention-days: 30 - name: Comment on PR - if: github.event_name == 'pull_request' + # Fork tokens cannot write PR comments; keep reports and test gates running. + if: github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository uses: actions/github-script@v9 with: script: | diff --git a/.github/workflows/skill-validation.yml b/.github/workflows/skill-validation.yml index 8b3c9de2e..d3cad5fb2 100644 --- a/.github/workflows/skill-validation.yml +++ b/.github/workflows/skill-validation.yml @@ -451,6 +451,8 @@ jobs: cat report.md - name: Comment on PR + # Fork tokens cannot write PR comments; keep the Tier 3 gate running. + if: github.event.pull_request.head.repo.full_name == github.repository uses: actions/github-script@v9 with: script: | diff --git a/tests/unit/scripts/fork-pr-comments.test.ts b/tests/unit/scripts/fork-pr-comments.test.ts new file mode 100644 index 000000000..f6b620c7e --- /dev/null +++ b/tests/unit/scripts/fork-pr-comments.test.ts @@ -0,0 +1,49 @@ +import { readFileSync } from 'node:fs'; +import { resolve } from 'node:path'; +import { runInNewContext } from 'node:vm'; +import { parse } from 'yaml'; +import { describe, expect, it } from 'vitest'; + +type Step = { name?: string; if?: string }; +type Job = { if?: string; steps: Step[] }; +const root = resolve(import.meta.dirname, '../../..'); +const workflows = [ + ['optimized-ci.yml', 'dashboard', 'Comment on PR'], + ['mcp-tools-test.yml', 'mcp-summary', 'Create summary comment'], + ['skill-validation.yml', 'report', 'Comment on PR'], +] as const; + +// These workflow guards use comparisons and boolean operators shared by +// JavaScript and Actions expressions. Exercise the actual YAML conditions. +function allowed(condition: string | undefined, event: string, headRepo?: string): boolean { + if (!condition) return true; + return Boolean(runInNewContext(condition, { + always: () => true, + github: { + event_name: event, + repository: 'upstream/agentic-qe', + event: headRepo ? { pull_request: { head: { repo: { full_name: headRepo } } } } : {}, + }, + })); +} + +describe.each(workflows)('%s PR reporting permissions', (file, jobName, stepName) => { + const workflow = parse(readFileSync(resolve(root, '.github/workflows', file), 'utf8')); + const job: Job = workflow.jobs[jobName]; + const comment = job.steps.find((step) => step.name === stepName)!; + + it('keeps the reporting job available but skips writes for fork PRs', () => { + expect(comment).toBeDefined(); + expect(allowed(job.if, 'pull_request', 'contributor/agentic-qe')).toBe(true); + expect(allowed(comment.if, 'pull_request', 'contributor/agentic-qe')).toBe(false); + }); + + it('retains comments for same-repository PRs', () => { + expect(allowed(job.if, 'pull_request', 'upstream/agentic-qe')).toBe(true); + expect(allowed(comment.if, 'pull_request', 'upstream/agentic-qe')).toBe(true); + }); + + it.each(['push', 'workflow_dispatch'])('does not post a PR comment on %s', (event) => { + expect(allowed(job.if, event) && allowed(comment.if, event)).toBe(false); + }); +}); From 480c054e3a1f1c74f36a0c189e3401970dbf04fd Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 17:43:48 -0400 Subject: [PATCH 3/4] fix(ci): preserve runner exits and generate valid coverage reports --- .github/workflows/mcp-tools-test.yml | 18 ++--- scripts/ci-vitest-run.sh | 50 ++----------- tests/unit/scripts/ci-vitest-run.test.ts | 89 ++++++++++++++++++++++++ vitest.config.ts | 2 +- 4 files changed, 100 insertions(+), 59 deletions(-) create mode 100644 tests/unit/scripts/ci-vitest-run.test.ts diff --git a/.github/workflows/mcp-tools-test.yml b/.github/workflows/mcp-tools-test.yml index 64c0abced..138cd4c5f 100644 --- a/.github/workflows/mcp-tools-test.yml +++ b/.github/workflows/mcp-tools-test.yml @@ -147,21 +147,11 @@ jobs: - run: npm run build - name: Run MCP integration tests - run: | - timeout 480 npm run test:mcp:integration; EXIT=$? - if [ $EXIT -eq 124 ] && [ -f junit.xml ]; then - FAILURES=$(grep -c '/dev/null || echo "0") - if [ "$FAILURES" = "0" ]; then - echo "::warning::Vitest hung after tests passed (exit 124). Treating as success." - exit 0 - fi - fi - exit $EXIT + run: bash scripts/ci-vitest-run.sh tests/integration/mcp/ env: - NODE_OPTIONS: '--max-old-space-size=1024' - # C3: was `continue-on-error: true`, which let real failures pass as - # green. Removed so this job is an actual gate. The exit-124 hang - # tolerance above still absorbs the known vitest-hang flake. + NODE_OPTIONS: '--max-old-space-size=1024 --expose-gc' + CI_VITEST_TIMEOUT: '480' + # Preserve runner errors and timeouts even when a partial report exists. - name: Generate test report uses: dorny/test-reporter@v1 diff --git a/scripts/ci-vitest-run.sh b/scripts/ci-vitest-run.sh index 32260b6ec..7a1ddd706 100755 --- a/scripts/ci-vitest-run.sh +++ b/scripts/ci-vitest-run.sh @@ -1,50 +1,12 @@ #!/usr/bin/env bash -# CI wrapper for vitest that handles process hangs gracefully. -# -# Problem: vitest completes all tests but hangs due to open handles -# (SQLite connections, HNSW models, timers). The `timeout` command -# kills it with exit code 124, which CI treats as failure even though -# all tests passed. -# -# Solution: Capture vitest output via tee. When timeout kills vitest, -# check the captured output for the "Test Files X passed" summary -# line that vitest prints after all tests complete. junit.xml cannot -# be used because vitest writes it only on clean exit, and the killed -# process leaves it as 0 bytes. +# Bound CI test execution without changing Vitest's exit status. +# A passing test summary does not prove coverage/report generation or cleanup +# completed. Runner errors and timeouts must remain failures. # # Usage: scripts/ci-vitest-run.sh [vitest args...] TIMEOUT_SECONDS="${CI_VITEST_TIMEOUT:-480}" -OUTFILE=$(mktemp /tmp/vitest-output.XXXXXX) - -# --foreground: send signal only to the child process, not the process -# group. Without this, timeout kills this wrapper script too. -# Pipe through tee to capture output while still displaying it. -timeout --foreground "$TIMEOUT_SECONDS" npx vitest run "$@" 2>&1 | tee "$OUTFILE" -# PIPESTATUS[0] is timeout's exit code, not tee's -EXIT=${PIPESTATUS[0]} - -if [ "$EXIT" -eq 0 ]; then - rm -f "$OUTFILE" - exit 0 -fi - -# Check captured output for vitest's test summary. -# Vitest prints "Test Files X passed" after all tests complete, -# before the process hangs. If this line exists with no failures, -# tests passed and the exit code is from the timeout kill. -if grep -q "Test Files.*passed" "$OUTFILE" 2>/dev/null; then - if grep -q "Test Files.*failed" "$OUTFILE" 2>/dev/null; then - echo "::error::Some test files failed." - rm -f "$OUTFILE" - exit "$EXIT" - fi - echo "" - echo "::warning::Vitest process hung after all tests passed (exit $EXIT). Treating as success." - rm -f "$OUTFILE" - exit 0 -fi -echo "::error::Vitest was killed before tests completed (exit $EXIT)." -rm -f "$OUTFILE" -exit "$EXIT" +# --foreground sends the timeout signal to the child rather than this wrapper's +# process group. exec preserves the runner's status, including timeout exit 124. +exec timeout --foreground "$TIMEOUT_SECONDS" npx vitest run "$@" diff --git a/tests/unit/scripts/ci-vitest-run.test.ts b/tests/unit/scripts/ci-vitest-run.test.ts new file mode 100644 index 000000000..fa25cad4a --- /dev/null +++ b/tests/unit/scripts/ci-vitest-run.test.ts @@ -0,0 +1,89 @@ +import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join, resolve } from 'node:path'; +import { spawnSync } from 'node:child_process'; +import { afterEach, describe, expect, it } from 'vitest'; + +const wrapper = resolve(import.meta.dirname, '../../../scripts/ci-vitest-run.sh'); +// The wrapper is used by Ubuntu CI. Stock macOS and Windows do not ship all +// of its shell tools; qualify those local runs without letting Linux CI skip. +const timeoutVersion = spawnSync('timeout', ['--version'], { encoding: 'utf8' }); +const shellToolsAvailable = existsSync('/bin/bash') && timeoutVersion.status === 0 + && timeoutVersion.stdout.includes('GNU coreutils'); +if (process.platform === 'linux' && !shellToolsAvailable) { + throw new Error('CI Vitest wrapper tests require /bin/bash and GNU timeout on Linux; this CI prerequisite must not be skipped.'); +} +const skipReason = !shellToolsAvailable ? ' (requires Bash and GNU timeout on this platform)' : ''; +const fixtures: string[] = []; + +afterEach(() => { + for (const fixture of fixtures.splice(0)) { + rmSync(fixture, { recursive: true, force: true }); + } +}); + +function runRunner(exitCode: number, options: { hang?: boolean; summary?: boolean } = {}) { + const fixture = mkdtempSync(join(tmpdir(), 'aqe-ci-verdict-')); + fixtures.push(fixture); + const argsPath = join(fixture, 'args.txt'); + writeFileSync(join(fixture, 'npx'), `#!/bin/sh +printf '%s\n' "$@" > "$AQE_TEST_ARGS" +if [ "$AQE_TEST_SUMMARY" = 'true' ]; then + printf ' Test Files 1 passed (1)\n Tests 18 passed (18)\n' +fi +if [ "$AQE_TEST_HANG" = 'true' ]; then + exec sleep 15 +fi +if [ "$AQE_TEST_EXIT" != '0' ]; then + printf 'Unhandled Error: coverage report generation failed\n' >&2 +fi +exit "$AQE_TEST_EXIT" +`, { mode: 0o755 }); + + const result = spawnSync('/bin/bash', [wrapper, 'tests/fixture with spaces.test.ts', '--coverage'], { + cwd: fixture, + encoding: 'utf8', + timeout: 8000, + env: { + ...process.env, + PATH: `${fixture}:${process.env.PATH}`, + TMPDIR: fixture, + AQE_PROJECT_ROOT: fixture, + AQE_TEST_ARGS: argsPath, + AQE_TEST_EXIT: String(exitCode), + AQE_TEST_SUMMARY: String(options.summary ?? true), + AQE_TEST_HANG: String(options.hang ?? false), + CI_VITEST_TIMEOUT: '5', + }, + }); + return { ...result, args: readFileSync(argsPath, 'utf8').trim().split('\n') }; +} + +describe.skipIf(process.platform !== 'linux' && !shellToolsAvailable)(`CI Vitest runner exit status${skipReason}`, () => { + it('passes a successful runner and forwards arguments without splitting', () => { + const result = runRunner(0); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(0); + expect(result.stdout).toContain('18 passed'); + expect(result.args).toEqual(['vitest', 'run', 'tests/fixture with spaces.test.ts', '--coverage']); + }); + + it.each([1, 2, 124, 137, 143, 255])('preserves exit %i after a passing test summary', (code) => { + const result = runRunner(code); + expect(result.error).toBeUndefined(); + expect(result.status).toBe(code); + expect(result.stdout + result.stderr).toContain('coverage report generation failed'); + expect(result.stdout + result.stderr).not.toContain('Treating as success'); + }); + + it('preserves failures before the test summary', () => { + expect(runRunner(1, { summary: false }).status).toBe(1); + }); + + it('fails an actual timeout even after all tests report passing', () => { + const result = runRunner(0, { hang: true }); + expect(result.error).toBeUndefined(); + expect(result.stdout).toContain('18 passed'); + expect(result.status).toBe(124); + }); +}); diff --git a/vitest.config.ts b/vitest.config.ts index 3fd5fea8e..93e359754 100644 --- a/vitest.config.ts +++ b/vitest.config.ts @@ -38,7 +38,7 @@ export default defineConfig({ ], coverage: { provider: 'v8', - reporter: ['text', 'json', 'html', 'junit'], + reporter: ['text', 'json', 'json-summary', 'html'], include: ['src/**/*.ts'], exclude: ['src/**/*.d.ts', 'src/**/index.ts'], }, From 8152bc578fecaa58142cf4793ae8175e117ee493 Mon Sep 17 00:00:00 2001 From: Rudy Celekli Date: Mon, 21 Sep 2026 17:45:26 -0400 Subject: [PATCH 4/4] test(ci): bound local shell capability detection --- tests/unit/scripts/ci-vitest-run.test.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/unit/scripts/ci-vitest-run.test.ts b/tests/unit/scripts/ci-vitest-run.test.ts index fa25cad4a..fe5161af8 100644 --- a/tests/unit/scripts/ci-vitest-run.test.ts +++ b/tests/unit/scripts/ci-vitest-run.test.ts @@ -7,7 +7,7 @@ import { afterEach, describe, expect, it } from 'vitest'; const wrapper = resolve(import.meta.dirname, '../../../scripts/ci-vitest-run.sh'); // The wrapper is used by Ubuntu CI. Stock macOS and Windows do not ship all // of its shell tools; qualify those local runs without letting Linux CI skip. -const timeoutVersion = spawnSync('timeout', ['--version'], { encoding: 'utf8' }); +const timeoutVersion = spawnSync('timeout', ['--version'], { encoding: 'utf8', timeout: 2000 }); const shellToolsAvailable = existsSync('/bin/bash') && timeoutVersion.status === 0 && timeoutVersion.stdout.includes('GNU coreutils'); if (process.platform === 'linux' && !shellToolsAvailable) {