docs: harvest review lessons from PRs reviewed 2026-07-31 to 2026-08-07

.agents/commands/pre-ci.md

@@ -51,11 +51,12 @@ Auto-fixes formatting and lint issues in TypeScript/TSX files. If Biome reports
 
 ## Phase 2 — Fast checks (parallel)
 
-**IMPORTANT: Send ALL 3 commands below in a SINGLE message with 3 parallel Bash tool calls.** Do NOT run them one at a time.
+**IMPORTANT: Send ALL 4 commands below in a SINGLE message with 4 parallel Bash tool calls.** Do NOT run them one at a time.
 
-1. `uv run invoke main.lint` — If ruff reports issues, they were not auto-fixable — report them to the user.
-2. `uv lock --check` — Ensures `uv.lock` matches `pyproject.toml`. If this fails, run `uv lock` and commit the updated lockfile.
-3. `cd frontend/app && npm run codegen:graphql` — Regenerates `graphql-env.d.ts` and `graphql-cache.d.ts` from `schema/schema.graphql`. If the files change, they need to be staged and committed.
+1. `uv run invoke main.lint` — If ruff reports issues, report them to the user.
+2. `uv run ruff check . --exclude python_sdk` — The exact command CI's `python-lint` job runs. This is not redundant with `main.lint`: the invoke tasks run `ruff check --diff`, which exits 0 on violations that have no auto-fix (e.g. `BLE001`), so only the plain check proves CI will pass.
+3. `uv lock --check` — Ensures `uv.lock` matches `pyproject.toml`. If this fails, run `uv lock` and commit the updated lockfile.
+4. `cd frontend/app && npm run codegen:graphql` — Regenerates `graphql-env.d.ts` and `graphql-cache.d.ts` from `schema/schema.graphql`. If the files change, they need to be staged and committed.
 
 ---
 
@@ -67,7 +68,7 @@ Auto-fixes formatting and lint issues in TypeScript/TSX files. If Biome reports
 
 **IMPORTANT: Send ALL 7 commands below in a SINGLE message with 7 parallel Bash tool calls.** Do NOT run them one at a time.
 
-1. `uv run invoke backend.lint` — Run separately from main.lint to avoid `uv run invoke lint` which includes a `yamllint -s .` step that fails on vendored packages in `.venv`. If ruff reports issues, they were not auto-fixable — report them to the user.
+1. `uv run invoke backend.lint` — Run separately from main.lint to avoid `uv run invoke lint` which includes a `yamllint -s .` step that fails on vendored packages in `.venv`. Its ruff step shares the `--diff` blind spot noted in Phase 2; the ty/mypy output is what this check adds.
 2. `cd frontend/app && npx betterer` — Ensures no new TypeScript errors are introduced. The issue count must stay the same or decrease. If it increases, report the new issues to the user.
 3. `uv run invoke docs.lint` — Report any errors. Note: some pre-existing errors in `docs/docs/` may exist — only flag errors in files the user has changed.
 4. `uv run invoke backend.validate-generated` — Ensures generated schema and protocol files are up to date. If this fails, run `uv run invoke backend.generate` and report the regenerated files.
@@ -95,6 +96,7 @@ Summarize results in a table:
 | Docs format | ... |
 | Frontend format/lint | ... |
 | Main Python lint | ... |
+| Ruff (CI parity) | ... |
 | Lockfile sync | ... |
 | Frontend GraphQL types | ... |
 | Backend lint (ty/mypy) | ... |

.agents/rules/testing-python.md

@@ -63,7 +63,11 @@ Skip tests that only exercise library behavior: plain `Enum` value/round-trip ch
 
 ## Pick the cheapest test tier
 
-If the logic needs only in-memory inputs (a `SchemaBranch`, a dataclass, a pure function), write a unit test without DB fixtures — don't default to a component test because a neighbor uses one. Use the database or containers only when behavior genuinely depends on them.
+If the logic needs only in-memory inputs (a `SchemaBranch`, a dataclass, a pure function), write a unit test without DB fixtures — don't default to a component test because a neighbor uses one. Use the database or containers only when behavior genuinely depends on them. When the changed logic seems to need the full integration fixture, first check whether it can be extracted as a pure function over directly-constructible data and unit-tested there.
+
+## Wiring tests parse source, never instrument it
+
+Never add a marker, attribute, or `type: ignore` to production code so a test can observe it. To assert wiring or a convention (the right decorator applied, with the right arguments), parse the module with `ast` + `inspect.getsource` — see `backend/tests/unit/workflows/test_flow_session_convention.py`.
 
 ## Test file placement
 

.agents/skills/creating-changelog-entries/SKILL.md

