diff --git a/.github/scripts/check_migration_idempotency.py b/.github/scripts/check_migration_idempotency.py new file mode 100644 index 00000000..5c6606b6 --- /dev/null +++ b/.github/scripts/check_migration_idempotency.py @@ -0,0 +1,89 @@ +#!/usr/bin/env python3 +"""Fail CI on non-idempotent raw index SQL in EF migrations. + +EF applies migrations at startup on Render; a raw CREATE INDEX for an index that +already exists throws Postgres 42P07 and fails the deploy. Every raw CREATE INDEX +inside migrationBuilder.Sql(...) must carry IF NOT EXISTS, and every raw DROP +INDEX must carry IF EXISTS. The fluent migrationBuilder.CreateIndex(...) / +DropIndex(...) API is already idempotent-safe and is not flagged (its method +names carry no whitespace, so they never match the raw-SQL patterns). + +Mirrors checkEfMigrationRawIndex in the orbit-ui-mobile consumer repo +(.claude/hooks/_lib/rules-source.mjs): only the balanced-paren body of each +migrationBuilder.Sql(...) call is scanned, split into ;-separated statements so +one statement's IF [NOT] EXISTS clause cannot mask a sibling that lacks its own. + + check_migration_idempotency.py [ ...] +""" +import os +import re +import sys + +SQL_CALL = re.compile(r"migrationBuilder\.Sql\s*\(") +CREATE_INDEX = re.compile(r"\bCREATE\s+(?:UNIQUE\s+)?INDEX\b", re.IGNORECASE) +IF_NOT_EXISTS = re.compile(r"\bIF\s+NOT\s+EXISTS\b", re.IGNORECASE) +DROP_INDEX = re.compile(r"\bDROP\s+INDEX\b", re.IGNORECASE) +IF_EXISTS = re.compile(r"\bIF\s+EXISTS\b", re.IGNORECASE) + + +def find_violations(path: str, contents: str) -> list[str]: + """Return one message per non-idempotent raw index statement in the file.""" + findings: list[str] = [] + for call in SQL_CALL.finditer(contents): + index = call.end() + depth = 1 + start = index + while index < len(contents) and depth > 0: + char = contents[index] + if char == "(": + depth += 1 + elif char == ")": + depth -= 1 + index += 1 + sql = contents[start : index - 1] + line_number = contents.count("\n", 0, call.start()) + 1 + for statement in sql.split(";"): + if CREATE_INDEX.search(statement) and not IF_NOT_EXISTS.search(statement): + findings.append(f"{path}:{line_number} raw CREATE INDEX without IF NOT EXISTS") + if DROP_INDEX.search(statement) and not IF_EXISTS.search(statement): + findings.append(f"{path}:{line_number} raw DROP INDEX without IF EXISTS") + return findings + + +def _confine(candidate: str, base: str) -> str: + """Resolve candidate and confirm it stays within base, rejecting path traversal.""" + resolved = os.path.realpath(candidate) + if resolved != base and not resolved.startswith(base + os.sep): + raise ValueError(f"Refusing path outside {base}: {candidate}") + return resolved + + +def main() -> int: + base = os.path.realpath(os.getcwd()) + violations: list[str] = [] + for path in sys.argv[1:]: + try: + safe_path = _confine(path, base) + except ValueError as error: + print(error, file=sys.stderr) + continue + try: + with open(safe_path, encoding="utf-8") as handle: + contents = handle.read() + except (OSError, UnicodeDecodeError) as error: + print(f"Skipping unreadable file {path}: {error}", file=sys.stderr) + continue + violations.extend(find_violations(path, contents)) + + if violations: + print("::error::Non-idempotent raw index SQL in EF migration(s). Use CREATE INDEX IF NOT EXISTS / DROP INDEX IF EXISTS so a startup re-apply cannot throw Postgres 42P07 and fail the Render deploy.") + for violation in violations: + print(violation) + return 1 + + print("OK: no non-idempotent raw index SQL in the checked migrations.") + return 0 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index db68aa8d..a0f44e1e 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -113,6 +113,16 @@ jobs: fi echo "OK: no existing migration was edited (new migrations are allowed)." + - name: Fail on non-idempotent raw index SQL in new migrations + env: + BASE_REF: ${{ github.base_ref }} + run: | + git fetch origin "$BASE_REF" + echo "Changed migration files:" + git diff --name-only --diff-filter=d "origin/$BASE_REF...HEAD" -- ':(glob)**/Migrations/*.cs' + git diff --name-only -z --diff-filter=d "origin/$BASE_REF...HEAD" -- ':(glob)**/Migrations/*.cs' \ + | xargs -0 --no-run-if-empty python3 .github/scripts/check_migration_idempotency.py + convention-guard: name: Guard Conventions runs-on: ubuntu-latest diff --git a/AGENTS.md b/AGENTS.md index f462986c..65bbb2b3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -21,6 +21,25 @@ CLAUDE.md; this file holds the worker contract and the review rules and DEFERS t - DTO/contract changes are append-only and deploy-API-first; the TypeScript consumer (orbit-ui-mobile) updates in lockstep via its own ticket that this one blocks. +### Guardrails you must not trip + +These hold for EVERY worker and every engine. They are enforced by CI, GitHub branch +protection, and the lefthook pre-commit/pre-push hooks, NOT by the Claude Code session +hooks (those do not run under `codex exec` or a raw shell). This list is the readable +copy; the gates are the enforcement. + +- Never push or force-push to `main`/`master`. Branch to `feature/`|`fix/`|`chore/`, + open a PR, squash-merge only. Never reuse a squash-merged branch. +- Never bypass the git hooks: no `--no-verify` (or its `-n` commit alias), no + `--no-gpg-sign` and no `commit.gpgsign=false`. Fix what a hook flags, then commit. +- Never `git worktree remove --force`: on Windows it follows a junction and deletes the + link target. Remove the junctions first, then remove the worktree without `--force`. +- EF migrations must be idempotent: raw `CREATE INDEX` inside `migrationBuilder.Sql(...)` + needs `IF NOT EXISTS`, and raw `DROP INDEX` needs `IF EXISTS` (EF re-applies at startup + on Render; a duplicate raw `CREATE INDEX` throws Postgres 42P07 and fails the deploy). + The fluent `migrationBuilder.CreateIndex(...)`/`DropIndex(...)` API is already safe. + The Guard Migrations CI job enforces this over changed `Migrations/*.cs` files. + ## Code Review Rules Only what no gate can check; mechanical findings belong to CI. Flag P0/P1 only. diff --git a/lefthook.yml b/lefthook.yml index f0c90b97..8fbc38ff 100644 --- a/lefthook.yml +++ b/lefthook.yml @@ -7,3 +7,18 @@ pre-commit: stage_fixed: true dashes: run: node tools/check-dashes.mjs --files {staged_files} +pre-push: + commands: + protect-main: + use_stdin: true + run: | + blocked=0 + while read -r _local_ref _local_sha remote_ref _remote_sha; do + case "$remote_ref" in + refs/heads/main|refs/heads/master) blocked=1 ;; + esac + done + if [ "$blocked" -eq 1 ]; then + echo "BLOCKED: pushing to main/master is forbidden (branch protection, squash-merge only). Open a PR." + exit 1 + fi