Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ import {
flushCache,
startBackgroundInit,
} from "./services/startup.js";
import { checkWritePath } from "./services/write-health.js";
import { checkWritePathWithTimeout, getLastWritePathVerdict, formatWritePathLine } from "./services/write-health.js";
import { getEffectTracker } from "./services/effect-tracker.js";
import { RIPPLE, ANSI } from "./services/display-protocol.js";
import { getProject } from "./services/session-state.js";
Expand Down Expand Up @@ -371,9 +371,13 @@ export function createServer(): Server {
const tracker = getEffectTracker();
const report = tracker.getHealthReport(failureLimit);
const summary = tracker.formatSummary();
// GIT-102: the startup write-path verdict, including "timed out,
// durability UNVERIFIED" — previously visible only on stderr, if at all.
const writePath = getLastWritePathVerdict();
result = {
...report,
text: summary || "No tracked effects this session.",
write_path: writePath,
text: `${formatWritePathLine(writePath)}\n\n${summary || "No tracked effects this session."}`,
};
break;
}
Expand Down Expand Up @@ -501,14 +505,14 @@ export async function runServer(): Promise<void> {
}
// Verify the write path with the FINAL tier (post-validation): catches an
// invalid license silently downgrading writes to local files.
void checkWritePath();
void checkWritePathWithTimeout(); // GIT-102: bounded; verdict kept for health
}).catch((err) => {
console.error(`[gitmem:license] Validation error: ${err}`);
});
} else {
// No license key to validate — verify the write path with the detected tier
// (catches a GITMEM_TABLE_PREFIX / schema mismatch on the backward-compat path).
void checkWritePath();
void checkWritePathWithTimeout(); // GIT-102: bounded; verdict kept for health
}

