fix: count { success: false } results as failures in the effect tracker (GIT-104)

src/services/cache.ts

@@ -180,29 +180,37 @@ export class CacheService {
    * Set cached result
    */
   async setResult<T>(key: string, data: T, ttlMs: number): Promise<void> {
-    if (!this.enabled) return;
-
     try {
-      const now = Date.now();
-      const entry: CacheEntry<T> = {
-        key,
-        created_at: now,
-        expires_at: now + ttlMs,
-        data,
-      };
-
-      const filename = this.keyToFilename(key);
-      const filepath = join(this.resultsDir, filename);
-      const content = JSON.stringify(entry, null, 2);
-
-      writeFileSync(filepath, content, { mode: 0o600 });
-      console.error(`[cache] SET: ${key} (TTL: ${ttlMs}ms)`);
+      await this.writeResult(key, data, ttlMs);
     } catch (error) {
       console.warn(`[cache] Error writing ${key}:`, error);
       // Don't disable cache on write errors - might be transient
     }
   }
 
+  /**
+   * Like setResult, but rejects on a write error. Used under the effect
+   * tracker so cache write failures are counted (GIT-104).
+   */
+  private async writeResult<T>(key: string, data: T, ttlMs: number): Promise<void> {
+    if (!this.enabled) return;
+
+    const now = Date.now();
+    const entry: CacheEntry<T> = {
+      key,
+      created_at: now,
+      expires_at: now + ttlMs,
+      data,
+    };
+
+    const filename = this.keyToFilename(key);
+    const filepath = join(this.resultsDir, filename);
+    const content = JSON.stringify(entry, null, 2);
+
+    writeFileSync(filepath, content, { mode: 0o600 });
+    console.error(`[cache] SET: ${key} (TTL: ${ttlMs}ms)`);
+  }
+
   /**
    * Convenience method: get or fetch scar search results
    */
@@ -224,7 +232,7 @@ export class CacheService {
 
     // Cache the result (tracked, fire-and-forget)
     getEffectTracker().track("cache_set", "scar_search", () =>
-      this.setResult(key, data, TTL.SCAR_SEARCH)
+      this.writeResult(key, data, TTL.SCAR_SEARCH)
     );
 
     return { data, cache_hit: false };
@@ -250,7 +258,7 @@ export class CacheService {
 
     // Cache the result (tracked, fire-and-forget)
     getEffectTracker().track("cache_set", "decisions", () =>
-      this.setResult(key, data, TTL.DECISIONS)
+      this.writeResult(key, data, TTL.DECISIONS)
     );
 
     return { data, cache_hit: false };
@@ -276,7 +284,7 @@ export class CacheService {
 
     // Cache the result (tracked, fire-and-forget)
     getEffectTracker().track("cache_set", "wins", () =>
-      this.setResult(key, data, TTL.WINS)
+      this.writeResult(key, data, TTL.WINS)
     );
 
     return { data, cache_hit: false };
@@ -314,7 +322,7 @@ export class CacheService {
 
     const data = await fetcher();
     getEffectTracker().track("cache_set", "sessions", () =>
-      this.setResult(key, data, TTL.SESSIONS)
+      this.writeResult(key, data, TTL.SESSIONS)
     );
     return { data, cache_hit: false };
   }
@@ -337,7 +345,7 @@ export class CacheService {
 
     const data = await fetcher();
     getEffectTracker().track("cache_set", "scar_usage", () =>
-      this.setResult(key, data, TTL.SCAR_USAGE)
+      this.writeResult(key, data, TTL.SCAR_USAGE)
     );
     return { data, cache_hit: false };
   }

src/services/effect-tracker.ts

@@ -91,6 +91,27 @@ class RingBuffer<T> {
   }
 }
 
+// ---------------------------------------------------------------------------
+// Result inspection
+// ---------------------------------------------------------------------------
+
+/**
+ * If a resolved value reports its own failure (`{ success: false }`), return a
+ * message describing it; otherwise null. Prefers the value's error, then
+ * message, then display text.
+ */
+function reportedFailure(value: unknown): string | null {
+  if (typeof value !== "object" || value === null) return null;
+  const v = value as Record<string, unknown>;
+  if (v.success !== false) return null;
+  for (const key of ["error", "message", "display"] as const) {
+    const field = v[key];
+    if (field instanceof Error) return field.message;
+    if (typeof field === "string" && field.length > 0) return field;
+  }
+  return "resolved with success: false";
+}
+
 // ---------------------------------------------------------------------------
 // EffectTracker
 // ---------------------------------------------------------------------------
@@ -106,7 +127,10 @@ export class EffectTracker {
    * Wrap an async fire-and-forget operation for tracking.
    *
    * - On success: records success + duration, returns the result.
-   * - On failure: records failure + error message, returns undefined.
+   * - On a result of the form `{ success: false, ... }`: records a failure
+   *   (GIT-104 — many tools catch their own errors and resolve with that
+   *   shape), and still returns the result to the caller.
+   * - On rejection: records failure + error message, returns undefined.
    *   The error is **not** rethrown — this is intentionally fire-and-forget.
    */
   track<T>(
@@ -118,29 +142,36 @@ export class EffectTracker {
     const stats = this.getOrCreatePath(path);
     stats.attempted++;
 
+    const recordFailure = (errorMsg: string) => {
+      const duration = Date.now() - start;
+      stats.failed++;
+      stats.totalDurationMs += duration;
+      stats.lastFailure = {
+        error: errorMsg,
+        timestamp: new Date().toISOString(),
+      };
+      this.failures.push({
+        path,
+        target,
+        error: errorMsg,
+        timestamp: new Date().toISOString(),
+        durationMs: duration,
+      });
+    };
+
     return fn().then(
       (result) => {
-        const duration = Date.now() - start;
+        const reported = reportedFailure(result);
+        if (reported !== null) {
+          recordFailure(reported);
+          return result;
+        }
         stats.succeeded++;
-        stats.totalDurationMs += duration;
+        stats.totalDurationMs += Date.now() - start;
         return result;
       },
       (error) => {
-        const duration = Date.now() - start;
-        const errorMsg = error instanceof Error ? error.message : String(error);
-        stats.failed++;
-        stats.totalDurationMs += duration;
-        stats.lastFailure = {
-          error: errorMsg,
-          timestamp: new Date().toISOString(),
-        };
-        this.failures.push({
-          path,
-          target,
-          error: errorMsg,
-          timestamp: new Date().toISOString(),
-          durationMs: duration,
-        });
+        recordFailure(error instanceof Error ? error.message : String(error));
         return undefined;
       },
     );

src/services/triple-writer.ts

@@ -276,14 +276,19 @@ export function extractDecisionTriples(params: DecisionTripleParams): TripleCand
 
 /**
  * Write triples to knowledge_triples table.
- * Fire-and-forget: errors logged but never thrown to caller.
+ *
+ * Every candidate is attempted even if an earlier one fails. If any failed,
+ * rejects afterwards with a summary so the caller's effect tracker records the
+ * failure (GIT-104) — callers run this under getEffectTracker().track(), which
+ * never rethrows.
  */
 export async function writeTriples(candidates: TripleCandidate[]): Promise<number> {
   if (candidates.length === 0 || !hasSupabase()) {
     return 0;
   }
 
   let written = 0;
+  const errors: string[] = [];
 
   for (const candidate of candidates) {
     try {
@@ -307,9 +312,11 @@ export async function writeTriples(candidates: TripleCandidate[]): Promise<numbe
       });
       written++;
     } catch (error) {
+      const message = error instanceof Error ? error.message : String(error);
+      errors.push(message);
       console.error(
         `[triple-writer] Failed to write triple: ${candidate.subject} ${candidate.predicate} ${candidate.object}`,
-        error instanceof Error ? error.message : error
+        message
       );
     }
   }
@@ -318,14 +325,18 @@ export async function writeTriples(candidates: TripleCandidate[]): Promise<numbe
     console.error(`[triple-writer] Wrote ${written}/${candidates.length} triples`);
   }
 
+  if (errors.length > 0) {
+    throw new Error(`${errors.length}/${candidates.length} triples failed: ${errors[0]}`);
+  }
+
   return written;
 }
 
 // --- Public API ---
 
 /**
  * Generate and write triples for a newly created learning.
- * Fire-and-forget — call with .catch(() => {}).
+ * Fire-and-forget — run under getEffectTracker().track(); rejects if any triple failed.
  */
 export function writeTriplesForLearning(params: LearningTripleParams): Promise<number> {
   const triples = extractLearningTriples(params);
@@ -334,7 +345,7 @@ export function writeTriplesForLearning(params: LearningTripleParams): Promise<n
 
 /**
  * Generate and write triples for a newly created decision.
- * Fire-and-forget — call with .catch(() => {}).
+ * Fire-and-forget — run under getEffectTracker().track(); rejects if any triple failed.
  */
 export function writeTriplesForDecision(params: DecisionTripleParams): Promise<number> {
   const triples = extractDecisionTriples(params);
@@ -430,7 +441,7 @@ export function extractThreadResolutionTriples(params: ThreadResolutionTriplePar
 
 /**
  * Generate and write triples for a newly created thread.
- * Fire-and-forget — call with .catch(() => {}).
+ * Fire-and-forget — run under getEffectTracker().track(); rejects if any triple failed.
  */
 export function writeTriplesForThreadCreation(params: ThreadCreationTripleParams): Promise<number> {
   const triples = extractThreadCreationTriples(params);
@@ -439,7 +450,7 @@ export function writeTriplesForThreadCreation(params: ThreadCreationTripleParams
 
 /**
  * Generate and write triples for a resolved thread.
- * Fire-and-forget — call with .catch(() => {}).
+ * Fire-and-forget — run under getEffectTracker().track(); rejects if any triple failed.
  */
 export function writeTriplesForThreadResolution(params: ThreadResolutionTripleParams): Promise<number> {
   const triples = extractThreadResolutionTriples(params);

src/services/variant-generation.ts

@@ -335,4 +335,12 @@ export async function generateVariantsForScar(scar: ScarData): Promise<void> {
   console.error(
     `[variant-generation] ${generationSource}: created ${created}/2 variants for scar ${scar.id} (pipeline ${PIPELINE_VERSION})`
   );
+
+  // Surface partial failure to the effect tracker (GIT-104) instead of
+  // resolving as if both variants were written.
+  const firstFailure = results.find((r): r is PromiseRejectedResult => r.status === "rejected");
+  if (firstFailure) {
+    const reason = firstFailure.reason instanceof Error ? firstFailure.reason.message : String(firstFailure.reason);
+    throw new Error(`created ${created}/${variants.length} variants: ${reason}`);
+  }
 }

src/tools/record-scar-usage-batch.ts

@@ -206,6 +206,7 @@ export async function recordScarUsageBatch(
 
     return {
       success: false,
+      error: error instanceof Error ? error.message : String(error),
       usage_ids: usageIds,
       resolved_count: resolvedCount,
       failed_count: params.scars.length - resolvedCount,

src/tools/session-close.ts

@@ -1384,6 +1384,8 @@ export async function sessionClose(
             })
             .catch((err) => console.error("[session_close] Transcript processing failed:", err instanceof Error ? err.message : err));
         }
+        // Return the result so a { success: false } save is counted as a failure (GIT-104).
+        return saveResult;
       });
     }
   }

src/types/index.ts

@@ -480,6 +480,8 @@ export interface RecordScarUsageBatchResult {
   resolved_count: number;
   failed_count: number;
   failed_identifiers?: string[];
+  /** Why the batch failed (set when success is false) */
+  error?: string;
   display?: string;
   performance: PerformanceData;
 }