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
10 changes: 9 additions & 1 deletion schema/setup.sql
Original file line number Diff line number Diff line change
Expand Up @@ -404,7 +404,12 @@ BEGIN
updated_at = NOW()
FROM usage_stats us
WHERE l.id = us.scar_id
AND COALESCE(l.is_active, true) = true;
AND COALESCE(l.is_active, true) = true
-- Only touch rows whose multiplier actually changes. Rewriting unchanged
-- rows bumped updated_at on every session_start, which invalidates every
-- client's on-disk embedding cache (GIT-98) for no reason.
AND l.decay_multiplier IS DISTINCT FROM
GREATEST(0.1, 1.0 - (us.times_dismissed::FLOAT / us.times_surfaced::FLOAT) * 0.8);

GET DIAGNOSTICS v_updated = ROW_COUNT;

Expand Down Expand Up @@ -529,9 +534,12 @@ CREATE TABLE IF NOT EXISTS gitmem_license_activations (
ALTER TABLE gitmem_licenses ENABLE ROW LEVEL SECURITY;
ALTER TABLE gitmem_license_activations ENABLE ROW LEVEL SECURITY;

-- Drop-then-create so setup.sql can be re-applied (CREATE POLICY has no IF NOT EXISTS)
DROP POLICY IF EXISTS "Service role full access" ON gitmem_licenses;
CREATE POLICY "Service role full access" ON gitmem_licenses
FOR ALL USING (auth.role() = 'service_role');

DROP POLICY IF EXISTS "Service role full access" ON gitmem_license_activations;
CREATE POLICY "Service role full access" ON gitmem_license_activations
FOR ALL USING (auth.role() = 'service_role');

Expand Down
2 changes: 1 addition & 1 deletion src/services/analytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,7 +201,7 @@ export async function queryScarUsageByDateRange(
surfaced_at: `gte.${startDate}`,
};

return directQueryAll<ScarUsageRecord>("scar_usage", {
return directQueryAll<ScarUsageRecord>(getTableName("scar_usage"), {
select: "scar_id,scar_title,scar_severity,agent,reference_type,execution_successful,surfaced_at",
filters,
order: "surfaced_at.desc",
Expand Down
3 changes: 2 additions & 1 deletion src/services/behavioral-decay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
*/

import { isConfigured, safeInFilter } from "./supabase-client.js";
import { getTableName } from "./tier.js";

const SUPABASE_URL = process.env.SUPABASE_URL || "";
const SUPABASE_KEY = process.env.SUPABASE_SERVICE_ROLE_KEY || process.env.SUPABASE_KEY || "";
Expand Down Expand Up @@ -98,7 +99,7 @@ export async function fetchDismissalCounts(

try {
// Query scar_usage for the given scar IDs (last 90 days)
const url = new URL(`${SUPABASE_REST_URL}/scar_usage`);
const url = new URL(`${SUPABASE_REST_URL}/${getTableName("scar_usage")}`);
url.searchParams.set("select", "scar_id,reference_type");
url.searchParams.set("scar_id", safeInFilter(scarIds));
url.searchParams.set("surfaced_at", `gte.${new Date(Date.now() - 90 * 86400000).toISOString()}`);
Expand Down
2 changes: 1 addition & 1 deletion src/tools/record-scar-usage-batch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ export async function recordScarUsageBatch(
// Insert all usage records in parallel
const insertPromises = usageRecords.map(async (record) => {
const result = await supabase.directUpsert<{ id: string }>(
"scar_usage",
getTableName("scar_usage"),
record
);
return result?.id || null;
Expand Down
4 changes: 2 additions & 2 deletions src/tools/record-scar-usage.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
import { v4 as uuidv4 } from "uuid";
import { wrapDisplay } from "../services/display-protocol.js";
import * as supabase from "../services/supabase-client.js";
import { hasSupabase } from "../services/tier.js";
import { hasSupabase, getTableName } from "../services/tier.js";
import { getStorage } from "../services/storage.js";
import { detectAgent } from "../services/agent-detection.js";
import { getCurrentSession } from "../services/session-state.js";
Expand Down Expand Up @@ -61,7 +61,7 @@ export async function recordScarUsage(

if (hasSupabase()) {
const upsertStart = Date.now();
await supabase.directUpsert("scar_usage", usageData);
await supabase.directUpsert(getTableName("scar_usage"), usageData);
breakdown.upsert = {
latency_ms: Date.now() - upsertStart,
source: "supabase",
Expand Down
57 changes: 57 additions & 0 deletions tests/unit/schema-setup-reapply.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
/**
* schema/setup.sql must be safe to apply twice (GIT-84).
*
* Customers re-run setup.sql to pick up schema changes. CREATE POLICY and
* CREATE TRIGGER have no IF NOT EXISTS, so each needs a DROP ... IF EXISTS
* first; one missing pair (gitmem_licenses) made the second run fail at
* statement 72 on a real Supabase.
*
* Also pins the GIT-84 guard on refresh_scar_behavioral_scores(): rows whose
* decay_multiplier would not change must not be rewritten, because the rewrite
* bumps updated_at and invalidates every client's GIT-98 disk cache.
*/

import { describe, it, expect } from "vitest";
import { readFileSync } from "fs";
import { join } from "path";

const SQL = readFileSync(join(__dirname, "../../schema/setup.sql"), "utf-8")
// strip -- comments so commented-out statements don't count
.replace(/--[^\n]*/g, "");

const norm = (s: string) => s.replace(/\s+/g, " ").trim().toLowerCase();

describe("schema/setup.sql re-apply safety (GIT-84)", () => {
it("drops every policy before creating it", () => {
const created = [...SQL.matchAll(/CREATE POLICY\s+("[^"]+")\s+ON\s+([\w.]+)/gi)].map((m) => `${m[1]} ON ${m[2]}`);
expect(created.length).toBeGreaterThan(0);
const missing = created.filter((p) => {
const [name, table] = p.split(" ON ");
const drop = new RegExp(`DROP POLICY IF EXISTS\\s+${name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\s+ON\\s+${table}\\s*;`, "i");
const dropAt = SQL.search(drop);
const createAt = SQL.search(new RegExp(`CREATE POLICY\\s+${name.replace(/[.*+?^${}()|[\]\\]/g, "\\$&")}\\s+ON\\s+${table}\\b`, "i"));
return dropAt < 0 || dropAt > createAt;
});
expect(missing).toEqual([]);
});

it("drops every trigger before creating it", () => {
const created = [...SQL.matchAll(/CREATE TRIGGER\s+(\w+)[\s\S]*?\bON\s+([\w.]+)/gi)].map((m) => [m[1], m[2]]);
expect(created.length).toBeGreaterThan(0);
const missing = created.filter(([name, table]) => !new RegExp(`DROP TRIGGER IF EXISTS\\s+${name}\\s+ON\\s+${table}\\s*;`, "i").test(SQL));
expect(missing).toEqual([]);
});

it("creates tables and indexes with IF NOT EXISTS", () => {
const bare = [...SQL.matchAll(/CREATE\s+(?:UNIQUE\s+)?(TABLE|INDEX)\s+(?!IF NOT EXISTS)(\w+)/gi)].map((m) => `${m[1]} ${m[2]}`);
expect(bare).toEqual([]);
});

it("refresh_scar_behavioral_scores() only rewrites rows whose multiplier changes", () => {
const fn = SQL.slice(SQL.search(/CREATE OR REPLACE FUNCTION refresh_scar_behavioral_scores/i));
const update = norm(fn.slice(0, fn.indexOf("GET DIAGNOSTICS")));
const expr = "greatest(0.1, 1.0 - (us.times_dismissed::float / us.times_surfaced::float) * 0.8)";
expect(update).toContain(`set decay_multiplier = ${expr}`);
expect(update).toContain(`and l.decay_multiplier is distinct from ${expr}`);
});
});
115 changes: 115 additions & 0 deletions tests/unit/table-name-literals.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,115 @@
/**
* Guard: Supabase table names must go through getTableName() (GIT-84).
*
* The user schema is prefixed (gitmem_ by default, GITMEM_TABLE_PREFIX to
* override). A bare literal like "scar_usage" passed straight to PostgREST
* silently 404s on every customer project — that is how scar usage was lost.
*
* Checked:
* - directUpsert / directQuery / directQueryAll / directPatch whose table
* argument is a string literal
* - `table: "..."` option objects (listRecords and friends)
* - REST URLs built as `${SUPABASE_REST_URL}/name` or `/rest/v1/name`
* The storage layer (getStorage().upsert/query/get) takes a COLLECTION name
* and applies the prefix itself, so there the rule is reversed: passing a
* prefixed name or getTableName() would double-prefix.
*
* Known literals awaiting a ruling are allowlisted below, each with a reason.
* Removing an entry is the point — the list should only shrink.
*/

import { describe, it, expect } from "vitest";
import { readFileSync, readdirSync, statSync } from "fs";
import { join, relative } from "path";

const SRC = join(__dirname, "../../src");

function collectTsFiles(dir: string): string[] {
const out: string[] = [];
for (const entry of readdirSync(dir)) {
const full = join(dir, entry);
if (statSync(full).isDirectory()) out.push(...collectTsFiles(full));
else if (entry.endsWith(".ts") && !entry.endsWith(".test.ts") && !entry.endsWith(".d.ts")) out.push(full);
}
return out;
}

/**
* Literal table names that setup.sql creates under exactly that name (or that
* live outside the user schema). Changing the code alone would break them, so
* they stay literal until ruled on. Keyed by table name.
*/
const ALLOWED_LITERALS: Record<string, string> = {
gitmem_query_metrics: "setup.sql creates it with a hard-coded gitmem_ prefix — ruling needed (GIT-84 sweep)",
knowledge_triples: "setup.sql creates it unprefixed — ruling needed (GIT-84 sweep)",
scar_enforcement_variants: "setup.sql creates it unprefixed; column drift tracked in GIT-106",
variant_assignments: "not created by setup.sql at all — ruling needed (GIT-84 sweep)",
variant_performance_metrics: "not created by setup.sql at all — ruling needed (GIT-84 sweep)",
community_feedback: "gitmem's own feedback endpoint (feedback-remote.ts), not the user schema",
};

/** Files that build REST URLs for diagnostics with a literal prefix — ruling needed (GIT-84 sweep). */
const ALLOWED_URL_FILES = new Set(["src/commands/check.ts"]);

const DIRECT_CALL = /\b(directUpsert|directQuery|directQueryAll|directPatch)\s*(?:<(?:[^<>]|<[^<>]*>)*>)?\s*\(\s*(["'`])([A-Za-z_][\w]*)\2/g;
const TABLE_OPTION = /\btable:\s*(["'`])([A-Za-z_][\w]*)\1/g;
const REST_URL = /(?:SUPABASE_REST_URL\}|\/rest\/v1)\/([A-Za-z_][\w]*)/g;
const STORAGE_CALL = /\b(?:storage|getStorage\(\))\s*\.\s*(upsert|query|get)\s*(?:<(?:[^<>]|<[^<>]*>)*>)?\s*\(\s*([^,)]+)/g;

interface Finding { file: string; line: number; text: string }

function scan(): { bare: Finding[]; doublePrefixed: Finding[]; allowlistedSeen: Set<string> } {
const bare: Finding[] = [];
const doublePrefixed: Finding[] = [];
const allowlistedSeen = new Set<string>();

for (const full of collectTsFiles(SRC)) {
const file = relative(join(SRC, ".."), full);
const src = readFileSync(full, "utf-8");
const lineOf = (i: number) => src.slice(0, i).split("\n").length;
const flag = (name: string, i: number, text: string) => {
if (name in ALLOWED_LITERALS) { allowlistedSeen.add(name); return; }
bare.push({ file, line: lineOf(i), text });
};

for (const m of src.matchAll(DIRECT_CALL)) flag(m[3], m.index!, `${m[1]}("${m[3]}")`);
for (const m of src.matchAll(TABLE_OPTION)) flag(m[2], m.index!, `table: "${m[2]}"`);
for (const m of src.matchAll(REST_URL)) {
if (m[1] === "rpc") continue; // RPC function names are not tables
if (ALLOWED_URL_FILES.has(file)) continue;
flag(m[1], m.index!, `REST /${m[1]}`);
}
for (const m of src.matchAll(STORAGE_CALL)) {
const arg = m[2].trim();
if (/getTableName\s*\(/.test(arg) || /^["'`]gitmem_/.test(arg)) {
doublePrefixed.push({ file, line: lineOf(m.index!), text: `storage.${m[1]}(${arg})` });
}
}
}
return { bare, doublePrefixed, allowlistedSeen };
}

describe("Supabase table names go through getTableName() (GIT-84)", () => {
const { bare, doublePrefixed, allowlistedSeen } = scan();
const fmt = (fs: Finding[]) => fs.map((f) => `${f.file}:${f.line} ${f.text}`).join("\n");

it("passes no bare table-name literal to directUpsert/directQuery/directQueryAll/directPatch, table: options or REST URLs", () => {
expect(fmt(bare), "wrap these in getTableName(<base name>)").toBe("");
});

it("never hands the storage layer an already-prefixed name (it prefixes collections itself)", () => {
expect(fmt(doublePrefixed), "pass the bare collection name to storage").toBe("");
});

it("keeps the allowlist honest: every entry is still in use", () => {
const stale = Object.keys(ALLOWED_LITERALS).filter((name) => !allowlistedSeen.has(name));
expect(stale, "remove allowlist entries that no longer occur").toEqual([]);
});

it("would catch the GIT-84 bug (self-test on a synthetic source)", () => {
const sample = `await supabase.directUpsert<{ id: string }>(\n "scar_usage",\n record\n);`;
const hits = [...sample.matchAll(DIRECT_CALL)].map((m) => m[3]);
expect(hits).toEqual(["scar_usage"]);
expect([...`new URL(\`\${SUPABASE_REST_URL}/scar_usage\`)`.matchAll(REST_URL)].map((m) => m[1])).toEqual(["scar_usage"]);
});
});
Loading