if (hasSupabase()) {
Expand Down
125 changes: 123 additions & 2 deletions src/services/write-health.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,12 @@
* Fire-and-forget: never throws (whole body is guarded), never blocks startup.
* Logs a loud warning to stderr when misconfigured, a one-line confirmation
* otherwise.
*
* GIT-102: the probe had no timeout. A store that accepts the connection and
* never answers left it pending forever — no warning, no confirmation, and
* nothing anywhere saying durability had not been checked. The server now runs
* it through checkWritePathWithTimeout(): raced against 10 s, with a
* "timed_out" verdict that says so, and the last verdict kept for `health`.
*/

import { getTier, hasSupabase, getTablePrefix, getTableName } from "./tier.js";
Expand All @@ -30,12 +36,115 @@ export type WritePathMode =
| "free_with_credentials" // creds present but tier free — writes silently local (bug)
| "missing_tables" // pro/dev but resolved tables absent (bug)
| "supabase" // healthy: pro/dev with tables present
| "skipped"; // unexpected internal error — stayed silent, did not block startup
| "skipped" // unexpected internal error — stayed silent, did not block startup
| "timed_out" // GIT-102: no answer within the budget — durability UNVERIFIED
| "unreachable"; // GIT-102: probes errored (network/auth) — durability UNVERIFIED

export interface WritePathResult {
ok: boolean;
mode: WritePathMode;
missing?: string[];
/** unreachable: the probe error, as the store reported it. */
error?: string;
}

/** The most recent write-path verdict, for `health` (GIT-102). */
export interface WritePathVerdict extends WritePathResult {
/** One line a person can act on. */
summary: string;
checked_at: string;
duration_ms: number;
/** A check that finished after its timeout had already been reported. */
late?: boolean;
}

export const WRITE_PATH_TIMEOUT_MS = 10_000;

let lastVerdict: WritePathVerdict | null = null;

/** The last write-path verdict, or null when no check has finished or timed out yet. */
export function getLastWritePathVerdict(): WritePathVerdict | null {
return lastVerdict;
}

/** For tests. */
export function resetWritePathVerdict(): void {
lastVerdict = null;
}

/** The `health` line for a verdict (GIT-102). */
export function formatWritePathLine(v: WritePathVerdict | null): string {
if (!v) return "Write path: not checked yet (the startup check has not finished or timed out)";
return `Write path: ${v.summary} (${v.ok ? "ok" : "NOT OK"}, checked ${v.checked_at}, ${v.duration_ms} ms` +
`${v.late ? ", finished after its timeout" : ""})`;
}

function summarize(r: WritePathResult, timeoutMs: number): string {
switch (r.mode) {
case "supabase": return "Supabase — learnings/decisions tables present, writes are durable";
case "local": return "local files (free tier, no Supabase configured) — intended";
case "free_with_credentials": return "Supabase is configured but the tier resolved to FREE — writes are going to local files, NOT Supabase";
case "missing_tables": return `tables missing on the Supabase backend (${(r.missing || []).join(", ")}) — create_learning / create_decision will fail`;
case "skipped": return "check skipped after an internal error — durability UNVERIFIED";
case "timed_out": return `timed out after ${Math.round(timeoutMs / 1000)} s, durability UNVERIFIED`;
case "unreachable": return `store unreachable (${r.error ?? "probe failed"}), durability UNVERIFIED`;
}
}

/**
* Run checkWritePath() against a deadline and record the verdict.
*
* Resolves by the deadline at the latest. If the check finishes afterwards,
* its real verdict replaces "timed_out" (marked late) — the store answered
* slowly, and the latest word on it is the one health should show.
*/
export async function checkWritePathWithTimeout(
timeoutMs: number = WRITE_PATH_TIMEOUT_MS,
check: () => Promise<WritePathResult> = checkWritePath
): Promise<WritePathVerdict> {
const started = Date.now();
let timedOut = false;
let timer: ReturnType<typeof setTimeout> | undefined;

const record = (r: WritePathResult, late = false): WritePathVerdict => {
lastVerdict = {
...r,
summary: summarize(r, timeoutMs),
checked_at: new Date().toISOString(),
duration_ms: Date.now() - started,
...(late && { late: true }),
};
return lastVerdict;
};

const checked = check().then((r) => {
if (timedOut) {
const v = record(r, true);
console.error(`[gitmem] Write-path check finished late (${v.duration_ms} ms): ${v.summary}`);
return v;
}
return record(r);
});

const deadline = new Promise<WritePathVerdict>((resolve) => {
timer = setTimeout(() => {
timedOut = true;
const v = record({ ok: false, mode: "timed_out" });
console.error(
`\n\u26a0\ufe0f [gitmem] WRITE PATH: ${v.summary}.\n` +
" The Supabase store did not answer the write-path probe. Writes may or may not be landing;\n" +
" check connectivity to SUPABASE_URL. `health` shows the latest verdict.\n"
);
resolve(v);
}, timeoutMs);
timer.unref?.();
});

try {
return await Promise.race([checked, deadline]);
} finally {
if (timer) clearTimeout(timer);
}
}

export async function checkWritePath(): Promise<WritePathResult> {
Expand Down Expand Up @@ -65,6 +174,7 @@ export async function checkWritePath(): Promise<WritePathResult> {
const prefix = getTablePrefix();
const prefixSource = process.env.GITMEM_TABLE_PREFIX ? "GITMEM_TABLE_PREFIX" : "default";
const missing: string[] = [];
const probeErrors: string[] = [];

for (const base of ["learnings", "decisions"]) {
const table = getTableName(base);
Expand All @@ -73,7 +183,9 @@ export async function checkWritePath(): Promise<WritePathResult> {
} catch (err) {
const msg = err instanceof Error ? err.message : String(err);
if (SCHEMA_MISS.test(msg)) missing.push(table);
// Transient network/auth errors are out of scope for this check.
// GIT-102: a network/auth error used to be ignored here, so a store
// that refused every connection was reported as "Write-path OK".
else probeErrors.push(`${table}: ${msg}`);
}
}

Expand All @@ -89,6 +201,15 @@ export async function checkWritePath(): Promise<WritePathResult> {
return { ok: false, mode: "missing_tables", missing };
}

if (probeErrors.length > 0) {
const error = probeErrors.join("; ").slice(0, 300);
console.error(
"\n\u26a0\ufe0f [gitmem] WRITE PATH: the Supabase store could not be probed — durability UNVERIFIED.\n" +
` ${error}\n`
);
return { ok: false, mode: "unreachable", error };
}

console.error(
`[gitmem] Write-path OK (tier ${getTier()}, prefix "${prefix}", learnings/decisions present).`
);
Expand Down
59 changes: 53 additions & 6 deletions src/tools/session-close.ts
Original file line number Diff line number Diff line change
Expand Up @@ -329,7 +329,7 @@ async function sessionCloseFree(
clearCurrentSession();
const latencyMs = timer.stop();
const perfData = buildPerformanceData("session_close", latencyMs, 0);
const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`]);
const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], undefined, undefined, false);

return {
success: false,
Expand Down Expand Up @@ -363,6 +363,8 @@ function formatCloseDisplay(
errors?: string[],
transcriptStatus?: TranscriptStatus,
blindspotSnippet?: string | null,
/** GIT-102: whether the session content itself landed; defaults to `success`. */
sessionStored: boolean = success,
): string {
const lines: string[] = [];

Expand Down Expand Up @@ -434,15 +436,55 @@ function formatCloseDisplay(
}

// Write health — only on failure
const healthReport = getEffectTracker().getHealthReport();
if (healthReport.overall.failed > 0) {
const warnings = formatWriteWarnings(getEffectTracker().getHealthReport(), sessionStored);
if (warnings.length > 0) {
lines.push("");
lines.push(`${STATUS.warn} ${healthReport.overall.failed} write failure${healthReport.overall.failed > 1 ? "s" : ""}`);
for (const w of warnings) lines.push(`${STATUS.warn} ${w}`);
}

return wrapDisplay(lines.join("\n"));
}

/** GIT-102: what each tracked write path is, in words. */
const SUBSYSTEM_NAMES: Record<string, string> = {
triple_write: "knowledge graph (triples)",
embedding: "session embedding",
scar_usage: "scar usage records",
transcript: "transcript upload",
metrics: "query metrics",
relevance_update: "relevance feedback",
variant_generation: "scar variants",
cache_set: "local cache",
};

/**
* GIT-102: one self-describing line per failing write subsystem.
*
* The close used to print "WARN 3 write failures" and nothing else: not which
* writes, not why, and — the question every reader actually has — not whether
* the session itself was saved. Each line now names the subsystem, the cause
* (its most recent error), and states whether the session content was stored.
* That last clause is mandatory on every line, so no WARN can be read as
* "the close failed" when it didn't, or as harmless when it wasn't.
*/
export function formatWriteWarnings(
report: ReturnType<ReturnType<typeof getEffectTracker>["getHealthReport"]>,
sessionStored: boolean
): string[] {
const where = hasSupabase() ? "" : " (local)";
const stored = sessionStored ? `session content stored OK${where}` : "session content NOT stored";
const lines: string[] = [];
for (const [path, s] of Object.entries(report.byPath)) {
if (s.failed === 0) continue;
const name = SUBSYSTEM_NAMES[path] ?? path;
const cause = s.lastFailure?.error
? truncate(s.lastFailure.error.replace(/\s+/g, " ").trim(), 160)
: "no error message recorded";
lines.push(`${name}: ${s.failed} of ${s.attempted} write${s.attempted > 1 ? "s" : ""} failed — ${cause} · ${stored}`);
}
return lines;
}

/**
* GIT-21: Clean up all session files for a closed session.
* Unregisters from registry, deletes per-session directory, and removes legacy file.
Expand Down Expand Up @@ -932,6 +974,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;
// Whether the session row reached the durable store (GIT-102: the close
// display says so on every WARN; set only after the upsert resolves).
let sessionRowStored = false;
try {
if (fs.existsSync(payloadPath)) {
const filePayload = JSON.parse(fs.readFileSync(payloadPath, "utf-8")) as Partial<SessionCloseParams>;
Expand Down Expand Up @@ -1470,6 +1515,7 @@ export async function sessionClose(
supabase.directUpsert(sessionsTable, sessionRow),
blindspotPromise,
]);
sessionRowStored = true;

// Tracked fire-and-forget embedding generation + session update + thread detection
if (isEmbeddingAvailable()) {
Expand Down Expand Up @@ -1642,7 +1688,8 @@ export async function sessionClose(

const allErrors = [...persistErrors, ...coverageWarnings, ...validation.warnings];

const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet);
// The session row is upserted above; a partial persist is threads, not the session.
const display = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, !partialPersist, allErrors.length > 0 ? allErrors : undefined, transcriptStatus, blindspotSnippet, true);

return {
success: !partialPersist,
Expand All @@ -1660,7 +1707,7 @@ export async function sessionClose(
// Clear session state even on error (session is done either way)
clearCurrentSession();

const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet);
const errorDisplay = formatCloseDisplay(sessionId, closeCompliance, params, learningsCount, false, [`Failed to persist session: ${errorMessage}`], transcriptStatus, blindspotSnippet, sessionRowStored);

return {
success: false,
Expand Down
15 changes: 15 additions & 0 deletions tests/e2e/blank-supabase.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,12 @@ async function flow() {
await waitQuiet(netlog, { minMs: 2000, quietMs: 3000 });
const health = await step("health", { failure_limit: 20 });

// GIT-102: every WARN on the close names a subsystem and cause and says
// whether the session content was stored; health reports the write-path verdict.
const closeWarns = sc.replace(/\x1b\[[0-9;]*m/g, "").split("\n").filter((l) => /^WARN /.test(l));
const closeWarnsSelfDescribing = closeWarns.every((l) => /^WARN .+: \d+ of \d+ writes? failed — .+ · session content (stored OK|NOT stored)/.test(l));
const healthHasWritePath = /^Write path: /m.test(health);

// Did the close land? Is relevance readable from the store (GIT-109)?
const closedRows = sessionId
? await rest("GET", `gitmem_sessions?select=id,closing_reflection&id=eq.${sessionId}`, undefined, { Prefer: "" }).then((r) => r.json())
Expand Down Expand Up @@ -408,6 +414,9 @@ 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,
close_warns: closeWarns,
close_warns_self_describing: closeWarnsSelfDescribing,
health_has_write_path: healthHasWritePath,
relevance_readable: relevance.some((r) => scarIds.some((id) => (r.memories_applied || []).includes(id) && r.memory_relevance?.[id])),
relevance,
health_failed_total: failed,
Expand Down Expand Up @@ -520,6 +529,12 @@ 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.close_warns_self_describing) {
failureCheck.unexpected.push(`session_close WARN is not self-describing (GIT-102): ${result.close_warns.join(" | ")}`);
}
if (mode === "flow" && !result.health_has_write_path) {
failureCheck.unexpected.push("health does not report the write-path verdict (GIT-102)");
}
if (mode === "flow" && !result.session_close_persisted) {
failureCheck.unexpected.push(`session_close did not persist session ${result.session_id} (closing_reflection missing)`);
}
Expand Down
Loading
Loading