fix(security): stop reporting incomplete scans as clean

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

docs/security/scan-execution-evidence.md

@@ -0,0 +1,103 @@
+# Security scan execution evidence
+
+Security findings and execution completeness are separate results. A completed
+scan may contain critical findings. An empty finding list may mean that no
+analysis ran.
+
+This implements the execution-truth portion of [#694](https://github.com/proffesor-for-testing/agentic-qe/issues/694).
+It does not provide full SAST assurance, add a secret-scanning engine, or close
+all of that issue's scope and reuse requirements.
+
+## Receipt and counting semantics
+
+`SASTResult.evidence` is additive and uses `schemaVersion: 1`. The production
+SAST scanner supplies it. Consumers treat a legacy result without evidence as
+unverified; they must not infer successful analysis from its finding count or
+old coverage numbers.
+
+- `requestedPaths` is captured before engine execution. Paths are normalized
+  absolute paths and deduplicated lexically; symlink aliases are not resolved.
+  `duplicateInputs` counts repeated normalized inputs.
+- Each built-in file analysis records its disposition, SHA-256 of the exact
+  bytes read when readable, `readLines`, and `analyzedLines`. Unsupported
+  readable inputs have a digest and read count but zero analyzed lines.
+- `coverage.filesScanned` and `linesScanned` count completed built-in JS/TS
+  pattern analyses. `rulesApplied` counts unique selected rules actually
+  evaluated, with `rulesAppliedScope: 'built-in-patterns'`. It is independent
+  of the number of findings. Unknown requested rule-set IDs are rejected.
+- Engines carry their requested/required flags, execution status, declared
+  scope, known rule IDs/digest, errors, and limitations. Unknown external
+  file membership and rule coverage remain unknown.
+- `completeness: complete` means the **declared required scope** completed;
+  `partial` retains completed work alongside gaps; `none` establishes no
+  completed required file analysis. These values never mean a system is safe.
+
+Line counts retain the existing `content.split('\n').length` convention.
+The per-engine digest identifies bytes read, not an atomic repository snapshot:
+files can change between discovery and reads, or between independent engines.
+
+## Discovery and engine boundaries
+
+CLI and comprehensive MCP scanning share the `aqe-security-source-files@1`
+discovery policy. Directory scans select established source extensions, exclude
+known dependency/build/private-state directories, and do not follow observed
+symlinks. Direct file targets are retained so unsupported inputs are visible.
+Missing/unreadable roots, unreadable subtrees, and file/depth limits have
+separate discovery dispositions. A completely inventoried empty directory does
+not imply any analysis executed. Limits default to 5,000 files and 64 levels.
+
+The SAST domain runs its existing JavaScript/TypeScript patterns. Comprehensive
+MCP scanning also runs its existing generic source-text patterns and optional
+manifest-name advisories. Its `filesScanned` counts unique requested source
+paths with a completed analysis; the nested `coverage` retains the narrower
+built-in SAST counts. Generic patterns on Python or other source languages do
+not establish language-specific SAST coverage. `deepAnalysisPerformed` now
+requires a returned built-in SAST execution receipt; it is not a claim of
+semantic completeness.
+
+Semgrep remains optional (`required: false`). Its absence, process failure,
+malformed output, partial errors, or clean completion are distinct. Valid
+findings survive partial output. Semgrep still targets the common parent
+directory and can return findings outside the requested file list; its receipt
+makes that scope explicit and does not invent per-file/rule coverage. An
+optional Semgrep failure does not invalidate completed required built-in
+checks. Callers requiring Semgrep must inspect that engine's receipt.
+
+## Consumer changes
+
+- `aqe security` now actually runs SAST by default and passes `FilePath` values
+  to the domain API. Incomplete, unavailable, failed, or unverified requested
+  checks exit nonzero. Complete scans retain the existing severity codes:
+  critical/high = 1, medium = 2, otherwise 0.
+- CLI text, JSON, Markdown, and SARIF retain execution status. SARIF
+  `executionSuccessful` describes execution, independently of findings. The
+  CLI's existing `--dast` placeholder is explicitly not-run. Compliance checks
+  preserve skipped/unverified/failed execution separately from violations.
+- `security_scan_comprehensive` retains findings, execution receipts,
+  discovery, scope limitations, and actual counts through the registered MCP
+  path. Its result can be `partial`, `unavailable`, or `unverified` in addition
+  to `completed`. Transport/task success does not mean scan completeness.
+  `targetUrl` describes the DAST target; current receiptless DAST output remains
+  unverified. Requested compliance that this task does not implement is
+  explicitly not-run. Saved security reports also retain execution status.
+- The exported TypeScript `SecurityAuditProtocol` no longer estimates a clean
+  secret scan. It reports that implementation unavailable and blocks its
+  deployment recommendation when requested checks are failed, incomplete, or
+  unverified, including the protocol's placeholder compliance reports. Trigger
+  scope is honored: pre-release requires secrets; dependency-update requests
+  dependency scanning. This protocol is tested directly; the registered comprehensive
+  MCP route uses a different task handler.
+
+Security receipt errors use bounded codes/static explanations rather than raw
+provider error text. Findings retain the existing source snippets; receipts
+are not a redaction mechanism for findings or user-supplied paths/URLs.
+
+## Remaining work in #694
+
+This change does not add an atomic source snapshot, alias identity resolution,
+per-file Semgrep execution proof, a mandatory-external-engine policy, or new
+DAST/secret-scanner implementations. It does not add cross-run receipt reuse
+or integration with a general release gate. The comprehensive MCP tool already
+bypasses the session result cache; integration tests verify that changing a
+file and repeating the same call returns a fresh digest and findings. That is
+a freshness control, not a new cache invalidation mechanism.

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