@@ -18,16 +18,17 @@ compatibility: Requires the project to use Towncrier for changelog management 
 - You're asked to "add a changelog entry", "create a news fragment", or "run towncrier".
 - A PR checklist or CI flags a missing changelog entry.
 
-When NOT to use: the project doesn't use Towncrier; pure internal refactors with no user-facing or maintenance impact; and changes the team has explicitly decided don't warrant an entry. (Most internal maintenance still gets a `housekeeping` fragment.)
+When NOT to use: the project doesn't use Towncrier; pure internal refactors with no user-facing or maintenance impact; and changes the team has explicitly decided don't warrant an entry. (The boundary for internal maintenance is below — don't add a `housekeeping` fragment by default.)
 
 **Exception — unreleased features need no fragment.** A fix or follow-up to a feature that has not
 shipped in any release is not user-observable: the feature's own `added` fragment already covers
 everything a user will ever see, and a `fixed` entry for something never released is noise.
 
 **`housekeeping` is not a catch-all.** It covers internal work a user could still notice — a
 dependency bump, a build or tooling change. A change with no user-facing effect at all (an internal
-type annotation, a behavior-preserving refactor, cleanup of internal docs or spec scaffolding) gets
-no fragment. When it's unclear whether a change is user-facing, ask instead of adding one by default.
+type annotation, a behavior-preserving refactor, a lint or type-checker config cleanup that touches
+no source code, cleanup of internal docs or spec scaffolding) gets no fragment. When it's unclear
+whether a change is user-facing, ask instead of adding one by default.
 
 ## Quick Reference
 

backend/AGENTS.md

@@ -80,6 +80,9 @@ See `dev/knowledge/backend/testing.md` for detailed testing infrastructure docum
 - New database indexes
 - Core schema definition changes
 - New GraphQL mutations/queries
+- New REST endpoints — serve SDK/client needs through existing GraphQL queries first; every REST
+  endpoint is a second public contract that ripples into `schema/openapi.json` and the generated
+  frontend REST types
 
 ### Never Do
 
@@ -125,6 +128,7 @@ Each entry says *when* to load it — open the doc before working in that area.
 - `dev/guides/backend/creating-events.md` - Creating new events
 - `dev/guides/backend/creating-async-tasks.md` - Creating async tasks
 - `dev/guides/backend/creating-messages.md` - Creating message bus messages
+- `dev/guides/backend/creating-migrations.md` - Graph migrations (batching, idempotency, error handling); read before writing a migration or fixing data a migration got wrong
 
 ### ADRs (Why we decided)
 

dev/guidelines/backend/checklist.md

@@ -57,6 +57,19 @@ Plan the access control requirements:
 - **Object-level permissions** - should users only access/modify their own resources?
 - **Branch permissions** - are branch-specific restrictions needed?
 
+## Configuration
+
+### Does this feature add a setting to `infrahub.config.Settings`?
+
+A new setting must be reachable from every Compose entry point, and the two files are maintained
+differently:
+
+- The root `docker-compose.yml` env block is **generated** from the `Settings` classes and
+  CI-validated — regenerate it with `uv run invoke release.validate-dockercomposeenv`
+- `development/docker-compose.yml` forwards each `INFRAHUB_*` variable through a **hand-maintained**
+  anchor that nothing generates or validates — add the mapping yourself, or the setting is
+  documented but untunable in the dev stack
+
 ## Error Handling & User Experience
 
 ### How will users be informed when the feature fails?

dev/guidelines/backend/testing.md

@@ -201,42 +201,8 @@ def test_my_function(test_case: MyFunctionTestCase) -> None:
 
 ### Complex Test Cases
 
-For tests with complex inputs or expected outputs, the dataclass can contain nested objects:
-
-```python
-from dataclasses import dataclass
-
-from infrahub.core.schema import NodeSchema, SchemaRoot
-
-
-@dataclass
-class SchemaValidationTestCase:
-    name: str
-    """Descriptive name for the test scenario."""
-
-    schema: SchemaRoot
-    """The schema to validate."""
-
-    expected_errors: list[str]
-    """List of expected validation error messages."""
-
-
-SCHEMA_VALIDATION_TEST_CASES: list[SchemaValidationTestCase] = [
-    SchemaValidationTestCase(
-        name="missing_required_field",
-        schema=SchemaRoot(
-            nodes=[
-                NodeSchema(
-                    namespace="Test",
-                    name="Device",
-                    attributes=[],
-                )
-            ]
-        ),
-        expected_errors=["Node TestDevice requires at least one attribute"],
-    ),
-]
-```
+Case fields can hold nested objects (a `SchemaRoot`, a list of expected error messages) — the
+structure above scales unchanged; keep constructing them with keyword arguments.
 
 ### When to Use This Pattern
 
