preflight: engine-role privilege checks with typed GRANT-naming refusals

[Kiran01bm]

Summary

Implements the engine-role privilege preflight: before touching a table, migrate now verifies the connected role holds the access its path needs, per the tiered contract in docs/engine-role.md. A missing grant becomes an up-front typed refusal naming the exact GRANT statement, instead of a mid-change server error.

What

• pkg/preflight.CheckPrivileges(ctx, pool, schema, table, Requirement) walks the contract's tiers bottom-up — database CONNECT, schema USAGE, owning-role membership, schema CREATE, SET ROLE-usable membership — and gathers all catalog facts in one snapshot so checks cannot disagree. Replication access (rds_replication membership or the REPLICATION attribute) is checked only when the requirement declares logical decoding; the native path never asks for it.
• PostgreSQL-version-aware semantics: PG 16+ checks pg_has_role(..., 'SET') (grants issued WITH SET FALSE are refused); PG 14/15 use plain membership, which is what SET ROLE consults there.
• Every miss is a typed *PrivilegeError carrying the tier, the failed catalog check, and the exact provisioning statement. On success, a PrivilegedRole proof carries the catalog-resolved owner for later SET ROLE use.
• An unresolved target is separated into its causes: schema missing, schema USAGE missing (which hides tables from name resolution — including the 42501 raised by qualified-name resolution), or table genuinely absent.
• migrate runs the check at the in-place-ALTER tier before the size guard and maps *PrivilegeError to a new refusal reason, insufficient-privileges, with the grant in detail.
• Tests: an integration walk of the full ladder (each refusal's own Grant statement unlocks the next rung), SET FALSE membership refusal on PG 16+, replication-access variants, unresolved-target causes, and a CLI-level test asserting refused-then-executed around the exact grant. The throwaway-role helper moved to internal/testutil for reuse.

Why

Real targets own DDL under application roles the engine is not a member of by default. The provisioning contract is documented but nothing verified it at run time — an operator learned about a missing grant from must be owner of table ... halfway into a change. Fail-closed refusal with the exact remediation is the same posture every other refusal in the engine takes, and the typed error is the seam an orchestrator adapter consumes.

Before / after

Before:
  parse ── gate ── size guard ── ALTER TABLE ──► server error mid-change:
                                                 "must be owner of table ..."

After:
  parse ── gate ── privilege preflight ── size guard ── ALTER TABLE
                        │
                        └─ missing access ──► refused (insufficient-privileges)
                           detail: the failed catalog check + the exact
                           GRANT that satisfies it (docs/engine-role.md)

[chatgpt-codex-connector]

You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard.

[morgo]

🤖 Approving on Morgan's behalf (automated review; pg-sprite liberal bar).

The safety sweep is clean across the board: catalog queries are fully parameterized (schema/table via $n with server-side quote_ident), every identifier in the executable Grant remediation text goes through pgx.Identifier{}.Sanitize(), the preflight is strictly read-only (SELECTs against catalogs and has_*_privilege/pg_has_role; it names the GRANT, never runs it), and every error/NULL path fails toward refusal — including COALESCE(..., false) on the one NULL-able membership probe and fail-closed handling of unknown step shapes. The PG 16 gating is right (pg_has_role(..., 'SET') and WITH INHERIT TRUE behind server_version_num >= 160000, with the ≤15 remediation correctly switching to ALTER ROLE ... INHERIT), and the ladder integration tests apply the asserted GRANT string verbatim to prove each rung unlocks the next — a nice closed loop.

Design notes, none blocking:

• The "single snapshot so checks cannot disagree" comment holds for tiers 0–2 only; the SET-ROLE and replication probes are later queries. All such paths still fail closed, so it's comment accuracy, not a hole.
• TOCTOU between preflight and execution is inherent (a grant revoked after the check surfaces as a mid-change server error); the PR doesn't claim otherwise.
• The CONNECT rung is near-tautological for an already-connected session and can't see pg_hba — it documents the contract more than it catches failures.
• requiredTier fails closed on step shapes the router doesn't emit today; when the router grows (e.g. a DROP INDEX cleanup step), remember to map it or migrate will abort with a plain error instead of a typed verdict. Relatedly, TierCopyAndSwap/LogicalDecoding are package-complete but not yet CLI-reachable.

Nit: PrivilegeError.Check interpolates names unquoted — display-only (the executable Grant field is sanitized), but exotic identifiers render ambiguously in the message.

[aparajon]

🤖 Adversarial correctness review, requested by Armand and performed by his agent. Reviewed at head fee66f3, built and run against a live PostgreSQL 16.14.

Verdict: the central claim holds — I tried hard to make the preflight refuse something the server would have allowed, and could not. The tier ladder matches what PostgreSQL actually enforces: I verified rung by rung that CREATE INDEX and CREATE INDEX CONCURRENTLY really do fail with permission denied for schema on table ownership alone, that ADD CONSTRAINT ... USING INDEX does not, and that an index rebuild during a table rewrite does not either. Tier 2 is a real requirement, not an over-strict gate, and the refusal → GRANT → re-run loop closes end-to-end.

What I did find is that the promise in execute's doc comment — "a role that would die mid-change is refused with the exact provisioning statement instead" — has two live holes left. Neither is a regression (both died mid-change before this PR too, just with a worse message), so I'm not blocking on them, but finding 1 is squarely inside the contract this PR is establishing and I'd like it closed before the tier work is considered done.

Findings

1. requiredTier maps every ALTER TABLE step to Tier 1, but three ALTER TABLE shapes build a new index and need Tier 2. The tier is derived from the step's statement kind, so an index-creating ALTER is indistinguishable from an in-place one. I confirmed against the server that all three of these need CREATE on the schema:

=== ALTER TABLE s.target ADD CONSTRAINT u1 UNIQUE (c) ===        ERROR: permission denied for schema s
=== ALTER TABLE s.target ADD COLUMN e int UNIQUE ===             ERROR: permission denied for schema s
=== ALTER TABLE s.target ADD CONSTRAINT p PRIMARY KEY (c) ===    ERROR: permission denied for schema s

On the default path this is masked: those shapes are ReasonSaferIdiom, so they are either substituted into usingIndexSequence (whose CREATE UNIQUE INDEX CONCURRENTLY step correctly lifts the requirement to Tier 2) or refused as rewrite-required. With --force they are not, and --force's own help text promises the forced run "is still parsed, preflighted, size-guarded, and budget-bounded". Reproduced end-to-end on a role holding Tier 1 but not Tier 2 — the unforced run gives the verdict this PR exists to give, and the forced run dies exactly the way the PR says it prevents:

$ pg-sprite migrate --alter "ALTER TABLE s.target ADD CONSTRAINT u2 UNIQUE (d)"
refused (insufficient-privileges)
  detail: engine role lacks access for index builds: has_schema_privilege(engine, s, 'CREATE') is false;
          provision with: GRANT CREATE ON SCHEMA "s" TO "app" (see docs/engine-role.md)

$ pg-sprite migrate --alter "ALTER TABLE s.target ADD CONSTRAINT u2 UNIQUE (d)" --force s.target
failed (execution-failed)
  detail: execution failed; nothing committed — a started bounded attempt rolls back
pg-sprite: error: run schema change on s.target: optimistic attempt: ERROR: permission denied for schema s (SQLSTATE 42501)

The fix is to derive the tier from the step's operations rather than its kind: an ALTER TABLE whose ops include ADD CONSTRAINT UNIQUE/PRIMARY KEY without USING INDEX, or ADD COLUMN with an inline UNIQUE/PRIMARY KEY, creates a new index and is Tier 2. The boundary is narrow and I pinned both sides of it empirically, so it's cheap to encode: USING INDEX stays Tier 1 (verified: succeeds with no schema CREATE), and so does a rewriting ALTER COLUMN ... TYPE that rebuilds existing indexes (also verified). TestRequiredTier would have caught this with one more case — it currently exercises ADD COLUMN c int as its in-place example, which is genuinely Tier 1.

Repro: unit-level, no database (fails on fee66f3, passes once the tier is derived from ops)

func TestRequiredTierCoversIndexCreatingAlters(t *testing.T) {
	for _, sql := range []string{
		"ALTER TABLE s.t ADD CONSTRAINT t_c_key UNIQUE (c)",
		"ALTER TABLE s.t ADD CONSTRAINT t_pkey PRIMARY KEY (c)",
		"ALTER TABLE s.t ADD COLUMN e int UNIQUE",
	} {
		t.Run(sql, func(t *testing.T) {
			tier, err := requiredTier([]string{sql})
			require.NoError(t, err)
			assert.Equal(t, preflight.TierIndexBuild, tier,
				"the server requires CREATE on the schema for the index this step builds")
		})
	}
}

All three subtests fail today with expected: 2, actual: 1. The counterpart cases that must stay Tier 1 — ADD CONSTRAINT ... UNIQUE USING INDEX and ALTER COLUMN ... TYPE — already pass and are worth pinning alongside them, since they're what keeps the fix from over-reaching.

2. A cross-owner foreign key still dies mid-sequence, on the default unforced path. ADD CONSTRAINT ... FOREIGN KEY needs REFERENCES on the referenced table, which the ladder never looks at — it only resolves the target's owner. Where every table shares one owning role (the setup docs/engine-role.md assumes, and the reason it can promise "exactly one GRANT") this never fires. Where the referenced table belongs to a different role it does, and the substituted safer sequence fails on step 1:

$ pg-sprite migrate --alter "ALTER TABLE s.target ADD CONSTRAINT fk FOREIGN KEY (pid) REFERENCES s.parent (id)"
failed (execution-failed)
  failed at: step 1: ALTER TABLE s.target ADD CONSTRAINT fk FOREIGN KEY (pid) REFERENCES s.parent (id) NOT VALID
pg-sprite: error: ... optimistic attempt: ERROR: permission denied for table parent (SQLSTATE 42501)

I'd treat this as a scope question rather than a defect: either extend the Tier-1 rung to check has_table_privilege(<referenced>, 'REFERENCES') when the plan's ops name one, or state in docs/engine-role.md that the contract covers the target's own access and that cross-owner references need their own grant. Right now the page's framing — "run schema changes against tables it does not own" — reads like it covers this, and it doesn't.

3. (nit) The Tier-0 CONNECT rung cannot fire through the CLI. You can't fail a has_database_privilege(..., 'CONNECT') check on a connection you already hold; revoking CONNECT makes the pool fail first, well before the preflight:

pg-sprite: error: ping after connect: ... FATAL: permission denied for database "postgres" (SQLSTATE 42501)

Nothing is lost — that message is already actionable — so the rung is contract documentation rather than a live check. Worth one clause in the TierConnect comment saying so, otherwise a future reader will assume it's load-bearing and preserve it through a refactor for no reason.

4. (nit) The Tier-2 refusal's Check and Grant name different roles, with nothing explaining why. The operator reads has_schema_privilege(engine, s, 'CREATE') is false; provision with: GRANT CREATE ON SCHEMA "s" TO "app" and has to already know that the engine inherits through the Tier-1 membership before that stops looking like a bug in the message. The rationale is in the code comment and in the docs table, but not in the one line the operator actually sees. Half a clause — "granted to the owning role, which the engine inherits" — would close it.

5. (nit) docs/engine-role.md advertises two checks that don't exist. The Tier-3 row lists pg_has_role(..., 'MEMBER') for 14–15, but checkSetRoleAccess deliberately returns early there (correctly — USAGE implies MEMBER, so the Tier-1 rung already proved it). And the Tier-4 row has a filled-in "Preflight check" column while Tier stops at copy-and-swap and CheckPrivileges rejects Tier(4) as out of range. Both are the same shape as the doc/code gap: the page is the contract, so a reader auditing it against the implementation finds two rows that don't correspond to code.

Drive-by, pre-existing and not this PR's: when step 1 of a sequence fails, the wrapped error says "steps before it committed and their state remains" even though none did. The verdict's Detail gets this right ("no earlier steps had committed"); it's only the error string that misleads, and only in the Step == 1 case.

Action items

1. (Finding 1) Derive the tier from the step's operations, not its kind, so index-creating ALTER TABLE shapes reach Tier 2; add the three cases above to TestRequiredTier plus the two Tier-1 counterparts that must not regress.
2. (Finding 2) Decide the scope for cross-owner REFERENCES — either check it or say in docs/engine-role.md that it's out of contract.
3. (optional) (Findings 3–5) The comment clause on TierConnect, the grantee explanation in the Tier-2 message, and the two doc rows.

Verified (tried to break, couldn't)

Built the branch and ran it against PostgreSQL 16.14 with the exact fixture shape from docs/engine-role.md — superuser-owned schema, table owned by an application role, engine role holding only LOGIN. Attacked the Tier-2 requirement first, on the theory that it might be a false gate: it is not — plain CREATE INDEX, CREATE INDEX CONCURRENTLY, and both constraint-implied index builds all fail with permission denied for schema on ownership alone, so nothing here refuses a change the server would have run. Pinned the other side of that boundary too: ADD CONSTRAINT ... USING INDEX, a binary-coercible ALTER COLUMN ... TYPE on an indexed column, a rewriting int → bigint on an indexed column, plain ADD/DROP COLUMN, SET NOT NULL, ADD CONSTRAINT ... CHECK ... NOT VALID, and VALIDATE CONSTRAINT all succeed without schema CREATE — so Tier 1 is not silently under-provisioned for the shapes the planner actually emits, and I walked every SaferSQL constructor (setNotNullSequence, usingIndexSequence, AddNotValid, the Concurrently rewrites) to confirm the reachable step shapes are exactly ALTER TABLE and CREATE INDEX. Checked that the ordering claim survives the surrounding flow: schemadiff.Introspect runs earlier in run and reads pg_class by name, which needs no schema USAGE, so it does not pre-empt the privilege verdict — a qualified target in a schema the role cannot see reaches the check and refuses with GRANT USAGE, exactly as designed, rather than the "table not found" this PR set out to eliminate. Confirmed a genuinely absent table in a readable schema still reports not-found, so the two causes really are separated at the boundary and not just in the tests. Verified the closed loop the tests assert also holds through the CLI: applying the refusal's own Grant string verbatim lets the identical command execute — checked for the Tier-1 membership refusal, the Tier-2 schema CREATE refusal, and the Tier-0 USAGE refusal. Read the PG16 membership-option handling against the 14/15 role-attribute path and agree the split is right, including that pg_has_role(..., 'USAGE') implies MEMBER so the pre-16 SET ROLE early return is sound rather than a gap; the CI matrix covering 14 through 18 is what backs the versions I couldn't run locally. to_regclass on a qualified name does raise 42501 rather than returning NULL when USAGE is missing, so that branch is real and not defensive. Unit tests green at head. Leak check passed on the body, diff, and this comment.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Second-pass review through the two lenses Armand asks for on pg-sprite — ease of OSS adoption and SchemaBot integration — performed by his agent. Correctness findings are in the separate comment above; nothing here blocks.

Lens 1: OSS adoption ease

The standout is the closed loop, and it's worth naming because it's rare. Most tools that check privileges tell you that you lack them. This one hands you the statement, and the tests then execute that same string and assert the change goes through. I ran the loop by hand at all three rungs and it holds — copy the Grant out of the refusal, paste it, re-run the identical command, done. For an adopter evaluating this against "just give the tool an owner login", that is the argument: you can go from a fresh LOGIN-only role to a working engine role in three refusals, without reading anything.

docs/engine-role.md is the page that makes the argument land — especially "Why ownership, not privileges", which pre-empts the first question every DBA asks, and the "must not have" section, which is what a security reviewer will actually skim. Two things would raise it further:

• --dry-run is the front door for an evaluator, and it can't tell them their role is under-provisioned. run diverts to runDryRun before execute, so the privilege check never runs on the path people try first. Provisioning is exactly the thing you want to discover before you commit to a change, and right now the only way to discover it is to attempt one for real. Surfacing the privilege verdict from --dry-run — or a small pg-sprite preflight --tier that reports the whole ladder in one shot — turns provisioning from a three-round trial-and-error loop into one command. That is a meaningful adoption difference for a tool whose first-run experience is "point it at a database you don't own".
• The Tier-2 message asks the operator to grant to a role they didn't expect (covered as a nit in the other comment). It's the one place in the loop where copy-paste needs a moment of understanding first.

Smaller notes:

• The rds_replication handling is good OSS hygiene: it's gated on the role existing, so a self-managed cluster gets the REPLICATION attribute advice instead and nobody outside AWS reads an error about a role they've never heard of. Worth keeping that shape as more managed-flavor quirks arrive.
• docs/engine-role.md documents a Tier 4 that Tier doesn't implement. On a public contract page an unimplemented row reads as a capability, not a roadmap.
• The new tests create cluster-level roles, so running the suite against an external PG_DSN now requires role-creation rights on that server — a real step up from "a database you can create schemas in". TestCheckPrivilegesReplicationAccess already skips itself for this reason; the rest don't. And because NewRole's cleanup only t.Logfs on failure, a failed run against a shared server leaks roles rather than failing loudly. A line in the contributing docs about what PG_DSN now needs would save a contributor a confusing first failure.

Lens 2: SchemaBot integration

The seam is right, and the exit-code split is the part that matters most. SchemaBot needs to distinguish "this environment is not provisioned for this change" from "this change failed", because they route to completely different operator experiences — a blocked check with remediation versus a paged apply failure. ExitCodeRefused plus a typed Reason gives that split cleanly, and insufficient-privileges is fail-closed in the direction SchemaBot needs: an under-provisioned role can never become a passing check by accident.

Three integration points I'd raise now, while the shape is still cheap to change:

1. Tier derivation lives in internal/cli, and it shouldn't. requiredTier is a property of the routed plan — which steps the engine decided to run — not of the CLI. Any orchestrator embedding pg-sprite as a library has to re-derive it from router.Statement.ExecSQL on its own, and would independently reproduce finding 1 from the other comment, because the mapping from step shape to required access is genuinely non-obvious (I had to check it against a live server to be sure). Moving it next to the router and exporting it means the tier is decided once, tested once, and every consumer gets the same answer. This is the single change that would most reduce the work on SchemaBot's side.

2. The check runs at apply time; SchemaBot's value is at plan time. Privilege state is a property of the environment, not the diff — which makes it exactly the kind of thing that belongs in a PR check, days before anyone types an apply command. Same underlying ask as the --dry-run point above, but the stakes differ: for an OSS user a late refusal is an annoyance, whereas for SchemaBot a privilege problem discovered at apply time means an operator is already watching a PR waiting for a change to land. If the check is reachable without executing, SchemaBot can surface "this database's engine role can't run this plan, here's the grant" while the PR is still being reviewed.

3. Detail will be rendered into public PR comments, so the safety contract on its fields needs stating. PrivilegeError.Error() interpolates catalog identifiers with %s — role, schema, and database names. PostgreSQL identifiers can contain | and newlines, and SchemaBot renders verdict detail into markdown tables where either one breaks the layout. The right split already exists in the type and just isn't documented: Grant is built with pgx.Identifier{}.Sanitize() and is safe to echo; Check is display-only and its caller owns sanitizing it. Saying that in the PrivilegeError doc comment makes it a contract rather than an accident, and tells the SchemaBot side which field it must clamp before rendering. (SchemaBot has to do that clamping regardless — this just makes it obvious which field needs it.)

Two smaller things that read well from the integration side: PrivilegedRole being constructible only inside the package is the right shape for a proof that a later copy-and-swap path will want to demand rather than re-derive, and the cost is one extra round trip per change, which is nothing next to a schema change.

This review was generated by Claude Code (claude-opus-5).

[aparajon]

🤖 Approving. The tier ladder matches what PostgreSQL actually enforces — I verified rung by rung against a live server that Tier 2 is a real requirement and not an over-strict gate, and the refusal → GRANT → re-run loop closes end-to-end through the CLI. The two gaps I found (adversarial review, findings 1 and 2) are follow-ups rather than blockers: both shapes died mid-change before this PR too, so it strictly narrows the hole rather than opening one. Finding 1 is inside the contract this PR establishes, though, and I'd like it closed before the tier work is called done. Adoption and integration notes are in the second comment.

This review was generated by Claude Code (claude-opus-5).

[Kiran01bm]

Review response from Kiran's (@Kiran01bm) AI code review assessment agent (Amp, Claude Opus 4.6)

One-line summary: all five adversarial findings plus the pre-existing drive-by are fixed in the follow-up commit; the approval review's design notes are folded in where they overlap, with one display-only nit rejected.

Adversarial review (findings 1–5 + drive-by):

#	Finding	Status	Explanation
1	Index-building ALTER TABLE forms mapped to Tier 1, so --force dies mid-change instead of being refused	fixed	Tier now derives from the statement's operations: pkg/statement exposes BuildsIndex(), marking ADD CONSTRAINT UNIQUE/PRIMARY KEY/EXCLUDE without USING INDEX and ADD COLUMN with inline UNIQUE/PRIMARY KEY as Tier 2. The suggested repro cases are in TestRequiredTier, alongside the Tier-1 counterparts (USING INDEX, rewriting ALTER COLUMN ... TYPE) that must not regress.
2	Cross-owner FK needs REFERENCES on the referenced table; docs read as if covered	fixed (documented as out of contract)	docs/engine-role.md now states the contract covers the target table's own access, names the GRANT REFERENCES a cross-owner FK needs, and says the preflight does not check it. Scope decision: document, not check — the single-owning-role setup the page assumes never hits it.
3	Tier-0 CONNECT rung can't fire on a live connection	fixed	TierConnect's comment now says the CONNECT rung documents the contract rather than catching live failures, while the USAGE rung is the load-bearing one.
4	Tier-2 refusal's Check and Grant name different roles with no explanation	fixed	PrivilegeError gained a Hint; the Tier-2 refusal now says the grant targets the owning role, which the engine inherits through its Tier 1 membership.
5	docs/engine-role.md advertises two checks that don't exist (Tier-3 14–15 row, Tier-4 row)	fixed	Tier-3 row now explains the 14–15 early return (USAGE already proves SET ROLE pre-16); Tier-4 row marked not yet implemented, surfacing at scratch creation instead.
—	(drive-by) step-1 SequenceStepError claims earlier steps committed	fixed	Error() now says "no earlier steps had committed" for step 1, matching the verdict's Detail; pinned by a renderer unit test.

Approval review design notes:

#	Note	Status	Explanation
1	"Single snapshot" comment only holds for tiers 0–2	fixed	Comment now scopes the snapshot to the tier 0–2 facts and notes the SET-ROLE/replication probes are separate, still fail-closed queries.
2	TOCTOU between preflight and execution is inherent	no action	Agreed — inherent, and the PR makes no contrary claim.
3	CONNECT rung near-tautological	fixed	Same fix as adversarial finding 3.
4	requiredTier fails closed on future step shapes; Tiers 3–4 not yet CLI-reachable	no action	Intentional fail-closed posture; the tiers become reachable as the copy-and-swap path lands, and the mapping grows with the router.
5	(nit) PrivilegeError.Check interpolates names unquoted	rejected	Display-only by design — the executable Grant field is the sanitized artifact; quoting inside the has_*_privilege(...) rendering would misstate the actual probe.

Review response for the lens comment:

#	Note	Status	Explanation
L2.1	Tier derivation lives in internal/cli; embedders would re-derive it and reproduce the index-shape bug	fixed	Exported as preflight.RequiredTier(execSQL) — placed next to Tier and CheckPrivileges (which own the ladder) rather than literally next to the router, same outcome: decided once, tested once (pkg/preflight/tier_test.go), every consumer gets the same answer.
L1.1 / L2.2	--dry-run (and plan-time checks generally) can't surface an under-provisioned role; the privilege verdict only exists at apply time	deferred	Agreed on both stakes — evaluator first-run and PR-check-time remediation. Surfacing the ladder from --dry-run (or a dedicated preflight command) is tracked as follow-up work in the project tracker; RequiredTier + CheckPrivileges being both exported and side-effect-free is the enabling piece and landed here.
L2.3	Detail renders into PR comments; which PrivilegeError field is safe to echo needs stating as a contract	fixed	Doc comments now state it: Grant is Sanitize()-quoted and safe to echo verbatim as executable SQL; Check is display prose whose renderer owns escaping.
L1.5	Suite now needs cluster-level role creation on an external PG_DSN, undocumented; NewRole cleanup only logs on failure	fixed (docs) / rejected (cleanup)	docs/testing.md now documents the CREATEROLE requirement and when it applies. Cleanup stays log-not-fail by design: a drop failure on a shared server shouldn't fail an otherwise-green run, and process-unique names mean a leaked role can't collide with a later one.
L1.2	Tier-2 message asks the operator to grant to an unexpected role	fixed	Landed with the correctness follow-up: the refusal now explains the grantee is the owning role, inherited through Tier 1 membership.
L1.4	Tier 4 documented but unimplemented reads as capability	fixed	Landed with the correctness follow-up: the row is marked not yet implemented, surfacing at scratch creation instead.
L1.3	rds_replication gating on role existence is the right OSS shape	no action	Agreed — noted as the pattern for future managed-flavor quirks.