chore: add local cubic review with Infrahub checklists

[pa-lem]

Why

cubic writes most of the review comments on Infrahub PRs: 1,564 inline comments across 129 PRs between 2026-09-02 and 09-23. About 70% of them were accepted and fixed, which means most of that work could have happened before the PR opened. There was no way to run the same reviewer locally with Infrahub's own rules loaded, and our guidelines live in dev/guidelines/, which isn't one of the locations cubic auto-reads.

Goal: run cubic before pushing so PRs open with fewer threads to answer.

Non-goals: no pre-push hook (a review takes minutes), and no change to CI.

What changed

• Review checklists in .cubic/ (frontend.md, backend.md, testing.md). They are distilled from dev/guidelines/ and .agents/rules/, and leave out anything Biome, ruff, mypy, betterer or knip already enforce. Each ends with a "Do NOT flag" section built from reviewer pushback that kept recurring (migration timestamps, transaction ownership, Cypher comments, React Compiler memoization). Each stays under cubic's 10,000-character limit per agent.
• cubic.yaml attaches each checklist to PR reviews for its paths.
• uv run invoke dev.cubic-review checks the CLI is installed and signed in, picks the base (the closer of stable and develop), then runs cubic with the checklists that match the changed paths.
• reviewing-local-changes skill runs that task, checks every finding against the code, fixes the real ones with approval, and reviews again until clean. It also checks what the diff can't show and cubic often flags: specs that contradict the code, PR description and changelog claims, missing changelog fragments, and stale generated SDK and GraphQL files.
• dev/guides/reviewing-with-cubic.md covers one-time setup and usage.

No product code, schema or API changes.

How to review

