Preflight: required foreign keys must not form a cycle

[davegaeddert]

Now that foreign keys are NOT DEFERRABLE, every FK is checked at the INSERT that would violate it rather than at commit, which makes a cycle of foreign keys where every edge is required (allow_null=False) permanently un-insertable — each row needs a row that doesn't exist yet, and there is no ordering that fixes it inside or outside a transaction (the one-edge case being a required self-FK, where the first row has nothing to point at). This adds ForeignKeyField._check_required_cycle() next to the existing _check_on_delete(), a depth-first walk over required FK edges that fails at plain preflight time with fields.foreign_key_required_cycle and names every edge in the path, reported once per cycle rather than once per participating field. Nullable back-references remain the supported way to express a cycle — a single allow_null=True edge breaks it and is never flagged, as the CircA/CircB example models covered in the tests show.

---

Generated by Claude Code

[claude]

Code review

One issue found.

Overlapping required-FK cycles: some cycles are reported by nobody

plain/plain-postgres/plain/postgres/fields/related.py

Lines 494 to 500 in 4280b8f

	# Every field in the cycle finds the same cycle, so only the
	# first-sorting edge reports it.
	edges = [(edge.model.model_options.label, edge.name) for edge in cycle]
	if min(edges) != (start.model_options.label, self.name):
	return []

The dedup rests on the premise stated in the comment — "Every field in the cycle finds the same cycle" — but the walk breaks at the first model is start hit, so each field discovers at most one cycle, chosen by stack/declaration order. When a field sits on more than one required-FK cycle, that premise fails: every non-min edge of a cycle suppresses itself in deference to the min edge of that cycle, but the walk run by the min edge may have found a different cycle and reported that one instead. The deferred-to report never happens.

Concrete case — four required FKs (allow_null=False), labels sorting A < B < C:

A.a -> B      B.b -> C      B.z -> A      C.c -> A

Two real cycles: C1 = {A.a, B.z} and C2 = {A.a, B.b, C.c}. A.a is the min edge of both.

field	cycle its walk finds	min edge of that cycle	reports?
A.a	[a, z] = C1	A.a	yes
B.b	[b, c, a] = C2	A.a	no
B.z	[z, a] = C1	A.a	no
C.c	[c, a, b] = C2	A.a	no

One report, two cycles. C2 is named by nobody. Flip the field declaration order on B (z before b) and it inverts — A.a finds C2, and now C1 is the one nobody names. So which cycle you are told about depends on field declaration order.

Impact is bounded: preflight still fails. The globally minimal cycle-participating edge always finds a cycle it is the min of, so at least one error always fires and a broken schema cannot pass. The cost is incomplete diagnostics — a developer fixes the one named cycle, re-runs plain preflight, and gets a fresh cycle error they were never told about, converging over N runs instead of naming all N at once. That is narrower than the stated contract in the PR description: "names every edge in the path", "reported once per cycle".

Fix direction: drop the break and collect every start-hit in the walk, reporting each cycle for which self is the min edge; or compute strongly-connected components over the required-FK graph once and attribute each cycle to its own min edge. The existing tests only cover disjoint single-cycle graphs, so an overlapping-cycle case would be worth adding alongside them.

---

Also checked for CLAUDE.md compliance (root, plain-postgres) — no violations. Test placement in tests/internal/ matches .claude/rules/tests-layout.md and the existing preflight-test convention, and the PreflightResult / model_options.label / _model_meta.fields API usage all verified against the codebase.