chore(agents): make pre-ci detect changed areas and close the frontend gap - #10405
Conversation
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
|
Would it make sense to point stable? |
4fbe926 to
cae9f48
Compare
cae9f48 to
6f8a302
Compare
There was a problem hiding this comment.
1 issue found across 1 file (changes from recent commits).
Confidence score: 5/5
- In
.agents/commands/pre-ci.md, thegit status --porcelainparsing only handles plain paths; pending renames output asR old -> newwould be treated as a literal path and fail to match area globs, potentially skipping a staged rename from the diff. Since this is a build/pre-ci helper with no user-facing impact, it's low risk, but consider usinggit diff --name-onlyor splitting on whitespace before the arrow to cover renames.
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=".agents/commands/pre-ci.md">
<violation number="1" location=".agents/commands/pre-ci.md:45">
P3: The porcelain `cut -c4-` parsing does not handle pending renames: `git status --porcelain` reports them as `R <old> -> <new>`, which becomes the literal path `old -> new` and matches no area glob, so a staged-but-uncommitted rename can be classified as 'no changes'. Use `git status --porcelain -z` NUL parsing or split rename lines on ' -> ' to capture both paths.</violation>
</file>
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
…d gap Scope the pre-ci checks to the areas actually touched, and bring the frontend side up to full parity with ci.yml. - Add a Phase 0 detection step whose area globs mirror the `<area>_all` lists in .github/file-filters.yml, so local gating matches the `files-changed` outputs CI branches on. - Always run the frontend lint trio: the `frontend-lint` job has no path filter, so a backend-only change still fails CI when frontend lint is broken. - Cover the frontend checks that were missing entirely: `knip`, `betterer ci`, the `@infrahub/graph` package suite, and the three generated-artifact validations (openapi types, graphql types, error catalogue bindings), each gated on the same narrow filter as its CI job rather than the whole frontend area. - Correct the CI job names in the summary table: there is no `backend-lint` job (ty runs in `python-lint`), JSON schema validation is `json-schema` not `openapi-schema`, docs.lint maps to `markdown-lint` and `validate-documentation-style` rather than `documentation`, and the lockfile check lives in uv-check.yml. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…pre-ci
Address the open cubic review threads.
- Add a `yaml` area and a Phase 2C running `uv run yamllint -s .`, the exact
command CI's `yaml-lint` job runs. That job fires on any `**/*.{yml,yaml}`
change, so a yaml-only edit previously passed pre-ci all-green and could
still fail CI.
- Split `**/*.py` out of the `backend` area into its own `python` area gating
only the lint phases. `python-lint` fires on `backend || python`, but
`backend-tests-unit`, `graphql-schema` and `json-schema` gate on `backend`
alone, so a change to `models/` or a root script no longer drags in the slow
backend phases.
- Unwrap `git status`'s rename form in the Phase 0 detection snippet, which
otherwise yields the literal path `old -> new` and matches no area.
- Correct the 4B rationale: `.yamllint.yml` ignores `.venv`, so the reason to
call `backend.lint` directly is that `invoke lint` bypasses the area gating,
not a vendored-package failure. Drop `python_testcontainers/**` from the
backend row - it is in `e2e_all`, not `backend_all`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…ase ref Address the second round of cubic review threads. - Add a `testcontainers` area and Phase 5.2 running `uv run --directory python_testcontainers pytest --rootdir=. -c pyproject.toml -vs tests`, mirroring the `backend-testcontainers-unit` job. That suite was never covered: `backend.test-unit` runs `backend/tests/unit` only, and `python_testcontainers` is a separate uv project. The job also fires on any `.github/workflows/*.yml`, not just `ci.yml`. - Fix Phase 0 base-ref selection. Both `develop` and `stable` exist, so preferring `develop` unconditionally diffed a stable-based branch against a merge base tens of commits back - on this branch it reported 28 changed files instead of 4, marking every area as changed. Pick whichever candidate leaves HEAD fewest commits ahead instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
…o pre-ci Address the third round of cubic review threads. - The Phase 0 pipeline called `git rev-list`, `sort`, `head`, `cut` and `sed`, none of them in `allowed-tools`, so the documented detection could not run without extra authorization. Rewrite it to use only `git rev-parse`, `git merge-base`, `git diff`, `git ls-files` and `sort`: base selection now compares the two merge bases with `--is-ancestor` instead of counting commits, and uncommitted work comes from `git diff HEAD` plus `git ls-files --others` instead of parsing `git status --porcelain`. `--no-renames` keeps both sides of a rename so the area that lost the file is still flagged. - Add `uv run ty check .` to Phase 2B. CI's `python-lint` job runs it on the `python` filter, but ty was only reachable through Phase 4B `backend.lint`, which the `python` area does not enable - so a `models/` or root-script change cleared pre-ci and still failed CI. 4B now advertises mypy as what it adds. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The previous commit claimed Phase 1B made CI's `ruff format --check` step redundant. It does not: `uv run invoke format` reformats only `tasks`, `models`, `utilities`, `python_testcontainers` and `backend`, while `python-lint` runs `ruff format --check --diff --exclude python_sdk .` over the whole repo. An unformatted file under `development/`, `tests/` or the repo root cleared pre-ci and still failed CI - the same coverage gap already documented one step earlier for `ruff check`. Add the whole-repo format check as its own Phase 2B step. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
fee04b7 to
85c0c04
Compare
There was a problem hiding this comment.
All reported issues were addressed across 1 file
Shadow auto-approve: would not auto-approve because issues were found.
Re-trigger cubic
…n area Address the fourth round of cubic review threads. - Add a `schema` area (`schema/**`) enabling Phase 4D. The phase guard already read "if backend or schema/** changed" and the note called that deliberately stricter than CI, but Phase 0 classified paths into six areas and none of them was schema - a schema-only change detected nothing and 4D could never fire. - Guard every base ref, not just `origin/develop`. On a fork or a remote carrying only one of the two branches, the unguarded `git merge-base HEAD origin/stable` aborted the block and left `MERGE_BASE` empty, silently detecting nothing instead of degrading to the documented all-areas fallback. The loop now handles both refs the same way and falls through when neither resolves. - Fetch `stable` and `develop` before comparing merge bases. Stale refs move a merge base backwards, which usually just over-detects, but between two candidates a stale ref can flip the choice and narrow the diff instead. Warn and treat the run as best-effort if the fetch fails. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Four rounds of review fixes each left a paragraph of rationale behind, and the justification had grown longer than the instructions it justified. Collapse it: Phase 0's four explanatory paragraphs become one "do not simplify this" bullet list, Phase 2B's per-step essays become a single note covering steps 2-4, and every phase keeps its command, its gate and the trap worth naming - `biome ci` not `check --write`, `betterer ci` not bare, 3A always runs, `--no-renames`, the separate uv project behind 5.2. No command, gate or summary row changes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Updates the pre-ci agent command to detect changed areas and add frontend lint/test checks at CI parity. Pure internal developer tooling documentation with no production, security, or contract impact.
Re-trigger cubic
polmichel
left a comment
polmichel
left a comment
There was a problem hiding this comment.
This is a great idea 👍
Have you tried to use /pruning-residues skill on this skill? When I've run it locally it offers me to trim/condense a number of items.
…script Phase 0 was a shell block that picked a base ref by comparing merge bases, plus five bullets defending each of its lines. Replace it with the instruction itself: find the areas this branch touched relative to whichever long-lived branch it forked from, and include anything uncertain. `release-*` bases now work without extending a script. Also prune the commentary the review loop deposited across the rest of the file — restated rationale, "easy to miss" framing, and defences of choices against alternatives no longer present. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
Verified every command and job name in the file against .github/workflows/ci.yml and .github/file-filters.yml: - `--fast` claimed to skip ty, but `ty check .` now runs in Phase 2B, before the --fast stop point. Only mypy is skipped. - The area table claimed each row was a verbatim `<area>_all` list. Two rows are deliberate deviations (`python` narrower, `schema` has no CI filter); say so instead of over-claiming parity. - `docs.validate` mirrors `validate-generated-documentation`, which fires on `python || documentation` — the reference docs are generated from Python source. Gate 4C.2 accordingly. - `infrahub-testcontainers-uv-check` had no local mirror; add the second `uv lock --check` for the python_testcontainers project. - Summary table named a workflow file, not a job: `uv-check (uv-check.yml)` is really `infrahub-uv-check` plus `infrahub-testcontainers-uv-check`. - backend enables Phase 5.1, not all of Phase 5 (5.2 is the testcontainers suite). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
4C.2 was gated on the `python` area alone, but that area is written as "any *other* `**/*.py`" so `backend/foo.py` lands in `backend` and never reached it. CI's `python` filter is `**/*.py` including `backend/**`, so a backend-only Python change fires `validate-generated-documentation` while pre-ci reported all-clear. `backend.validate-generated` does not cover the reference docs — it diffs `core/schema/generated`, `protocols.py` and the SDK submodule, never `docs/` — so nothing else caught it. Enable 4C.2 for `backend` too. Phase 0 asked for "both sides of a rename" without saying how, and the allowed-tools list offered `git status`, whose `R old -> new` collapses into a single unmatched path. Name the three collection commands and the `--no-renames` requirement, and drop `git status` from allowed-tools. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The note claimed `git status --porcelain` collapses `R old -> new` into one path that matches no area. It does not: the entry carries both paths. That claim was true only of the `cut -c4-` pipeline it was originally written about, which yields the literal string `old -> new`; the premise went away when the pipeline did. Justify `--no-renames` by what it actually prevents — verified against a real rename, `git diff --name-only HEAD` reports only the new path — and drop the `git status` aside, which no longer has a tool in allowed-tools to warn about. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Updates the pre-ci agent command definition to detect changed areas and run frontend lint/test checks at CI parity. Pure internal developer tooling; no production, security, data, or contract impact.
Re-trigger cubic
Three rounds of review left justifications aimed at the reviewer rather than at the agent that runs this command: proof that the 4C.2 gap was real, a defence of which areas diverge from CI's filters, and two rebuttals of "isn't this step redundant?". Each rule they argue for is already stated by a phase gate or an area row, so the argument is noise to whoever reads this next. Gating, commands and job names are untouched. Also drops the now-unused `Bash(sort:*)` permission — Phase 0 no longer shows a pipeline. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
All reported issues were addressed across 1 file (changes from recent commits).
Shadow auto-approve: would not auto-approve because issues were found.
Tip: Review your code locally with the cubic CLI to iterate faster.
Re-trigger cubic
The previous commit cut the caveat but left the absolute claim it qualified: "local gating matches the files-changed outputs CI branches on" is false for two rows. `backend_all` carries no `schema/**` entry, so graphql-schema and json-schema never fire on a schema-only change and Phase 4D exists only behind the local `schema` area; `python` is narrower than CI's `**/*.py` on purpose. An agent told to match CI would resolve the contradiction by deleting the schema area or widening python — both regressions. Mark the two rows as intentional instead, in one line rather than the paragraph that was cut. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
0 issues found across 1 file (changes from recent commits).
Confidence score: 5/5
- Automated review surfaced no issues in the provided summaries.
- No files require special attention.
Shadow auto-approve: would auto-approve. Updates the internal pre-ci agent command to detect changed areas and run frontend checks at CI parity; delta since prior approval is documentation wording cleanup only. Developer tooling; no production, security, data, or contract impact.
Re-trigger cubic
3 participants
Problem
/pre-cidescribes itself as running "all locally-executable CI checks", but it ran only part of the frontend gate. It could report all-green whilefrontend-lintandfrontend-testsfailed on the PR.Checked against
.github/workflows/ci.yml:Missing entirely
pnpm knip— step 2 of thefrontend-lintjobpnpm test:coverage— thefrontend-testsjob (Phase 4 ran onlybackend.test-unit)frontend/packages/graphtests — a separate step of the same job, outsidefrontend/appWrong variants
npx biome check --write .instead ofpnpm exec biome ci .— the auto-fix variant, not the check-only one CI assertsnpx bettererinstead ofpnpm exec betterer ci— barebettererrewrites the snapshot rather than failing on an increasenpx/npmused throughout a pnpm workspaceChange
Phase 0 — detect what changed. Diffs against the merge-base with
origin/develop(falling back toorigin/stable) and includes the working tree, since uncommitted work is about to be pushed too. Paths are classified with the same globs as.github/file-filters.yml, so each area's phases run only when relevant.--allforces every phase.Phase 3 — complete frontend gate, at CI parity, with each command annotated with the job it mirrors.
Two things worth flagging for reviewers:
frontend-linthas no path filter inci.yml— it runs on every PR. A backend-only change still fails CI if frontend lint is broken. Phase 3A therefore always runs the lint trio regardless of detection.frontend-testsruns a second suite infrontend/packages/graph. It is also absent from the four-command gate documented infrontend/app/AGENTS.md.The summary table now lists skipped rows explicitly (
skipped (no backend changes)) rather than omitting them, so partial coverage is visible rather than reading as a full pass.Notes
🤖 Generated with Claude Code