diff --git a/src/services/write-result.ts b/src/services/write-result.ts new file mode 100644 index 0000000..850fb8a --- /dev/null +++ b/src/services/write-result.ts @@ -0,0 +1,47 @@ +/** + * WriteResult — the one contract every write tool answers with (GIT-101). + * + * create_thread established it (GIT-63/GIT-67, R3/R4): say where the record + * landed and whether it survives this machine, and only call it a success + * when it reached the store of truth for this tier. Other write tools grew + * their own shapes — resolve_thread returned success: true whether or not the + * durable store accepted the write — so a caller could not tell a saved + * record from a lost one without reading prose. + * + * durable the record is in Supabase (pro/dev). Never true on free. + * stored_in "supabase" | "local" (free: the local store IS the truth) | + * "local_only" (pro: kept on this machine only, the durable + * write failed) | null (stored nowhere). + * success durable || !hasSupabase() — and false whenever nothing was + * stored at all. + */ + +import { hasSupabase } from "./tier.js"; +import type { WriteResult, StoredIn } from "../types/index.js"; + +export type { WriteResult, StoredIn }; + +/** + * The outcome of a write. + * + * @param durable the durable store (Supabase) accepted the write + * @param storedLocally a local copy exists (threads.json, the free-tier + * store, session state). Default: true on free, where + * reaching this call means the local write succeeded; + * false on pro, where a failed durable write keeps + * nothing unless the tool says it did. + */ +export function writeResult(durable: boolean, storedLocally: boolean = !hasSupabase()): WriteResult { + if (durable && hasSupabase()) return { success: true, durable: true, stored_in: "supabase" }; + if (!hasSupabase()) { + return storedLocally + ? { success: true, durable: false, stored_in: "local" } + : notStored(); + } + return { success: false, durable: false, stored_in: storedLocally ? "local_only" : null }; +} + +/** Nothing was stored anywhere. */ +export function notStored(): WriteResult { + return { success: false, durable: false, stored_in: null }; +} diff --git a/src/tools/archive-learning.ts b/src/tools/archive-learning.ts index 898413d..eeb020e 100644 --- a/src/tools/archive-learning.ts +++ b/src/tools/archive-learning.ts @@ -16,6 +16,8 @@ import { getStorage } from "../services/storage.js"; import { flushCache } from "../services/startup.js"; import { Timer } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; export interface ArchiveLearningParams { /** UUID or short ID prefix of the learning to archive */ @@ -24,8 +26,7 @@ export interface ArchiveLearningParams { reason?: string; } -export interface ArchiveLearningResult { - success: boolean; +export interface ArchiveLearningResult extends WriteResult { id: string; archived_at?: string; reason?: string; @@ -95,7 +96,7 @@ export async function archiveLearning(params: ArchiveLearningParams): Promise {}); return { - success: true, + ...writeResult(hasSupabase()), decision_id: decisionId, performance: perfData, display: wrapDisplay(`Decision logged: "${params.title}"\nID: ${decisionId}`), @@ -151,7 +152,7 @@ export async function createDecision( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_decision", latencyMs, 0); return { - success: false, + ...notStored(), decision_id: "", performance: perfData, display: wrapDisplay(`Failed to log decision`), diff --git a/src/tools/create-learning.ts b/src/tools/create-learning.ts index b65a660..db9b929 100644 --- a/src/tools/create-learning.ts +++ b/src/tools/create-learning.ts @@ -32,6 +32,7 @@ import type { PerformanceBreakdown, ComponentPerformance, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; /** * Validate scar-specific requirements @@ -85,7 +86,7 @@ export async function createLearning( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_learning", latencyMs, 0); return { - success: false, + ...notStored(), learning_id: "", embedding_generated: false, errors, @@ -314,7 +315,7 @@ export async function createLearning( const se = params.severity ? (SEV[params.severity] || "") + " " : ""; return { - success: true, + ...writeResult(hasSupabase()), learning_id: learningId, embedding_generated: embeddingGenerated, performance: perfData, @@ -327,7 +328,7 @@ export async function createLearning( const latencyMs = timer.stop(); const perfData = buildPerformanceData("create_learning", latencyMs, 0); return { - success: false, + ...notStored(), learning_id: "", embedding_generated: false, errors: [errorMsg], diff --git a/src/tools/create-thread.ts b/src/tools/create-thread.ts index 691de05..36e72d4 100644 --- a/src/tools/create-thread.ts +++ b/src/tools/create-thread.ts @@ -41,7 +41,7 @@ import { import { wrapDisplay, truncate } from "../services/display-protocol.js"; import { formatThreadForDisplay } from "../services/timezone.js"; import type { ThreadWithEmbedding } from "../services/thread-dedup.js"; -import type { ThreadObject, PerformanceData, Project } from "../types/index.js"; +import type { ThreadObject, PerformanceData, Project, WriteResult, StoredIn } from "../types/index.js"; // --- Types --- @@ -73,21 +73,17 @@ export type NotStoredReason = /** * Where a thread actually landed (R3: every response names its store). * `local_only` is the honest middle state from R4 — written somewhere, but - * not somewhere durable. + * not somewhere durable. GIT-101: this contract is now shared by every write + * tool (WriteResult). */ -export type StoredIn = "supabase" | "local" | "local_only" | null; +export type { StoredIn } from "../types/index.js"; -export interface CreateThreadResult { - success: boolean; +export interface CreateThreadResult extends WriteResult { /** * Whether a row exists. Distinct from `success` on purpose: a dedup refusal * is a well-formed answer to a well-formed request, not a crash. */ stored: boolean; - /** Whether what was stored survives this machine. */ - durable: boolean; - /** R3: names the store in-band, which also kills the f104e10d silent-local trap. */ - stored_in: StoredIn; reason?: NotStoredReason; thread?: ThreadObject; error?: string; diff --git a/src/tools/promote-suggestion.ts b/src/tools/promote-suggestion.ts index 1d00b5f..1b359d2 100644 --- a/src/tools/promote-suggestion.ts +++ b/src/tools/promote-suggestion.ts @@ -22,6 +22,8 @@ import { buildPerformanceData, } from "../services/metrics.js"; import type { ThreadObject, PerformanceData, Project, ThreadSuggestion } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; // --- Types --- @@ -30,8 +32,7 @@ export interface PromoteSuggestionParams { project?: Project; } -export interface PromoteSuggestionResult { - success: boolean; +export interface PromoteSuggestionResult extends WriteResult { thread?: ThreadObject; suggestion?: ThreadSuggestion; error?: string; @@ -49,7 +50,7 @@ export async function promoteSuggestion( if (!params.suggestion_id) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "suggestion_id is required", performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Failed: suggestion_id is required`), @@ -64,7 +65,7 @@ export async function promoteSuggestion( if (!target) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: `Pending suggestion not found: "${params.suggestion_id}"`, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Suggestion not found: ${params.suggestion_id}`), @@ -79,8 +80,12 @@ export async function promoteSuggestion( if (!threadResult.success || !threadResult.thread) { const latencyMs = timer.stop(); + // GIT-101: pass create_thread's own outcome through (a local_only thread + // is not a promotion), rather than restating it. return { - success: false, + ...notStored(), + durable: threadResult.durable, + stored_in: threadResult.stored_in, error: `Thread creation failed: ${threadResult.error || "unknown"}`, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 0), display: wrapDisplay(`Failed to promote suggestion`), @@ -93,7 +98,9 @@ export async function promoteSuggestion( const latencyMs = timer.stop(); return { - success: true, + success: threadResult.success, + durable: threadResult.durable, + stored_in: threadResult.stored_in, thread: threadResult.thread, suggestion: target, performance: buildPerformanceData("promote_suggestion" as any, latencyMs, 1), diff --git a/src/tools/record-scar-usage-batch.ts b/src/tools/record-scar-usage-batch.ts index d364a08..5c634b7 100644 --- a/src/tools/record-scar-usage-batch.ts +++ b/src/tools/record-scar-usage-batch.ts @@ -15,6 +15,7 @@ import type { ScarUsageEntry, Project, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; const TARGET_LATENCY_MS = 2000; // Target for batch operation @@ -98,9 +99,13 @@ export async function recordScarUsageBatch( const metricsId = uuidv4(); if (!hasSupabase()) { - // Free tier: scar usage tracked locally via record-scar-usage (single), not batch + // Free tier: scar usage tracked locally via record-scar-usage (single), not batch. + // GIT-101: nothing is stored here, and the result says so (stored_in: null); + // success stays true because there is no durable store to have missed. return { success: true, + durable: false, + stored_in: null, usage_ids: [], resolved_count: 0, failed_count: 0, @@ -191,7 +196,7 @@ export async function recordScarUsageBatch( }); return { - success: true, + ...writeResult(hasSupabase()), usage_ids: usageIds, resolved_count: resolvedCount, failed_count: failedIdentifiers.length, @@ -205,7 +210,7 @@ export async function recordScarUsageBatch( console.error("[record-scar-usage-batch] Error recording batch:", error); return { - success: false, + ...notStored(), error: error instanceof Error ? error.message : String(error), usage_ids: usageIds, resolved_count: resolvedCount, diff --git a/src/tools/record-scar-usage.ts b/src/tools/record-scar-usage.ts index 40ca688..9b5daa9 100644 --- a/src/tools/record-scar-usage.ts +++ b/src/tools/record-scar-usage.ts @@ -24,6 +24,7 @@ import type { RecordScarUsageResult, PerformanceBreakdown, } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; /** * Execute record_scar_usage tool @@ -101,7 +102,7 @@ export async function recordScarUsage( }).catch(() => {}); return { - success: true, + ...writeResult(hasSupabase()), usage_id: usageId, performance: perfData, display: wrapDisplay(`Scar usage recorded\nID: ${usageId}`), @@ -112,7 +113,7 @@ export async function recordScarUsage( const latencyMs = timer.stop(); const perfData = buildPerformanceData("record_scar_usage", latencyMs, 0); return { - success: false, + ...notStored(), usage_id: "", errors: [errorMsg], performance: perfData, diff --git a/src/tools/resolve-thread.ts b/src/tools/resolve-thread.ts index 6e0e0fb..3fbbc78 100644 --- a/src/tools/resolve-thread.ts +++ b/src/tools/resolve-thread.ts @@ -44,6 +44,8 @@ import { buildPerformanceData, } from "../services/metrics.js"; import { wrapDisplay } from "../services/display-protocol.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import { hasSupabase } from "../services/tier.js"; import { formatThreadForDisplay } from "../services/timezone.js"; import type { ResolveThreadParams, ResolveThreadResult, ThreadObject } from "../types/index.js"; @@ -57,7 +59,7 @@ export async function resolveThread( if (!params.thread_id && !params.text_match) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "Either thread_id or text_match is required", performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(`Either thread_id or text_match is required`), @@ -87,7 +89,7 @@ export async function resolveThread( if (pos < 1 || pos > openThreads.length) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: `Thread #${pos} out of range (${openThreads.length} open threads)`, performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(`Thread #${pos} out of range (${openThreads.length} open threads)`), @@ -162,7 +164,7 @@ export async function resolveThread( : `Thread not found: "${searchKey}"`; return { - success: false, + ...notStored(), error: message, performance: buildPerformanceData("resolve_thread", latencyMs, 0), display: wrapDisplay(message), @@ -193,7 +195,9 @@ export async function resolveThread( // Persist to local file (cache) saveThreadsFile(threads); - // Update Supabase (source of truth) — graceful fallback on failure + // Update Supabase (source of truth). GIT-101: the outcome is the result — + // it used to be computed, logged to metrics and then dropped, so a resolve + // the durable store never saw still answered success: true. let supabaseSynced = false; const supabaseSuccess = await resolveThreadInSupabase(resolved.id, { resolvedAt: resolved.resolved_at, @@ -275,8 +279,18 @@ export async function resolveThread( if (alsoResolved.length > 0) resolveMsg += `\nAlso resolved: ${alsoResolved.map(t => t.id).join(", ")}`; + // The local file was written above either way; on pro that alone is not a success. + const outcome = writeResult(supabaseSynced, true); + if (hasSupabase() && !outcome.durable) { + resolveMsg = + `RESOLVED LOCALLY ONLY — not durable (stored_in: local_only, durable: false).\n` + + `${resolveMsg}\n` + + `The durable store did not accept this resolve, so other sessions still see ${resolved.id} as open. ` + + `Retry when the store is reachable.`; + } + return { - success: true, + ...outcome, resolved_thread: formatThreadForDisplay(resolved), ...(alsoResolved.length > 0 && { also_resolved: alsoResolved.map(formatThreadForDisplay), diff --git a/src/tools/save-transcript.ts b/src/tools/save-transcript.ts index 88a487d..b48e6f5 100644 --- a/src/tools/save-transcript.ts +++ b/src/tools/save-transcript.ts @@ -12,6 +12,8 @@ import * as supabase from "../services/supabase-client.js"; import { Timer, buildPerformanceData } from "../services/metrics.js"; import { getProject } from "../services/session-state.js"; import type { Project, PerformanceData, PerformanceBreakdown } from "../types/index.js"; +import { writeResult, notStored } from "../services/write-result.js"; +import type { WriteResult } from "../types/index.js"; export interface SaveTranscriptParams { session_id: string; @@ -20,8 +22,7 @@ export interface SaveTranscriptParams { project?: Project; } -export interface SaveTranscriptResult { - success: boolean; +export interface SaveTranscriptResult extends WriteResult { transcript_path?: string; size_bytes?: number; size_kb?: number; @@ -43,7 +44,7 @@ export async function saveTranscript( if (!params.session_id) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "session_id is required", performance: buildPerformanceData("save_transcript", latencyMs, 0), }; @@ -52,7 +53,7 @@ export async function saveTranscript( if (!params.transcript) { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), error: "transcript content is required", performance: buildPerformanceData("save_transcript", latencyMs, 0), }; @@ -108,7 +109,8 @@ export async function saveTranscript( const estimatedTokens = Math.ceil(result.size_bytes / 4); return { - success: true, + // Transcripts only go to the durable store; reaching here means it took them. + ...writeResult(true), transcript_path: result.transcript_path, size_bytes: result.size_bytes, size_kb: Math.round(result.size_bytes / 1024 * 10) / 10, @@ -122,7 +124,7 @@ export async function saveTranscript( const latencyMs = timer.stop(); const errorMessage = error instanceof Error ? error.message : String(error); return { - success: false, + ...notStored(), error: `Failed to save transcript: ${errorMessage}`, performance: buildPerformanceData("save_transcript", latencyMs, 0), }; diff --git a/src/tools/session-close.ts b/src/tools/session-close.ts index 2388a4c..9cfe54a 100644 --- a/src/tools/session-close.ts +++ b/src/tools/session-close.ts @@ -36,6 +36,7 @@ import { import { wrapDisplay, truncate, productLine, boldText, dimText, STATUS, ANSI } from "../services/display-protocol.js"; import { queryScarUsageByDateRange, enrichScarUsageTitles, formatBlindspotSnippet } from "../services/analytics.js"; import { recordScarUsageBatch } from "./record-scar-usage-batch.js"; +import { writeResult, notStored } from "../services/write-result.js"; import { getEffectTracker } from "../services/effect-tracker.js"; import { saveTranscript } from "./save-transcript.js"; import { processTranscript } from "../services/transcript-chunker.js"; @@ -318,7 +319,7 @@ async function sessionCloseFree( const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, true); return { - success: true, + ...writeResult(false), session_id: sessionId, close_compliance: closeCompliance, performance: perfData, @@ -332,7 +333,7 @@ async function sessionCloseFree( const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`]); return { - success: false, + ...notStored(), session_id: sessionId, close_compliance: closeCompliance, validation_errors: [`Failed to persist session: ${errorMessage}`], @@ -862,7 +863,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: params.session_id, close_compliance: { close_type: params.close_type, @@ -908,7 +909,7 @@ export async function sessionClose( if (!params.session_id && params.close_type !== "retroactive") { const latencyMs = timer.stop(); return { - success: false, + ...notStored(), session_id: "", close_compliance: { close_type: params.close_type, @@ -932,6 +933,9 @@ export async function sessionClose( // This keeps the visible MCP tool call small: just session_id + close_type. const payloadPath = getGitmemPath("closing-payload.json"); let payloadConsumed = false; + // GIT-101: whether the session row reached the durable store. Set only after + // the upsert resolves, so every exit below can say so. + let sessionRowStored = false; try { if (fs.existsSync(payloadPath)) { const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial; @@ -1066,7 +1070,7 @@ export async function sessionClose( // Hard gate: quick close requires session under 30 minutes if (params.close_type === "quick" && activity.duration_min >= 30) { return { - success: false, + ...notStored(), session_id: params.session_id || "", close_compliance: { close_type: "quick", @@ -1099,7 +1103,7 @@ export async function sessionClose( // Exemptions: quick (micro sessions), autonomous (CODA-1), hasReflection (agent already wrote full reflection) if (params.close_type === "standard" && !isRecallCalled() && !hasReflection) { return { - success: false, + ...notStored(), session_id: params.session_id || "", close_compliance: { close_type: "standard", @@ -1155,7 +1159,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: "", // Empty string when no session found close_compliance: { close_type: params.close_type, @@ -1179,7 +1183,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: "", // Empty string when search fails close_compliance: { close_type: params.close_type, @@ -1206,7 +1210,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: params.session_id, close_compliance: { close_type: params.close_type, @@ -1304,7 +1308,7 @@ export async function sessionClose( const latencyMs = timer.stop(); const perfData = buildPerformanceData("session_close", latencyMs, 0); return { - success: false, + ...notStored(), session_id: sessionId, close_compliance: closeCompliance, validation_errors: [`Session ${sessionId} not found in Supabase, local files, or registry. Was session_start called?`], @@ -1470,6 +1474,7 @@ export async function sessionClose( supabase.directUpsert(sessionsTable, sessionRow), blindspotPromise, ]); + sessionRowStored = true; // Tracked fire-and-forget embedding generation + session update + thread detection if (isEmbeddingAvailable()) { @@ -1645,6 +1650,8 @@ export async function sessionClose( const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet); return { + // The session row is durable; success also requires every thread to sync. + ...writeResult(true), success: !partialPersist, session_id: sessionId, close_compliance: closeCompliance, @@ -1663,6 +1670,7 @@ export async function sessionClose( const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet); return { + ...writeResult(sessionRowStored), success: false, session_id: sessionId, close_compliance: closeCompliance, diff --git a/src/types/index.ts b/src/types/index.ts index 61654e7..a7468bf 100644 --- a/src/types/index.ts +++ b/src/types/index.ts @@ -291,8 +291,21 @@ export interface CloseCompliance { retroactive?: boolean; // Optional: marks sessions created post-mortem } -export interface SessionCloseResult { +/** + * GIT-101: the contract every write tool answers with — where the record + * landed and whether it survives this machine. See services/write-result.ts. + */ +export type StoredIn = "supabase" | "local" | "local_only" | null; +export interface WriteResult { + /** durable || !hasSupabase(), and false when nothing was stored. */ success: boolean; + /** Whether what was stored survives this machine. */ + durable: boolean; + /** Names the store in-band. */ + stored_in: StoredIn; +} + +export interface SessionCloseResult extends WriteResult { session_id: string; close_compliance: CloseCompliance; validation_errors?: string[]; @@ -325,8 +338,7 @@ export interface CreateLearningParams { self_check_criteria?: string[]; } -export interface CreateLearningResult { - success: boolean; +export interface CreateLearningResult extends WriteResult { learning_id: string; embedding_generated: boolean; /** Error details when success=false */ @@ -348,8 +360,7 @@ export interface CreateDecisionParams { project?: Project; } -export interface CreateDecisionResult { - success: boolean; +export interface CreateDecisionResult extends WriteResult { decision_id: string; display?: string; performance: PerformanceData; @@ -445,8 +456,7 @@ export interface RecordScarUsageParams { variant_id?: string; // UUID of assigned variant for A/B testing } -export interface RecordScarUsageResult { - success: boolean; +export interface RecordScarUsageResult extends WriteResult { usage_id: string; /** Error details when success=false */ errors?: string[]; @@ -474,8 +484,7 @@ export interface RecordScarUsageBatchParams { project?: Project; } -export interface RecordScarUsageBatchResult { - success: boolean; +export interface RecordScarUsageBatchResult extends WriteResult { usage_ids: string[]; resolved_count: number; failed_count: number; @@ -598,8 +607,7 @@ export interface ResolveThreadParams { resolution_note?: string; } -export interface ResolveThreadResult { - success: boolean; +export interface ResolveThreadResult extends WriteResult { resolved_thread?: ThreadObject; /** Threads that were also resolved via duplicate cascade */ also_resolved?: ThreadObject[]; diff --git a/tests/e2e/blank-supabase.mjs b/tests/e2e/blank-supabase.mjs index 84b343b..02a1eca 100644 --- a/tests/e2e/blank-supabase.mjs +++ b/tests/e2e/blank-supabase.mjs @@ -366,6 +366,17 @@ async function flow() { const lt = await step("list_threads", { project: PROJECT }); const ltOpen = Number((lt.match(/(\d+) open/) || [])[1] ?? -1); + // GIT-101: resolve a second driver thread; the durable store must record it + // and the response must not claim a local-only resolve as done. + const ct2 = await step("create_thread", { text: `Close out the quarterly vendor audit checklist (${label})`, allow_duplicate: true }); + const resolveId = (ct2.match(/\bt-[0-9a-f]{8}\b/) || [])[0] ?? null; + const rt = resolveId ? await step("resolve_thread", { thread_id: resolveId, resolution_note: "venue driver" }) : ""; + const resolvedRow = resolveId + ? await rest("GET", `gitmem_threads?select=status,resolved_by_session&thread_id=eq.${resolveId}`, undefined, { Prefer: "" }).then((r) => r.json()) + : []; + const resolveDurable = !!resolveId && /Thread resolved/.test(rt) && !/LOCALLY ONLY/.test(rt) + && resolvedRow[0]?.status === "resolved"; + const sc = await step("session_close", { session_id: sessionId, close_type: "standard", human_corrections: "none", closing_reflection: { @@ -408,6 +419,7 @@ async function flow() { remote_sessions_after: await count("gitmem_sessions"), remote_scar_usage_after: await count("gitmem_scar_usage"), session_close_persisted: closedRows.length === 1 && closedRows[0].closing_reflection != null, + resolve_thread_durable: resolveDurable, relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])), relevance, health_failed_total: failed, @@ -520,6 +532,9 @@ const failureCheck = checkFailures(allVenueRequests); const edgeCalls = allVenueRequests.filter((r) => r.path.startsWith("/functions/v1/")) .map((r) => `${r.method} ${r.path} -> ${r.status}`); if (edgeCalls.length) failureCheck.unexpected.push(...edgeCalls.map((c) => `edge function called (GIT-97): ${c}`)); +if (mode === "flow" && !result.resolve_thread_durable) { + failureCheck.unexpected.push("resolve_thread did not land durably in gitmem_threads, or claimed it did not (GIT-101)"); +} if (mode === "flow" && !result.session_close_persisted) { failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`); } diff --git a/tests/unit/tools/write-result-contract.test.ts b/tests/unit/tools/write-result-contract.test.ts new file mode 100644 index 0000000..de7455e --- /dev/null +++ b/tests/unit/tools/write-result-contract.test.ts @@ -0,0 +1,137 @@ +/** + * GIT-101: every write tool answers with WriteResult, and none of them calls a + * write the durable store never saw a success. + * + * One parameterized test: Pro tier, Supabase configured, and every request to + * it fails at the network. Each tool runs its real write path and must report + * success: false, durable: false — and a stored_in that names what actually + * survived (local_only when a local copy was kept, null when nothing was). + * + * resolve_thread used to return success: true here unconditionally. + */ +import { describe, it, expect, vi, beforeAll, afterAll, beforeEach } from "vitest"; +import * as fs from "fs"; +import * as os from "os"; +import * as path from "path"; + +const saved = vi.hoisted(() => { + const keys = ["GITMEM_TIER", "SUPABASE_URL", "SUPABASE_SERVICE_ROLE_KEY", "GITMEM_DIR", "OPENAI_API_KEY", "OPENROUTER_API_KEY", "GITMEM_EMBEDDING_PROVIDER"]; + const before = Object.fromEntries(keys.map((k) => [k, process.env[k]])); + const dir = require("fs").mkdtempSync(require("path").join(require("os").tmpdir(), "gitmem-git101-")); + Object.assign(process.env, { + GITMEM_TIER: "pro", + SUPABASE_URL: "https://stubbed-venue.supabase.co", + SUPABASE_SERVICE_ROLE_KEY: "stub-key", + GITMEM_DIR: dir, + }); + delete process.env.OPENAI_API_KEY; + delete process.env.OPENROUTER_API_KEY; + delete process.env.GITMEM_EMBEDDING_PROVIDER; + return { before, dir }; +}); + +import { resetTier, hasSupabase } from "../../../src/services/tier.js"; +import { setCurrentSession, clearCurrentSession } from "../../../src/services/session-state.js"; +import { saveThreadsFile } from "../../../src/services/thread-manager.js"; +import { saveSuggestions } from "../../../src/services/thread-suggestions.js"; +import { createLearning } from "../../../src/tools/create-learning.js"; +import { createDecision } from "../../../src/tools/create-decision.js"; +import { createThread } from "../../../src/tools/create-thread.js"; +import { resolveThread } from "../../../src/tools/resolve-thread.js"; +import { archiveLearning } from "../../../src/tools/archive-learning.js"; +import { recordScarUsage } from "../../../src/tools/record-scar-usage.js"; +import { recordScarUsageBatch } from "../../../src/tools/record-scar-usage-batch.js"; +import { sessionClose } from "../../../src/tools/session-close.js"; +import { promoteSuggestion } from "../../../src/tools/promote-suggestion.js"; +import { saveTranscript } from "../../../src/tools/save-transcript.js"; + +const SID = "393adb34-a80c-4c3a-b71a-bc0053b7a7ea"; +const SCAR = "11111111-2222-4333-8444-555555555555"; +const now = () => new Date().toISOString(); +let supabaseCalls = 0; + +type Case = { + tool: string; + run: () => Promise<{ success: boolean; durable: boolean; stored_in: unknown; display?: string }>; + /** What a failed durable write leaves behind. */ + storedIn: "local_only" | null; + setup?: () => void; +}; + +const CASES: Case[] = [ + { tool: "create_learning", storedIn: null, + run: () => createLearning({ learning_type: "win", title: "t", description: "d" } as never) }, + { tool: "create_decision", storedIn: null, + run: () => createDecision({ title: "t", decision: "d", rationale: "r" }) }, + { tool: "create_thread", storedIn: "local_only", + run: () => createThread({ text: "GIT-101 contract thread", allow_duplicate: true }) }, + { tool: "resolve_thread", storedIn: "local_only", + setup: () => saveThreadsFile([{ id: "t-git10101", text: "resolve me", status: "open", created_at: now() } as never]), + run: () => resolveThread({ thread_id: "t-git10101" }) }, + { tool: "archive_learning", storedIn: null, + run: () => archiveLearning({ id: SCAR }) }, + { tool: "record_scar_usage", storedIn: null, + run: () => recordScarUsage({ scar_id: SCAR, surfaced_at: now(), reference_type: "explicit", reference_context: "c" }) }, + { tool: "record_scar_usage_batch", storedIn: null, + run: () => recordScarUsageBatch({ scars: [{ scar_identifier: SCAR, surfaced_at: now(), reference_type: "explicit", reference_context: "c" }] } as never) }, + { tool: "session_close", storedIn: null, + run: () => sessionClose({ session_id: SID, close_type: "standard", human_corrections: "none", + closing_reflection: { what_broke: "x", what_took_longer: "x", do_differently: "x", what_worked: "x", + wrong_assumption: "x", scars_applied: [], institutional_memory_items: "x", collaborative_dynamic: "x", rapport_notes: "x" } } as never) }, + { tool: "promote_suggestion", storedIn: "local_only", + setup: () => saveSuggestions([{ id: "ts-git10101", text: "promote me", embedding: null, evidence_sessions: ["a", "b", "c"], + similarity_score: 0.9, status: "pending", created_at: now() } as never]), + run: () => promoteSuggestion({ suggestion_id: "ts-git10101" }) }, + { tool: "save_transcript", storedIn: null, + run: () => saveTranscript({ session_id: SID, transcript: "hello" }) }, +]; + +beforeAll(() => { + resetTier(); + vi.stubGlobal("fetch", vi.fn(async (input: unknown) => { + const url = String(input instanceof Request ? input.url : input); + if (url.includes("stubbed-venue.supabase.co")) supabaseCalls++; + throw new TypeError("fetch failed (GIT-101 stub: store unreachable)"); + })); +}); + +afterAll(() => { + vi.unstubAllGlobals(); + clearCurrentSession(); + for (const [k, v] of Object.entries(saved.before)) { + if (v === undefined) delete process.env[k]; else process.env[k] = v; + } + resetTier(); + fs.rmSync(saved.dir, { recursive: true, force: true }); +}); + +beforeEach(() => { + supabaseCalls = 0; + setCurrentSession({ sessionId: SID, agent: "cli", project: "default", startedAt: new Date() }); +}); + +describe("WriteResult contract with Supabase failing (GIT-101)", () => { + it("runs on the pro tier", () => { + expect(hasSupabase()).toBe(true); + }); + + it.each(CASES)("$tool: success false, durable false, stored_in $storedIn", async ({ run, setup, storedIn }) => { + setup?.(); + const result = await run(); + + expect(supabaseCalls).toBeGreaterThan(0); // the durable store was actually tried + expect(result.success).toBe(false); + expect(result.durable).toBe(false); + expect(result.stored_in).toBe(storedIn); + // Over MCP a client sees only `display` when a tool sets one (server.ts), + // so the display must carry the same verdict as the fields. + const display = (result as { display?: string }).display; + if (display !== undefined) expect(display).toMatch(/fail|not stored|not durable|locally only/i); + }); + + it("resolve_thread says so in its display, not just its fields", async () => { + saveThreadsFile([{ id: "t-git10102", text: "resolve me too", status: "open", created_at: now() } as never]); + const result = await resolveThread({ thread_id: "t-git10102" }); + expect(result.display).toContain("RESOLVED LOCALLY ONLY — not durable"); + }); +});