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
89 changes: 89 additions & 0 deletions .github/scripts/check_migration_idempotency.py
Original file line number Diff line number Diff line change
@@ -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 <migration.cs> [<migration.cs> ...]
"""
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]:

Check failure on line 29 in .github/scripts/check_migration_idempotency.py

View check run for this annotation

SonarQubeCloud / SonarCloud Code Analysis

Refactor this function to reduce its Cognitive Complexity from 18 to the 15 allowed.

See more on https://sonarcloud.io/project/issues?id=thomasluizon_orbit-api&issues=AZ-VK5E63hYaigBbYKCO&open=AZ-VK5E63hYaigBbYKCO&pullRequest=433
"""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())
10 changes: 10 additions & 0 deletions .github/workflows/test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
19 changes: 19 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
15 changes: 15 additions & 0 deletions lefthook.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Loading