fix(learning): stop treating guidance retrieval as successful reuse

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

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

src/integrations/agentic-flow/reasoning-bank/index.ts

@@ -304,20 +304,6 @@ export class EnhancedReasoningBankAdapter {
             ...guidance.suggestedActions.map(a => `Action: ${a}`),
             ...result.guidance,
           ];
-          this.stats.tokensSavedEstimate += guidance.estimatedTokenSavings;
-          this.stats.experiencesApplied++;
-
-          // Record application of each source experience for reuse tracking
-          for (const src of guidance.sourceExperiences) {
-            this.experienceReplay.recordApplication(
-              src.id,
-              request.task,
-              true, // success=true at routing time; updated later via recordOutcome
-              Math.round(guidance.estimatedTokenSavings),
-            ).catch(err => {
-              console.warn(`[EnhancedAdapter] Failed to record experience application: ${err}`);
-            });
-          }
         }
       }
 
@@ -458,23 +444,8 @@ export class EnhancedReasoningBankAdapter {
       return null;
     }
 
-    const guidance = await this.experienceReplay.getGuidance(task, domain);
-    if (guidance) {
-      this.stats.experiencesApplied++;
-
-      // Record application for reuse tracking (experience_applications table)
-      for (const src of guidance.sourceExperiences) {
-        this.experienceReplay.recordApplication(
-          src.id,
-          task,
-          true,
-          Math.round(guidance.estimatedTokenSavings),
-        ).catch(err => {
-          console.warn(`[EnhancedAdapter] Failed to record experience application: ${err}`);
-        });
-      }
-    }
-    return guidance;
+    // Retrieving guidance does not establish that it was applied successfully.
+    return this.experienceReplay.getGuidance(task, domain);
   }
 
   /**
@@ -496,6 +467,7 @@ export class EnhancedReasoningBankAdapter {
     if (!this.experienceReplay) return;
 
     await this.experienceReplay.recordApplication(experienceId, task, success, tokensSaved);
+    this.stats.experiencesApplied++;
     this.stats.tokensSavedEstimate += tokensSaved;
   }
 

tests/integration/experience-guidance-outcomes.test.ts

@@ -0,0 +1,179 @@
+/** Guidance retrieval must not manufacture successful execution evidence. */
+import { afterAll, afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest';
+import { createServer, type Server } from 'node:http';
+import { mkdtempSync, rmSync } from 'node:fs';
+import { tmpdir } from 'node:os';
+import { join } from 'node:path';
+import type { Database } from 'better-sqlite3';
+import type { EnhancedReasoningBankAdapter } from '../../src/integrations/agentic-flow/reasoning-bank/index.js';
+
+describe('Experience guidance outcome integrity', () => {
+  let server: Server;
+  let projectRoot: string;
+  let endpoint: string;
+  let adapter: EnhancedReasoningBankAdapter;
+  let db: Database;
+  let adapterModule: typeof import('../../src/integrations/agentic-flow/reasoning-bank/index.js');
+  let memoryModule: typeof import('../../src/kernel/unified-memory.js');
+  let embeddingsModule: typeof import('../../src/learning/real-embeddings.js');
+  let hnswModule: typeof import('../../src/kernel/hnsw-adapter.js');
+  let consolidationModule: typeof import('../../src/learning/experience-consolidation.js');
+  const task = 'Generate boundary checks for a numeric range';
+  const domain = 'test-generation' as const;
+
+  beforeAll(async () => {
+    projectRoot = mkdtempSync(join(tmpdir(), 'aqe-guidance-outcomes-'));
+    vi.stubEnv('AQE_PROJECT_ROOT', projectRoot);
+    vi.stubEnv('AQE_MEMORY_BACKEND', 'memory');
+    vi.stubEnv('AQE_EMBEDDER_TOKEN', '');
+
+    // The only external service is a deterministic loopback embedder. A single
+    // experience per domain makes retrieval independent of semantic quality.
+    server = createServer(async (request, response) => {
+      const chunks: Buffer[] = [];
+      for await (const chunk of request) chunks.push(Buffer.from(chunk));
+      const body = JSON.parse(Buffer.concat(chunks).toString()) as { input: string | string[] };
+      const inputs = Array.isArray(body.input) ? body.input : [body.input];
+      response.writeHead(200, { 'content-type': 'application/json' });
+      response.end(JSON.stringify({
+        data: inputs.map((_, index) => ({ index, embedding: [1, ...Array(383).fill(0)] })),
+      }));
+    });
+    await new Promise<void>(resolve => server.listen(0, '127.0.0.1', resolve));
+    endpoint = `http://127.0.0.1:${(server.address() as { port: number }).port}`;
+    vi.stubEnv('AQE_EMBEDDER_ENDPOINT', endpoint);
+
+    // Set isolation before production imports establish singleton defaults.
+    adapterModule = await import('../../src/integrations/agentic-flow/reasoning-bank/index.js');
+    memoryModule = await import('../../src/kernel/unified-memory.js');
+    embeddingsModule = await import('../../src/learning/real-embeddings.js');
+    hnswModule = await import('../../src/kernel/hnsw-adapter.js');
+    consolidationModule = await import('../../src/learning/experience-consolidation.js');
+  });
+
+  beforeEach(async () => {
+    adapter = new adapterModule.EnhancedReasoningBankAdapter({
+      enablePatternEvolution: false,
+      autoConsolidate: false,
+      base: { sqlite: { useUnified: true }, embeddings: { endpoint } },
+      experienceReplay: { autoPrune: false, embedding: { endpoint } },
+      trajectoryTracker: { autoEndTimeoutMs: 1000 },
+    });
+    await adapter.initialize();
+    const memory = memoryModule.getUnifiedMemory();
+    expect(memory.getDbPath()).toBe(':memory:');
+    db = memory.getDatabase()!;
+  });
+
+  afterEach(async () => {
+    await adapter?.dispose();
+    embeddingsModule?.resetInitialization();
+    memoryModule?.resetUnifiedMemory();
+    hnswModule?.HnswAdapter.closeAll();
+  });
+
+  afterAll(async () => {
+    server?.closeAllConnections();
+    if (server) await new Promise<void>(resolve => server.close(() => resolve()));
+    vi.unstubAllEnvs();
+    if (projectRoot) rmSync(projectRoot, { recursive: true, force: true });
+  });
+
+  async function seedExperience(): Promise<string> {
+    const trajectory = await adapter.startTaskTrajectory(task, { domain });
+    await adapter.recordTaskStep(trajectory, 'Check both range boundaries', { outcome: 'success' }, { quality: 0.6 });
+    await adapter.endTaskTrajectory(trajectory, true);
+    return (db.prepare('SELECT id FROM captured_experiences WHERE domain = ?').get(domain) as { id: string }).id;
+  }
+
+  function applications() {
+    return db.prepare('SELECT experience_id, success, tokens_saved FROM experience_applications ORDER BY rowid').all();
+  }
+
+  it('returns guidance repeatedly without recording applications or successful reuse', async () => {
+    const id = await seedExperience();
+    for (let i = 0; i < 4; i++) {
+      const guidance = await adapter.getExperienceGuidance(task, domain);
+      expect(guidance?.sourceExperiences).toEqual([expect.objectContaining({ id })]);
+    }
+    expect(applications()).toEqual([]);
+    expect(db.prepare('SELECT application_count FROM captured_experiences WHERE id = ?').get(id)).toEqual({ application_count: 0 });
+    expect((await adapter.getStats()).adapter.experiencesApplied).toBe(0);
+  });
+
+  it('includes routing guidance without recording a completed experience', async () => {
+    const id = await seedExperience();
+    const routed = await adapter.routeTaskWithExperience({ task, domain });
+    expect(routed.success).toBe(true);
+    if (!routed.success) throw routed.error;
+    expect(routed.value.experienceGuidance?.sourceExperiences).toEqual([expect.objectContaining({ id })]);
+    expect(routed.value.guidance.some(line => line.startsWith('Strategy:'))).toBe(true);
+    expect(applications()).toEqual([]);
+    expect((await adapter.getStats()).adapter).toMatchObject({ tasksRouted: 1, experiencesApplied: 0, tokensSavedEstimate: 0 });
+  });
+
+  it('does not turn failed execution after retrieval into positive reinforcement', async () => {
+    const id = await seedExperience();
+    await adapter.getExperienceGuidance(task, domain);
+    const trajectory = await adapter.startTaskTrajectory(task, { domain });
+    await adapter.recordTaskStep(trajectory, 'Run checks', { outcome: 'failure', error: 'Assertion failed' }, { quality: 0 });
+    expect((await adapter.endTaskTrajectory(trajectory, false)).outcome).toBe('failure');
+    await adapter.recordExperienceApplication(id, task, false);
+    expect(applications()).toEqual([{ experience_id: id, success: 0, tokens_saved: 0 }]);
+    const consolidator = new consolidationModule.ExperienceConsolidator();
+    await consolidator.initialize(db);
+    await consolidator.consolidateDomain(domain);
+    const quality = (db.prepare('SELECT quality FROM captured_experiences WHERE id = ?').get(id) as { quality: number }).quality;
+    expect(quality).toBeCloseTo(0.26);
+    expect((await adapter.getExperienceGuidance(task, domain))?.confidence).toBeCloseTo(0.26);
+  });
+
+  it('persists explicit success and failure with matching adapter and replay counts', async () => {
+    const id = await seedExperience();
+    await adapter.recordExperienceApplication(id, 'Successful execution', true, 30);
+    await adapter.recordExperienceApplication(id, 'Failed execution', false, 5);
+    expect(applications()).toEqual([
+      { experience_id: id, success: 1, tokens_saved: 30 },
+      { experience_id: id, success: 0, tokens_saved: 5 },
+    ]);
+    const stats = await adapter.getStats();
+    expect(stats.adapter).toMatchObject({ experiencesApplied: 2, tokensSavedEstimate: 35 });
+    expect(stats.experienceReplay).toMatchObject({ experiencesApplied: 2, totalTokensSaved: 35 });
+  });
+
+  it('keeps predicted savings from subsequent guidance separate from recorded savings', async () => {
+    const id = await seedExperience();
+    await adapter.recordExperienceApplication(id, task, true, 42);
+    expect((await adapter.getExperienceGuidance(task, domain))?.estimatedTokenSavings).toBe(42);
+    const routed = await adapter.routeTaskWithExperience({ task, domain });
+    expect(routed.success).toBe(true);
+    expect(applications()).toHaveLength(1);
+    expect((await adapter.getStats()).adapter).toMatchObject({ experiencesApplied: 1, tokensSavedEstimate: 42 });
+  });
+
+  it.each([{ success: true, quality: 0.66 }, { success: false, quality: 0.26 }])(
+    'reinforces explicitly recorded success=$success through consolidation',
+    async ({ success, quality }) => {
+      const id = await seedExperience();
+      await adapter.recordExperienceApplication(id, task, success);
+      const consolidator = new consolidationModule.ExperienceConsolidator();
+      await consolidator.initialize(db);
+      await consolidator.consolidateDomain(domain);
+      expect((await adapter.getExperienceGuidance(task, domain))?.confidence).toBeCloseTo(quality);
+      expect(applications()).toHaveLength(1);
+    },
+  );
+
+  it('leaves application evidence and counters unchanged when no guidance exists', async () => {
+    expect(await adapter.getExperienceGuidance(task, domain)).toBeNull();
+    expect(applications()).toEqual([]);
+    expect((await adapter.getStats()).adapter).toMatchObject({ experiencesApplied: 0, tokensSavedEstimate: 0 });
+  });
+
+  it('does not count an outcome whose SQLite persistence fails', async () => {
+    // The schema's real foreign key rejects an unknown source experience.
+    await expect(adapter.recordExperienceApplication('missing-experience', task, true, 99)).rejects.toThrow();
+    expect(applications()).toEqual([]);
+    expect((await adapter.getStats()).adapter).toMatchObject({ experiencesApplied: 0, tokensSavedEstimate: 0 });
+  });
+});