diff --git a/.claude/hooks/_lib/rules-git.mjs b/.claude/hooks/_lib/rules-git.mjs index acd698e999..c0bc0adf94 100644 --- a/.claude/hooks/_lib/rules-git.mjs +++ b/.claude/hooks/_lib/rules-git.mjs @@ -133,6 +133,31 @@ export function checkGitCommand(command, { resolveHeadBranch, resolveRemoteUrl, return null } +export function checkGitWorktreeRemove(command) { + if (typeof command !== "string" || !/\bgit\b/.test(command)) return null + // A heredoc/commit-message body that merely NAMES the flag is data, not a + // command — strip bodies before matching, same as checkGitCommand. + const scannable = stripHeredocBodies(command) + // Judge each segment on its own target, mirroring checkGitCommand: the force + // flag must belong to the SAME segment as `worktree remove`, so a later + // `&& npm test -- --force` cannot false-block a plain `git worktree remove`. + for (const segment of scannable.split(/[&|;\n]/)) { + if (!/\bgit\b[\s\S]*\bworktree\s+remove\b/.test(segment)) continue + if (!/(? 0) depth-- + continue + } + if (depth === 0 && contents.startsWith("throw", i) && !/\w/.test(contents[i - 1] || "") && !/\w/.test(contents[i + 5] || "")) { + lines.push(line) + i += 4 + } + } + return lines +} + +export function checkMobileSupabaseLazy(filePath, contents) { + const n = norm(filePath) + if (!/\/apps\/mobile\/.*supabase\.ts$/.test(n)) return null + if (typeof contents !== "string") return null + + const findings = [] + for (const lineNumber of moduleScopeThrowLines(contents)) { + findings.push(`${filePath}:${lineNumber} — module-scope throw`) + } + // A direct top-level assignment `const supabase[: Type] = createClient(` is + // eager init; the type annotation is optional (that is how the real file + // declares its state). The lazy `() => createClient(` arrow has `() =>` + // between the `=` and the call, so it does not match — and stays allowed. + for (const match of contents.matchAll(/^(?:export\s+)?const\s+\w+\s*(?::\s*[\w.<>[\]|& ]+?\s*)?=\s*createClient\s*\(/gm)) { + const lineNumber = contents.slice(0, match.index).split("\n").length + findings.push(`${filePath}:${lineNumber} — top-level createClient() init`) + } + if (findings.length === 0) return null + return { + block: true, + message: + `Mobile Supabase must stay lazy in ${filePath}:\n` + + findings.map((f) => ` - ${f}`).join("\n") + + `\n\nKeep the client lazy behind a getSupabaseClient() accessor — never init or throw at module scope.\n` + + `A module-eval throw or top-level createClient() runs during the app's first import and crashes to a grey\n` + + `screen at launch before any error boundary mounts (#172/#174).\n`, + } +} + +export function checkEfMigrationRawIndex(filePath, contents) { + const n = norm(filePath) + if (!/\/orbit-api\/.*Migrations\/.*\.cs$/.test(n)) return null + if (typeof contents !== "string") return null + + const findings = [] + const marker = /migrationBuilder\.Sql\s*\(/g + let call + while ((call = marker.exec(contents))) { + let index = call.index + call[0].length + let depth = 1 + const start = index + while (index < contents.length && depth > 0) { + const ch = contents[index] + if (ch === "(") depth++ + else if (ch === ")") depth-- + index++ + } + const sql = contents.slice(start, index - 1) + const lineNumber = contents.slice(0, call.index).split("\n").length + // Check each `;`-separated statement on its own — one statement's IF [NOT] + // EXISTS clause must not mask a sibling statement in the same batched Sql() + // call that lacks its own. + for (const statement of sql.split(";")) { + if (/\bCREATE\s+(?:UNIQUE\s+)?INDEX\b/i.test(statement) && !/\bIF\s+NOT\s+EXISTS\b/i.test(statement)) { + findings.push(`${filePath}:${lineNumber} — raw CREATE INDEX without IF NOT EXISTS`) + } + if (/\bDROP\s+INDEX\b/i.test(statement) && !/\bIF\s+EXISTS\b/i.test(statement)) { + findings.push(`${filePath}:${lineNumber} — raw DROP INDEX without IF EXISTS`) + } + } + } + if (findings.length === 0) return null + return { + block: true, + message: + `Non-idempotent raw index SQL in ${filePath}:\n` + + findings.map((f) => ` - ${f}`).join("\n") + + `\n\nEF applies migrations at startup on Render; a raw CREATE INDEX for an index that already exists throws\n` + + `Postgres 42P07 and fails the deploy. Use CREATE INDEX IF NOT EXISTS / DROP INDEX IF EXISTS (mirrors the\n` + + `Guard-Migrations CI). migrationBuilder.CreateIndex(...) is already idempotent-safe — this only flags raw Sql().\n`, + } +} + export function checkCsharpFluentConfig(filePath, contents) { const n = norm(filePath) if (!/\/orbit-api\/src\/Orbit\.Infrastructure\/Persistence\/OrbitDbContext\.cs$/.test(n)) return null diff --git a/.claude/hooks/forbid-ef-migration-raw-index.mjs b/.claude/hooks/forbid-ef-migration-raw-index.mjs new file mode 100644 index 0000000000..046aa18016 --- /dev/null +++ b/.claude/hooks/forbid-ef-migration-raw-index.mjs @@ -0,0 +1,22 @@ +#!/usr/bin/env node +// PostToolUse adapter for the EF raw-index idempotency guard. Thin: logic in +// _lib/rules-source.mjs (shared with the .opencode/plugin equivalent). Reads the +// edited file. Exits 0 or 2 + stderr. Any error exits 0. + +import { readFileSync, existsSync } from "node:fs" +import { readStdinJson, filePathFrom } from "./_lib/io.mjs" +import { checkEfMigrationRawIndex } from "./_lib/rules-source.mjs" + +try { + const input = readStdinJson() + const filePath = filePathFrom(input) + if (!filePath || !existsSync(filePath)) process.exit(0) + const verdict = checkEfMigrationRawIndex(filePath, readFileSync(filePath, "utf8")) + if (verdict?.block) { + process.stderr.write(verdict.message) + process.exit(2) + } + process.exit(0) +} catch { + process.exit(0) +} diff --git a/.claude/hooks/forbid-mobile-supabase-eager.mjs b/.claude/hooks/forbid-mobile-supabase-eager.mjs new file mode 100644 index 0000000000..a0b6dd7d76 --- /dev/null +++ b/.claude/hooks/forbid-mobile-supabase-eager.mjs @@ -0,0 +1,22 @@ +#!/usr/bin/env node +// PostToolUse adapter for the mobile Supabase lazy-client guard. Thin: logic in +// _lib/rules-source.mjs (shared with the .opencode/plugin equivalent). Reads the +// edited file. Exits 0 or 2 + stderr. Any error exits 0. + +import { readFileSync, existsSync } from "node:fs" +import { readStdinJson, filePathFrom } from "./_lib/io.mjs" +import { checkMobileSupabaseLazy } from "./_lib/rules-source.mjs" + +try { + const input = readStdinJson() + const filePath = filePathFrom(input) + if (!filePath || !existsSync(filePath)) process.exit(0) + const verdict = checkMobileSupabaseLazy(filePath, readFileSync(filePath, "utf8")) + if (verdict?.block) { + process.stderr.write(verdict.message) + process.exit(2) + } + process.exit(0) +} catch { + process.exit(0) +} diff --git a/.claude/hooks/git-guardrails.mjs b/.claude/hooks/git-guardrails.mjs index 72ef35e219..0e21fedb22 100644 --- a/.claude/hooks/git-guardrails.mjs +++ b/.claude/hooks/git-guardrails.mjs @@ -6,7 +6,7 @@ import { execFileSync } from "node:child_process" import { readStdinJson } from "./_lib/io.mjs" -import { checkGitCommand } from "./_lib/rules-git.mjs" +import { checkGitCommand, checkGitWorktreeRemove } from "./_lib/rules-git.mjs" try { const input = readStdinJson() @@ -25,7 +25,8 @@ try { stdio: ["ignore", "pipe", "ignore"], }).trim() - const verdict = checkGitCommand(command, { resolveHeadBranch, resolveRemoteUrl, cwd: input?.cwd || process.cwd() }) + const verdict = + checkGitCommand(command, { resolveHeadBranch, resolveRemoteUrl, cwd: input?.cwd || process.cwd() }) ?? checkGitWorktreeRemove(command) if (verdict?.block) { process.stderr.write(verdict.message) process.exit(2) diff --git a/.claude/hooks/test-hooks.mjs b/.claude/hooks/test-hooks.mjs index 1c74b2a773..fcaf68d1c1 100644 --- a/.claude/hooks/test-hooks.mjs +++ b/.claude/hooks/test-hooks.mjs @@ -13,10 +13,18 @@ import { tmpdir } from "node:os" import { dirname, join } from "node:path" import { fileURLToPath, pathToFileURL } from "node:url" -import { checkGitCommand, checkNpmExpoPin } from "./_lib/rules-git.mjs" +import { checkGitCommand, checkGitWorktreeRemove, checkNpmExpoPin } from "./_lib/rules-git.mjs" import { checkShellAllowlist, PRIMER_SHELL_ALLOWLIST } from "./_lib/rules-shell-allowlist.mjs" import { checkEmDashes, checkBrandColors } from "./_lib/rules-content.mjs" -import { checkTsAntipatterns, checkNewTodos, checkCsharpAuthz, checkCsharpTimezone, checkCsharpFluentConfig } from "./_lib/rules-source.mjs" +import { + checkTsAntipatterns, + checkMobileSupabaseLazy, + checkEfMigrationRawIndex, + checkNewTodos, + checkCsharpAuthz, + checkCsharpTimezone, + checkCsharpFluentConfig, +} from "./_lib/rules-source.mjs" import { classifyScope, parityMessages } from "./_lib/rules-parity.mjs" const hooksDir = dirname(fileURLToPath(import.meta.url)) @@ -87,6 +95,31 @@ T( checkGitCommand(`gh pr create --body "$(cat <<'PRBODY'\nthe bash <` between `=` and createClient — must NOT false-block. +T( + "supabase: lazy arrow accessor allows", + checkMobileSupabaseLazy("/x/apps/mobile/lib/supabase.ts", "export const getSupabaseClient = () => createClient(url, key)"), + null, +) +T("supabase: off-path skipped", checkMobileSupabaseLazy("/x/apps/web/lib/supabase.ts", 'throw new Error("no env")'), null) + +// EF raw index SQL must be idempotent — a bare CREATE INDEX throws 42P07 at startup deploy. +T( + "ef-index: raw CREATE UNIQUE INDEX blocks", + !!checkEfMigrationRawIndex("/x/orbit-api/src/Orbit.Infrastructure/Migrations/20260101_Add.cs", 'migrationBuilder.Sql("CREATE UNIQUE INDEX ix_foo ON foo (bar)");')?.block, + true, +) +T( + "ef-index: raw DROP INDEX without IF EXISTS blocks", + !!checkEfMigrationRawIndex("/x/orbit-api/src/Orbit.Infrastructure/Migrations/20260101_Add.cs", 'migrationBuilder.Sql("DROP INDEX ix_foo");')?.block, + true, +) +T( + "ef-index: IF NOT EXISTS form allows", + checkEfMigrationRawIndex("/x/orbit-api/src/Orbit.Infrastructure/Migrations/20260101_Add.cs", 'migrationBuilder.Sql("CREATE INDEX IF NOT EXISTS ix_foo ON foo (bar)");'), + null, +) +T( + "ef-index: CreateIndex API call allows", + checkEfMigrationRawIndex("/x/orbit-api/src/Orbit.Infrastructure/Migrations/20260101_Add.cs", 'migrationBuilder.CreateIndex(name: "ix_foo", table: "foo", column: "bar");'), + null, +) +T("ef-index: off-path skipped", checkEfMigrationRawIndex("/x/orbit-api/src/Orbit.Application/Foo.cs", 'migrationBuilder.Sql("CREATE INDEX ix_foo ON foo (bar)");'), null) +// Multi-statement Sql(): a sibling statement's IF NOT EXISTS must not mask another +// statement in the same call that lacks it (PR #556 review, per-statement check). +T( + "ef-index: batched Sql with one non-idempotent statement blocks", + !!checkEfMigrationRawIndex( + "/x/orbit-api/src/Orbit.Infrastructure/Migrations/20260101_Add.cs", + 'migrationBuilder.Sql("CREATE INDEX IF NOT EXISTS ix_b ON foo (b); CREATE INDEX ix_a ON foo (a);");', + )?.block, + true, +) +T( + "ef-index: batched Sql with all idempotent statements allows", + checkEfMigrationRawIndex( + "/x/orbit-api/src/Orbit.Infrastructure/Migrations/20260101_Add.cs", + 'migrationBuilder.Sql("CREATE INDEX IF NOT EXISTS ix_b ON foo (b); CREATE INDEX IF NOT EXISTS ix_a ON foo (a);");', + ), + null, +) T("todos: marker blocks", !!checkNewTodos("/x/apps/web/a.ts", `// ${MARK}: fix`)?.block, true) T("todos: tracked ref allows", checkNewTodos("/x/apps/web/a.ts", `// ${MARK} #123: later`), null) T("csharp-authz: missing blocks", !!checkCsharpAuthz("/x/orbit-api/src/Orbit.Api/Controllers/FooController.cs", "public class FooController {}")?.block, true) @@ -172,6 +266,8 @@ function runHook(file, payload) { T("cc git-guardrails: push main -> 2", runHook("git-guardrails.mjs", { tool_name: "Bash", tool_input: { command: "git push origin main" } }), 2) T("cc git-guardrails: feature -> 0", runHook("git-guardrails.mjs", { tool_name: "Bash", tool_input: { command: "git push origin feature/x" } }), 0) +T("cc git-guardrails: worktree remove --force -> 2", runHook("git-guardrails.mjs", { tool_name: "Bash", tool_input: { command: "git worktree remove --force .claude/worktrees/x" } }), 2) +T("cc git-guardrails: worktree remove (no force) -> 0", runHook("git-guardrails.mjs", { tool_name: "Bash", tool_input: { command: "git worktree remove .claude/worktrees/x" } }), 0) T("cc expo-pin: update -> 2", runHook("forbid-expo-pin-bump.mjs", { tool_name: "Bash", tool_input: { command: "npm update" } }), 2) T("cc em-dashes: locale dash -> 2", runHook("forbid-em-dashes.mjs", { tool_name: "Write", tool_input: { file_path: join(root, "packages/shared/src/i18n/en.json"), content: `{"k":"a ${EM} b"}` } }), 2) const tsBad = write("apps/web/bad.ts", "console.log(1)\n") @@ -184,6 +280,14 @@ const ctrlBad = write("orbit-api/src/Orbit.Api/Controllers/FooController.cs", "p T("cc csharp-authz: missing -> 2", runHook("csharp-authz.mjs", { tool_name: "Write", tool_input: { file_path: ctrlBad } }), 2) const tzBad = write("orbit-api/src/Orbit.Application/Foo.cs", "var x = DateTime.UtcNow;\n") T("cc csharp-tz: UtcNow -> 2", runHook("csharp-tz.mjs", { tool_name: "Write", tool_input: { file_path: tzBad } }), 2) +const supabaseBad = write("apps/mobile/lib/supabase.ts", 'throw new Error("no env")\n') +const supabaseGood = write("apps/mobile/lib/supabase.good.ts", "export function getSupabaseClient() {\n return createClient(url, key)\n}\n") +T("cc supabase-eager: module throw -> 2", runHook("forbid-mobile-supabase-eager.mjs", { tool_name: "Write", tool_input: { file_path: supabaseBad } }), 2) +T("cc supabase-eager: lazy accessor -> 0", runHook("forbid-mobile-supabase-eager.mjs", { tool_name: "Write", tool_input: { file_path: supabaseGood } }), 0) +const efBad = write("orbit-api/src/Orbit.Infrastructure/Migrations/20260101_Add.cs", 'migrationBuilder.Sql("CREATE UNIQUE INDEX ix_foo ON foo (bar)");\n') +const efGood = write("orbit-api/src/Orbit.Infrastructure/Migrations/20260102_Add.cs", 'migrationBuilder.Sql("CREATE INDEX IF NOT EXISTS ix_foo ON foo (bar)");\n') +T("cc ef-index: raw CREATE INDEX -> 2", runHook("forbid-ef-migration-raw-index.mjs", { tool_name: "Write", tool_input: { file_path: efBad } }), 2) +T("cc ef-index: IF NOT EXISTS -> 0", runHook("forbid-ef-migration-raw-index.mjs", { tool_name: "Write", tool_input: { file_path: efGood } }), 0) // primer's allowlist is wired in .claude/agents/primer.md's own frontmatter, not // settings.json, so this asserts the file the agent points at really behaves. @@ -235,10 +339,15 @@ const after = async (tool, args) => { T("oc before: push main throws", await before("bash", { command: "git push origin main" }), true) T("oc before: feature allows", await before("bash", { command: "git push origin feature/x" }), false) T("oc before: npm update throws", await before("bash", { command: "npm update" }), true) +T("oc before: worktree remove --force throws", await before("bash", { command: "git worktree remove --force .claude/worktrees/x" }), true) +T("oc before: worktree remove (no force) allows", await before("bash", { command: "git worktree remove .claude/worktrees/x" }), false) T("oc before: em dash throws", await before("write", { filePath: join(root, "packages/shared/src/i18n/en.json"), content: `a ${EM} b` }), true) T("oc after: console.log throws", await after("edit", { filePath: tsBad, newString: "x" }), true) T("oc after: clean allows", await after("edit", { filePath: tsGood, newString: "x" }), false) T("oc after: csharp authz throws", await after("write", { filePath: ctrlBad, content: "x" }), true) +T("oc after: supabase eager throws", await after("write", { filePath: supabaseBad, content: "x" }), true) +T("oc after: supabase lazy allows", await after("write", { filePath: supabaseGood, content: "x" }), false) +T("oc after: ef raw index throws", await after("write", { filePath: efBad, content: "x" }), true) T("oc exposes session.idle guard", typeof hooks["event"], "function") // --------------------------------------------------------------------------- diff --git a/.claude/settings.json b/.claude/settings.json index be063c9837..58d1127630 100644 --- a/.claude/settings.json +++ b/.claude/settings.json @@ -14,6 +14,8 @@ "matcher": "Edit|Write|MultiEdit", "hooks": [ { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/forbid-ts-antipatterns.mjs\"" }, + { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/forbid-mobile-supabase-eager.mjs\"" }, + { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/forbid-ef-migration-raw-index.mjs\"" }, { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/forbid-em-dashes.mjs\"" }, { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/forbid-hardcoded-brand-color.mjs\"" }, { "type": "command", "command": "node \"$CLAUDE_PROJECT_DIR/.claude/hooks/flag-new-todos.mjs\"" }, diff --git a/.opencode/plugin/orbit-guardrails.js b/.opencode/plugin/orbit-guardrails.js index 317be797aa..b1f6e960b9 100644 --- a/.opencode/plugin/orbit-guardrails.js +++ b/.opencode/plugin/orbit-guardrails.js @@ -74,7 +74,10 @@ export default async ({ directory, worktree } = {}) => { const tool = input?.tool const args = output?.args || {} if (tool === "bash" && typeof args.command === "string") { - throwBlocks([git.checkGitCommand(args.command, { resolveHeadBranch, resolveRemoteUrl, cwd: dir }), git.checkNpmExpoPin(args.command)], "command") + throwBlocks( + [git.checkGitCommand(args.command, { resolveHeadBranch, resolveRemoteUrl, cwd: dir }), git.checkGitWorktreeRemove(args.command), git.checkNpmExpoPin(args.command)], + "command", + ) } else if (tool === "edit" || tool === "write") { const { filePath, addedText } = io.fromOpenCodeEdit(args) if (filePath && addedText) { @@ -98,6 +101,8 @@ export default async ({ directory, worktree } = {}) => { throwBlocks( [ source.checkTsAntipatterns(filePath, contents), + source.checkMobileSupabaseLazy(filePath, contents), + source.checkEfMigrationRawIndex(filePath, contents), source.checkNewTodos(filePath, contents), source.checkCsharpAuthz(filePath, contents), source.checkCsharpTimezone(filePath, contents),