@@ -282,9 +248,20 @@ If you find yourself wanting to mock:
 2. **Move up the test pyramid** - A component test requiring extensive mocking to simulate an end-to-end flow is often better written as an integration or functional test
 3. **Question the test scope** - If testing requires mocking half the system, the unit under test may be too large
 
-### Don't shape a mutation to serve its own test
+### Don't shape production code to serve its own test
 
-A method that performs a side effect (write/delete) should not also return data whose only purpose is to let a test assert it ran. If you need to verify the effect happened, assert against the actual state it changed — e.g. read the fake cache/store the method wrote to — rather than trusting a return value added for that purpose. A method either acts or returns something; bending that rule just for a test is a smell, not a shortcut.
+Production code must not change shape just so a test can observe it — no return value added to a
+mutating method, no marker attribute stamped on a wrapper, no `type: ignore` absorbed to make a
+hook attachable. If you need to verify a side effect happened, assert against the actual state it
+changed — e.g. read the fake cache/store the method wrote to — rather than trusting a return value
+added for that purpose.
+
+To verify *wiring* — the right decorator applied, a convention held across a module — parse the
+source instead of instrumenting it: an `ast.parse(inspect.getsource(module))` test reads decorator
+names and arguments off the tree with zero production hooks.
+`backend/tests/unit/workflows/test_flow_session_convention.py` is the house example. Behavioral
+coverage (does the decorator retry?) belongs on the decorator's own tests; the wiring test only
+proves it is attached.
 
 ```python
 # ❌ Bad - return value exists only so the test can assert on it
@@ -346,6 +323,14 @@ not an ambient fact; treat it like any other injected collaborator (see
 Use monotonic time for durations. Wall-clock time (`datetime.now`) is for timestamps that get
 stored or displayed, and it can jump backwards.
 
+### Waiting on async effects: poll, don't sleep
+
+In integration-tier tests the clock cannot be injected — a consumer or worker really does need
+wall-clock time to act. Never guess that duration with a fixed `asyncio.sleep(n)` before
+asserting: on a loaded CI runner the test flakes, and on a fast machine it wastes the time. Poll
+the expected state in a small loop with a deadline, so the test waits exactly as long as the
+outcome takes and fails with a timeout when it never arrives.
+
 ## Exception Testing
 
 When testing that code raises an exception, use the `match` parameter of `pytest.raises` to validate the error message:
@@ -394,6 +379,7 @@ The exact-match principle above is not limited to error messages — it applies
 - **Don't stop at non-emptiness when a specific result is expected.** `assert result` (or `assert len(result) > 0`) is fine for an existence-only contract, but it does not verify *which* result came back — assert the specific expected value when that is part of the behavior under test. And avoid checks that don't even establish non-emptiness: `assert result != frozenset()` is `True` for an empty `list`/`dict`, so it passes when nothing was returned.
 - **Assert a positive count where the number matters.** A test that only checks "no failures" can pass while measuring zero of the thing it claims to test — e.g. if a workflow/name string changes so nothing is counted. Assert that the expected count is `> 0` (or the exact number) so a silently-zero run fails.
 - **Make the scenario actually hold.** A "missing row" test must not create the row; a "no second object" test must prove the count is one. Verify the setup produces the state under test.
+- **Make removal assertions branch-attributable.** A "data is gone" check must read on the branch that held the data, and assert the data resolved *before* the operation as well as after — a read on the wrong branch raises the same not-found either way, so the assertion passes whether or not the code ran.
 - **Denial tests must verify nothing changed.** When asserting an operation is rejected, also reload the target and assert its state is unchanged (or that no row was created/deleted). Asserting only that an error was returned does not prove the write was actually blocked.
 - **When a result is reachable via more than one code path, assert an intermediate signal too.** If "the lookup was never attempted" and "the lookup ran and found nothing" converge on the same final value (e.g. both produce an empty filter), asserting only that final value can't tell a working implementation from a regressed one that silently skipped the lookup. Also assert what was queried or which branch ran — a signal only the intended path produces.
 

dev/guides/backend/creating-migrations.md

@@ -16,6 +16,12 @@ Create a migration when you need to:
 
 Schema definition changes that can be resolved by normal schema migrations (renames, type changes) do not need a graph migration.
 
+When a bug's root cause is a migration that failed to materialize or transform data (e.g. on an
+inheritance change), fix the migration layer — do not compensate by lazily repairing state in
+runtime paths like `Node.save()`, even when that is the smaller diff. A runtime repair leaves the
+stored data wrong for every path that doesn't go through it, and the repair code itself runs after
+validation hooks that the normal write path relies on.
+
 ## Migration Types
 
 Choose the right base class in `backend/infrahub/core/migrations/shared.py`:
@@ -111,6 +117,18 @@ CALL (n) {
 
 `CALL ... IN TRANSACTIONS` requires the `MATCH` to be outside the subquery — move all matching up front.
 
+Size batches by the unit that actually bounds transaction memory. `IN TRANSACTIONS OF n ROWS`
+counts input rows, so when one row fans out to an unbounded set of edges or attributes (deleting a
+node deletes everything attached to it), the per-transaction cost is unbounded no matter how small
+`n` is. Batch over the fan-out unit where possible, and cap a fan-out-prone batch with
+`min(configured_size, ceiling)` rather than inheriting `query_size_limit` unchanged.
+
+**Pitfall — one bad item must not hide the rest.** A migration that iterates independent items
+(branches, nodes, kinds) wraps each item in its own `try`, collects per-item failures into
+`MigrationResult.errors`, and keeps going. A single `try` around the loop aborts on the first
+failure, reports one error, and leaves the state of every remaining item unknown — on re-run the
+operator cannot tell what was reclaimed and what was never attempted.
+
 **Pitfall — don't carry candidate ids across phases in application memory.** A multi-phase migration (phase 1 computes candidate node/edge ids, phase 2 acts on them — e.g. deleting orphans or restoring metadata for what phase 1 touched) must not hold those candidates in a Python list to drive the later phase. A crash between phases loses that in-memory list, and a resumed run can no longer tell what still needs cleanup. Keep candidate selection and its dependent cleanup database-side: re-derive candidates by the same filter inside the same `CALL ... IN TRANSACTIONS` pass as the write that produces them, rather than collecting ids in Python. Distinct logical phases (e.g. reopening edges vs. deleting edges) can and should stay as separate passes when planner or transaction-memory limits require it — each pass just needs to clean up after its own candidates rather than depending on ids collected by an earlier pass. See [Merge Failure Recovery](../../knowledge/backend/merge-failure-recovery.md) for a worked example.
 
 ### Step 4: Beware of Shared Nodes

dev/knowledge/backend/async-tasks.md

@@ -95,6 +95,14 @@ async def validate_schema(db: InfrahubDatabase, branch: Branch) -> bool:
     # ... implementation
 ```
 