1. Start with cubic.yaml. Needs a cubic admin: cubic runs at most 5 custom agents per repo, and a repo cubic.yaml takes priority over dashboard settings. The docs don't say whether YAML agents replace dashboard agents (such as "Flag AI Slop and Fabricated Changes") or add to them. Please confirm before merging.
2. Then the .cubic/*.md checklists. Area owners, please check that the rules and the "Do NOT flag" items match how you review.
3. Then tasks/dev.py (cubic_review) and the skill.

How to test

bash .agents/skills/reviewing-local-changes/scripts/check-cubic.sh
uv run invoke dev.cubic-review

Verified: the preflight script fails with the right fix when the CLI isn't installed, when it's installed but not on PATH, when you aren't signed in, and outside a Git repo. The path-to-checklist mapping returns the expected checklists for sample paths. The task runs end to end. Ruff passes, mypy reports no new errors, and markdownlint and yamllint pass.

Not verified: I haven't seen a review return findings. The only run so far returned Subscription expired, because my account has no seat on the OpsMill cubic plan yet.

Impact & rollout

• Config/env changes: cubic.yaml changes cubic's PR reviews for everyone once merged to stable, which is the only branch cubic reads config from.
• Access: each person who runs a local review needs a seat on the OpsMill cubic subscription.

Checklist

• Tests added/updated: not applicable, dev tooling only
• Changelog entry added: not applicable, not user-facing
• External docs updated: not applicable
• Internal .md docs updated
• I have reviewed AI generated content

Known follow-up: .agents/rules/testing-python.md allows freezegun, but dev/guidelines/backend/testing.md bans it and it isn't a dependency. .cubic/testing.md follows the guideline.

🤖 Generated with Claude Code

[cubic-dev-ai]

3 issues found across 9 files

Confidence score: 2/5

• In tasks/dev.py, interpolating base unquoted into the shell command allows crafted revisions such as stable; ... to execute arbitrary commands; quote the complete revision argument before passing it to _git_output.
• In _closest_base within tasks/dev.py, missing candidate refs can raise an invoke UnexpectedExit traceback instead of a usable message; handle failed ref lookups explicitly.
• In dev/guides/reviewing-with-cubic.md, the guide title exceeds the 2–5-word documentation convention; shorten the title.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="dev/guides/reviewing-with-cubic.md">

<violation number="1" location="dev/guides/reviewing-with-cubic.md:1">
P3: This guide title exceeds the documentation guideline's 2–5-word limit. Shorten it to match the guide-title convention.</violation>
</file>

<file name="tasks/dev.py">

<violation number="1" location="tasks/dev.py:329">
P2: `_closest_base` runs `git rev-list --count origin/{candidate}..HEAD` through `_run(warn=False)`, so any missing ref raises an invoke `UnexpectedExit` traceback instead of a usable message. `origin/stable`/`origin/develop` are absent in fork clones, shallow/filtered clones, and any checkout that never fetched both remote branches (`git rev-parse --verify origin/stable` exits 128 here). Same for the later `git diff --name-only origin/{base}...HEAD` when `--base` names a ref with no local `origin/` copy. The task also never fetches, so stale remote refs silently produce the wrong base, wrong diff, and wrong checklist selection. Verify each candidate ref with `git rev-parse --verify --quiet` (warn=True), fall back to the ones that exist, and tell the user to `git fetch` or pass `--base` when none is available.</violation>

<violation number="2" location="tasks/dev.py:363">
P2: `base` is interpolated into the shell command unquoted, so a value such as `stable; ...` can execute arbitrary commands before cubic runs. Quote the complete revision argument before passing it to `_git_output`.</violation>
</file>

<sub>Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: base is interpolated into the shell command unquoted, so a value such as stable; ... can execute arbitrary commands before cubic runs. Quote the complete revision argument before passing it to _git_output.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tasks/dev.py, line 363:

<comment>`base` is interpolated into the shell command unquoted, so a value such as `stable; ...` can execute arbitrary commands before cubic runs. Quote the complete revision argument before passing it to `_git_output`.</comment>

<file context>
@@ -293,3 +296,84 @@ def test_branch_graph_version(context: Context, branch: str) -> None:  # noqa: A
+        cubic_bin = check.stdout.strip()
+
+        base = base or _closest_base(context=context)
+        changed_files = _git_output(context=context, command=f"diff --name-only origin/{base}...HEAD").splitlines()
+        checklists = _checklists_for(changed_files=changed_files)
+
</file context>

Suggested change

	changed_files = _git_output(context=context, command=f"diff --name-only origin/{base}...HEAD").splitlines()
	changed_files = _git_output(context=context, command=f"diff --name-only {shlex.quote('origin/' + base + '...HEAD')}").splitlines()

[cubic-dev-ai]

P2: _closest_base runs git rev-list --count origin/{candidate}..HEAD through _run(warn=False), so any missing ref raises an invoke UnexpectedExit traceback instead of a usable message. origin/stable/origin/develop are absent in fork clones, shallow/filtered clones, and any checkout that never fetched both remote branches (git rev-parse --verify origin/stable exits 128 here). Same for the later git diff --name-only origin/{base}...HEAD when --base names a ref with no local origin/ copy. The task also never fetches, so stale remote refs silently produce the wrong base, wrong diff, and wrong checklist selection. Verify each candidate ref with git rev-parse --verify --quiet (warn=True), fall back to the ones that exist, and tell the user to git fetch or pass --base when none is available.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At tasks/dev.py, line 329:

<comment>`_closest_base` runs `git rev-list --count origin/{candidate}..HEAD` through `_run(warn=False)`, so any missing ref raises an invoke `UnexpectedExit` traceback instead of a usable message. `origin/stable`/`origin/develop` are absent in fork clones, shallow/filtered clones, and any checkout that never fetched both remote branches (`git rev-parse --verify origin/stable` exits 128 here). Same for the later `git diff --name-only origin/{base}...HEAD` when `--base` names a ref with no local `origin/` copy. The task also never fetches, so stale remote refs silently produce the wrong base, wrong diff, and wrong checklist selection. Verify each candidate ref with `git rev-parse --verify --quiet` (warn=True), fall back to the ones that exist, and tell the user to `git fetch` or pass `--base` when none is available.</comment>

<file context>
@@ -293,3 +296,84 @@ def test_branch_graph_version(context: Context, branch: str) -> None:  # noqa: A
+
+def _closest_base(context: Context) -> str:
+    ahead = {
+        candidate: int(_git_output(context=context, command=f"rev-list --count origin/{candidate}..HEAD"))
+        for candidate in CUBIC_BASE_CANDIDATES
+    }
</file context>

[cubic-dev-ai]

P3: This guide title exceeds the documentation guideline's 2–5-word limit. Shorten it to match the guide-title convention.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guides/reviewing-with-cubic.md, line 1:

<comment>This guide title exceeds the documentation guideline's 2–5-word limit. Shorten it to match the guide-title convention.</comment>

<file context>
@@ -0,0 +1,73 @@
+# Reviewing your branch with cubic before pushing
+
+> Part of: `dev/guides/` | Related: [reviewing-local-changes skill](../../.agents/skills/reviewing-local-changes/SKILL.md), [`cubic.yaml`](../../cubic.yaml)
</file context>

[codspeed]

Merging this PR will not alter performance

✅ 13 untouched benchmarks

---

_{Comparing ple-cubic-local-review (3c05a05) with stable (380f6d1)¹}

Footnotes

1. No successful run was found on stable (7c101af) during the generation of this report, so 380f6d1 was used instead as the comparison base. There might be some changes unrelated to this pull request in this report. ↩

[cubic-dev-ai]

7 issues found across 14 files (changes from recent commits).

Confidence score: 4/5

• dev/guidelines/review/testing.md leaves PT011, PT006, and PT007 uncovered in several test paths, so the review checklist may imply enforcement that Ruff does not provide—retain and explicitly review those cases in this pass.
• dev/guidelines/review/backend.md broadly excludes N801 and N806 under backend/infrahub/**, which can hide violations for rules Ruff does not actually check—limit the exclusion to enabled rules and review uncovered naming cases.
• dev/guidelines/review/frontend.md may report accepted patterns as violations, including the token-storage boundary, the documented 0-on-error badge policy, and the legacy getObjectDetailsUrl signature—scope these rules or add the documented exemptions and typed-wrapper requirements.
• dev/guidelines/review/README.md routes python_testcontainers/tests/** inconsistently with the backend filter, while dev/guides/reviewing-with-cubic.md contains an unusually long paragraph; correct the checklist scope and wrap the prose for clearer reviewer guidance.

Prompt for AI agents (unresolved issues)

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="dev/guides/reviewing-with-cubic.md">

<violation number="1" location="dev/guides/reviewing-with-cubic.md:71">
P3: Wrap this paragraph to match the repository's roughly 100-character prose style; this line is about 170 characters and makes the section harder to scan.</violation>
</file>

<file name="dev/guidelines/review/README.md">

<violation number="1" location="dev/guidelines/review/README.md:11">
P3: The backend row includes `python_testcontainers/tests/**`, but the actual backend filter excludes it and routes those files to the testing checklist. Amend the scope so readers know which pass reviews those tests.</violation>
</file>

<file name="dev/guidelines/review/testing.md">

<violation number="1" location="dev/guidelines/review/testing.md:3">
P2: Ruff does not enforce these rules across all in-scope paths: per-file ignores disable `PT011` in component/functional tests and `PT006`/`PT007` in component/integration tests. Keep the uncovered cases in this pass instead of exempting them globally.</violation>
</file>

<file name="dev/guidelines/review/frontend.md">

<violation number="1" location="dev/guidelines/review/frontend.md:28">
P3: This can flag the documented auth flow, where use cases access tokens through `api/token-storage.ts`. Restrict this rule to direct browser-storage access or explicitly exempt that storage boundary.</violation>

<violation number="2" location="dev/guidelines/review/frontend.md:55">
P3: This blanket rule also flags the documented legacy `getObjectDetailsUrl` signature. Limit it to family-specific helpers and require typed wrappers at their call sites.</violation>

<violation number="3" location="dev/guidelines/review/frontend.md:73">
P3: The route guideline permits a documented, consistent `0`-on-error policy, but this rule flags it unconditionally. Preserve that exception to avoid reporting an explicitly accepted tab-badge design.</violation>
</file>

<file name="dev/guidelines/review/backend.md">

<violation number="1" location="dev/guidelines/review/backend.md:3">
P2: The blanket naming exclusion skips violations Ruff does not check: `pyproject.toml` ignores `N801` and `N806` across `backend/infrahub/**`. Limit this exclusion to rules actually enabled for each path, and review uncovered cases.</violation>
</file>

<sub>Shadow auto-approve: would not auto-approve because issues were found.

Re-trigger cubic</sub>

[cubic-dev-ai]

P2: Ruff does not enforce these rules across all in-scope paths: per-file ignores disable PT011 in component/functional tests and PT006/PT007 in component/integration tests. Keep the uncovered cases in this pass instead of exempting them globally.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/review/testing.md, line 3:

<comment>Ruff does not enforce these rules across all in-scope paths: per-file ignores disable `PT011` in component/functional tests and `PT006`/`PT007` in component/integration tests. Keep the uncovered cases in this pass instead of exempting them globally.</comment>

<file context>
@@ -0,0 +1,73 @@
+# Infrahub backend test review
+
+Flag violations of these Infrahub conventions in `backend/tests/**` and `python_testcontainers/tests/**`. Ruff already enforces annotations, `pytest.raises` breadth (PT011) and the parametrize types, and bans `setup_task_manager`, so do not repeat those. Review only added or changed lines.
+
+## No mocking
</file context>

[cubic-dev-ai]

P2: The blanket naming exclusion skips violations Ruff does not check: pyproject.toml ignores N801 and N806 across backend/infrahub/**. Limit this exclusion to rules actually enabled for each path, and review uncovered cases.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/review/backend.md, line 3:

<comment>The blanket naming exclusion skips violations Ruff does not check: `pyproject.toml` ignores `N801` and `N806` across `backend/infrahub/**`. Limit this exclusion to rules actually enabled for each path, and review uncovered cases.</comment>

<file context>
@@ -0,0 +1,83 @@
+# Infrahub backend Python review
+
+Flag violations of these Infrahub conventions in `backend/infrahub/**`, `python_testcontainers/**` and `tasks/**`. Ruff (`select = ALL`), mypy and ty already enforce import placement, `TYPE_CHECKING`, annotations, `X | None`/`list[...]`, override variance, naming and formatting. Do not repeat those. Review only added or changed lines.
+
+## Exceptions (ruff BLE/TRY are disabled, so this is on you)
</file context>

[cubic-dev-ai]

P3: Wrap this paragraph to match the repository's roughly 100-character prose style; this line is about 170 characters and makes the section harder to scan.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guides/reviewing-with-cubic.md, line 71:

<comment>Wrap this paragraph to match the repository's roughly 100-character prose style; this line is about 170 characters and makes the section harder to scan.</comment>

<file context>
@@ -61,13 +61,14 @@ output says which.
 
 When cubic flags something that is intentional in Infrahub, add a line to the matching checklist's
-**Do NOT flag** section. When it misses something reviewers keep catching, add a rule. Keep each
+**Do NOT flag** section. Edit the file under `dev/guidelines/review/`, not the symlink. When it misses something reviewers keep catching, add a rule. Keep each
 checklist under 9,000 characters: cubic reads only the first 10,000 per checklist and drops the
 rest without warning. If you change which paths a checklist covers, update both `cubic.yaml` and
</file context>

[cubic-dev-ai]

P3: The backend row includes python_testcontainers/tests/**, but the actual backend filter excludes it and routes those files to the testing checklist. Amend the scope so readers know which pass reviews those tests.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/review/README.md, line 11:

<comment>The backend row includes `python_testcontainers/tests/**`, but the actual backend filter excludes it and routes those files to the testing checklist. Amend the scope so readers know which pass reviews those tests.</comment>

<file context>
@@ -0,0 +1,29 @@
+| Pass | Reads | Run by |
+| --- | --- | --- |
+| [Architectural review](architectural-review.md) | Risks the diff adds to the shape of the codebase: special cases that will be copied, files taking on a second job, imports pointing the wrong way | An agent or a human, on request |
+| [Backend conventions](backend.md) | `backend/infrahub/**`, `python_testcontainers/**`, `tasks/**` | cubic |
+| [Backend test conventions](testing.md) | `backend/tests/**`, `python_testcontainers/tests/**` | cubic |
+| [Frontend conventions](frontend.md) | `frontend/app/**` | cubic |
</file context>

[cubic-dev-ai]

P3: This can flag the documented auth flow, where use cases access tokens through api/token-storage.ts. Restrict this rule to direct browser-storage access or explicitly exempt that storage boundary.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/review/frontend.md, line 28:

<comment>This can flag the documented auth flow, where use cases access tokens through `api/token-storage.ts`. Restrict this rule to direct browser-storage access or explicitly exempt that storage boundary.</comment>

<file context>
@@ -0,0 +1,105 @@
+
+- Flag `gql`/`graphql()` strings or `graphqlClient.query/mutate` in `ui/` or pages. Flow: `api/*-from-api.ts` -> `domain/use-cases/` -> `ui/queries/*.query.ts`.
+- Flag a hand-rolled single-node lookup (`resolveUuid`, etc.). Use `useGetObject({ objectId, objectSchema })` with a schema from `useSchema`.
+- Flag imports of another entity's `api/` (cross-entity goes via `domain/` or `ui/`), and `domain/` importing `ui/`, React, TanStack, Jotai or browser storage. Review-only; no lint guard.
+- Flag `domain/` reading global state (branch, date, schema) or a page size. Inject from `ui/` as params.
+- Flag `queryOptions`/`useQuery` in `domain/`. They belong in `ui/queries/`.
</file context>

[cubic-dev-ai]

P3: The route guideline permits a documented, consistent 0-on-error policy, but this rule flags it unconditionally. Preserve that exception to avoid reporting an explicitly accepted tab-badge design.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/review/frontend.md, line 73:

<comment>The route guideline permits a documented, consistent `0`-on-error policy, but this rule flags it unconditionally. Preserve that exception to avoid reporting an explicitly accepted tab-badge design.</comment>

<file context>
@@ -0,0 +1,105 @@
+- Flag pages >~250 lines, forms >~300, pickers >~200, primitives >~150 mixing concerns (soft budgets).
+- Flag nested ternaries for multi-state rendering. Use early returns (order: `isPending`, `error`, `isSuccess`, default).
+- Flag react-aria overlays rendered conditionally (`{open && <Modal isOpen>}`). Pass the boolean to `isOpen` so exit animations run.
+- Flag a tab badge using `count ?? 0` (masks errors) or a loading policy differing from sibling tabs.
+
+## Styling
</file context>

[cubic-dev-ai]

P3: This blanket rule also flags the documented legacy getObjectDetailsUrl signature. Limit it to family-specific helpers and require typed wrappers at their call sites.

Prompt for AI agents

Check if this issue is valid — if so, understand the root cause and fix it. At dev/guidelines/review/frontend.md, line 55:

<comment>This blanket rule also flags the documented legacy `getObjectDetailsUrl` signature. Limit it to family-specific helpers and require typed wrappers at their call sites.</comment>

<file context>
@@ -0,0 +1,105 @@
+
+- Flag `?tab=` query params for tabs. Tabs are nested child routes + `<Outlet />`.
+- Flag inline object/detail paths (`` `/objects/${kind}/${id}` ``, `` `/branches/${name}/${tab}` ``). Use `getObjectDetailsUrl`, `getBranchDetailsUrl`, `getProposedChangeDetailsUrl`. `constructPath` is for non-object pages only.
+- Flag a detail-URL helper whose `tab` param is plain `string`. Require a string-literal union.
+- Flag tab bars not wrapped in `<nav aria-label="Tabs">` or not using `LinkTab`. E2E selectors depend on the nav.
+- Flag child tab routes re-calling the parent's query. Use `<Outlet context={{...} satisfies Ctx}>` + a typed `use-*-outlet` hook that throws outside the route.
</file context>