fix(ci): execute coherence tests and preserve runner failures

.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

.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 '<failure' junit.xml 2>/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
@@ -229,7 +219,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');

.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: |

.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: |

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",

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 "$@"

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', timeout: 2000 });
+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);
+  });
+});

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);
+  });
+});

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'],
     },