+### InfrahubBatch is concurrent, not ordered
+
+The SDK's `InfrahubBatch` runs everything added to it concurrently when executed — grouping tasks
+into one batch (or into a "phase" of batches) is not a serialization mechanism. When tasks must not
+overlap (e.g. writes touching overlapping vertices whose idempotency guards only protect
+*sequential* reruns), cap the batch's max concurrent execution to 1 or run the items in a plain
+loop.
+
 ## Naming Conventions
 
 ### Workflow and Task Names

dev/knowledge/backend/database-schema.md

@@ -43,9 +43,16 @@ Application data nodes. Labels: `Node`, `CoreNode`, `{kind}`, plus inherited sch
 | Property | Type | Description |
 |----------|------|-------------|
 | `uuid` | string | UUID |
-| `kind` | string | Node type (also in labels) |
+| `kind` | string | Concrete node kind (also in labels) |
 | `branch_support` | string | `"aware"`, `"local"`, or `"agnostic"` |
 
+The `kind` property always holds the *concrete* kind; generic kinds exist only as labels. A Cypher
+filter on `n.kind` therefore never matches a generic — it silently returns zero rows. Match
+generics via labels, and remember a label-based sum double-counts nodes that inherit several of the
+requested generics. When a query input must be concrete-only, accept `list[NodeSchema]` and derive
+the kind strings internally, so passing a `GenericSchema` fails type-checking instead of returning
+a silent zero at runtime.
+
 ### Relationship
 
 Links two Node vertices. Label: `Relationship`.

dev/knowledge/backend/events.md

@@ -25,6 +25,14 @@ All events extend `InfrahubEvent` from `backend/infrahub/events/models.py` and c
 - **related**: Additional context resources (returned by `get_related()`)
 - **payload**: Event-specific data (returned by `get_event_payload()`)
 
+### Sensitive values are masked only at construction
+
+The changelog models (`backend/infrahub/core/changelog/models.py`) mask `Password`/
+`HashedPassword` attribute values to `***` in a `model_validator(mode="after")` — which runs only
+when the model is constructed. Assigning `value`/`value_previous` on an existing instance bypasses
+the mask (`validate_assignment` is not enabled) and leaks the secret into the event payload. Build
+changelog entries with their final values; never patch them after construction.
+
 ### Related resources cap
 
 The Prefect API rejects any event whose `related` list exceeds