feat(standards): systemplane in MT mode — compliance pattern (§27)#406
Conversation
WalkthroughThis PR adds a mandatory "Systemplane in MT mode — compliance pattern" section and reviewer gates, updates severity examples for systemplane non-compliance, and inserts index and skill references for discoverability across standards and skills docs. ChangesMulti-Tenant Systemplane Compliance Standards
Comment |
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@dev-team/agents/multi-tenant-reviewer.md`:
- Line 58: The current detection uses grep -A 5 and can miss
AssignString/AssignBool/AssignInt/AssignInt64 occurrences that fall outside that
window; replace the fragile line-window approach with a block-aware multiline
match that captures the entire registration block for RuntimeClass:
systemplaneKeyRuntimeClassReadLive and then scan that block for
AssignString|AssignBool|AssignInt|AssignInt64. Concretely, change the check to
extract blocks starting at the token RuntimeClass:
systemplaneKeyRuntimeClassReadLive using a multiline/PCRE-aware command (eg.
grep -Poz 'RuntimeClass:
systemplaneKeyRuntimeClassReadLive(?:(?!RuntimeClass:).)*' or use awk/sed/yq to
pull the whole block) and then assert that a follow-up grep -E
'AssignString|AssignBool|AssignInt|AssignInt64' returns zero matches for each
extracted block.
In `@dev-team/docs/standards/golang/index.md`:
- Line 184: Update the Module Index entry for multi-tenant.md to reflect the
added section: change the "2 sections" count to "3 sections" in the Module Index
row for multi-tenant.md so the metadata stays in sync with the new section 27
(Systemplane in MT mode — compliance pattern).
In `@dev-team/docs/standards/golang/multi-tenant.md`:
- Line 2689: The internal anchor target
"`#tenant-discovery-and-cache-invalidation`" referenced in the sentence starting
"Use the existing event-driven discovery pattern documented in §[Tenant
Discovery and Cache Invalidation](`#tenant-discovery-and-cache-invalidation`)" is
invalid; either update that link to point to the correct existing heading anchor
(replace "`#tenant-discovery-and-cache-invalidation`" with the actual anchor for
the Tenant Discovery and Cache Invalidation section) or add a matching heading
"Tenant Discovery and Cache Invalidation" (which will create the
"`#tenant-discovery-and-cache-invalidation`" anchor) so the reference from §27
resolves correctly.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 829b4a45-651a-4bcb-9963-e83d65e3ae3a
📒 Files selected for processing (5)
dev-team/agents/multi-tenant-reviewer.mddev-team/docs/standards/golang/index.mddev-team/docs/standards/golang/multi-tenant.mddev-team/skills/dev-systemplane-migration/SKILL.mddev-team/skills/shared-patterns/standards-coverage-table.md
| | S3 Isolation | `s3.GetS3KeyStorageContext(ctx, key)` — no raw S3 keys | | ||
| | M2M Credentials | `secretsmanager.GetM2MCredentials` per tenant — NEVER env vars | | ||
| | Backward Compat | `MULTI_TENANT_ENABLED=false` → single-tenant mode preserved | | ||
| | Systemplane registration shape | Padrão A only: `ReadLive` keys MUST drop `Reads`/`AssignX`. Detect: `grep -A 5 "RuntimeClass: systemplaneKeyRuntimeClassReadLive" service_systemplane.go` followed by `grep "AssignString:\|AssignBool:\|AssignInt:\|AssignInt64:"` MUST return zero matches. | |
There was a problem hiding this comment.
Detection command can miss AssignX violations due to narrow context window.
Using grep -A 5 may skip Assign* fields when they appear later in the block, creating false negatives in reviewer enforcement. Use block-aware matching or a wider/multiline pattern so all ReadLive registrations are validated.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dev-team/agents/multi-tenant-reviewer.md` at line 58, The current detection
uses grep -A 5 and can miss AssignString/AssignBool/AssignInt/AssignInt64
occurrences that fall outside that window; replace the fragile line-window
approach with a block-aware multiline match that captures the entire
registration block for RuntimeClass: systemplaneKeyRuntimeClassReadLive and then
scan that block for AssignString|AssignBool|AssignInt|AssignInt64. Concretely,
change the check to extract blocks starting at the token RuntimeClass:
systemplaneKeyRuntimeClassReadLive using a multiline/PCRE-aware command (eg.
grep -Poz 'RuntimeClass:
systemplaneKeyRuntimeClassReadLive(?:(?!RuntimeClass:).)*' or use awk/sed/yq to
pull the whole block) and then assert that a follow-up grep -E
'AssignString|AssignBool|AssignInt|AssignInt64' returns zero matches for each
extracted block.
|
|
||
| This section captures the asymmetry in `lib-systemplane v1.4.0` (cache + push hot-reload disabled in MT) and the consumer-side pattern that makes ST and MT behave identically from the caller's perspective until `v1.5.0` ships the `Manager` and closes the asymmetry internally. | ||
|
|
||
| **Tenant cache and event listener are NOT in scope here.** Use the existing event-driven discovery pattern documented in §[Tenant Discovery and Cache Invalidation](#tenant-discovery-and-cache-invalidation) (`TenantCache` + `TenantEventListener` + `EventDispatcher` over `tenant-events:*` Pub/Sub) without modification. The systemplane patterns below sit on top of that infrastructure. |
There was a problem hiding this comment.
Fix broken internal anchor in §27 reference.
The link target #tenant-discovery-and-cache-invalidation does not exist in this file, so navigation from §27 breaks. Point it to an existing heading anchor (or add a matching heading).
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@dev-team/docs/standards/golang/multi-tenant.md` at line 2689, The internal
anchor target "`#tenant-discovery-and-cache-invalidation`" referenced in the
sentence starting "Use the existing event-driven discovery pattern documented in
§[Tenant Discovery and Cache
Invalidation](`#tenant-discovery-and-cache-invalidation`)" is invalid; either
update that link to point to the correct existing heading anchor (replace
"`#tenant-discovery-and-cache-invalidation`" with the actual anchor for the Tenant
Discovery and Cache Invalidation section) or add a matching heading "Tenant
Discovery and Cache Invalidation" (which will create the
"`#tenant-discovery-and-cache-invalidation`" anchor) so the reference from §27
resolves correctly.
Adds a new MANDATORY section to dev-team/docs/standards/golang/multi-tenant.md capturing the consumer-side pattern for using lib-systemplane in multi-tenant deployments. Derived from the plugin-br-bank-transfer D9 initiative. Covers: - Registration shape (Padrão A): ReadLive keys MUST drop Reads/AssignX. Legacy Padrão B with these fields still attached is non-compliant going forward. - Consumer reads: spClient.GetX(ctx) only. No cfg.X runtime fallback. No ST/MT branching. Same code path works in both modes — ST relies on the lib's in-process cache, MT relies on the seed migration (or the v1.5.0 Manager once bound). - Interface DI: narrow per-consumer interfaces (WebhookKnobsReader, ReconciliationAlertThresholdReader, UsageLimitsReader, etc.) that *systemplane.Client satisfies implicitly. Keeps adapter packages free of lib-systemplane import; only the bootstrap layer wires the concrete client. - Cold-tenant resolution: versioned SQL seed migration (000NNN_systemplane_defaults_seed.up.sql) mirroring the BACEN holidays seed pattern, plus a check-systemplane-seed drift guard wired into make check-generated-artifacts. - Manager binding (lib-systemplane v1.5.0+): Client.BindManager pattern and the four OnTenant* lifecycle handlers wired into the consumer's existing handler wrapper (not via a lib-commons EventDispatcher option — that approach was explicitly rejected during design). - ST↔MT symmetry awareness: pinned via the comparison table for the three states (ST any version, MT v1.4.0, MT v1.5.0+). - Anti-Rationalization table with six common rationalizations and the required actions. FOUR-FILE UPDATE RULE cycle: 1. dev-team/docs/standards/golang/multi-tenant.md — new §27 (215 lines) 2. dev-team/docs/standards/golang/index.md — TOC entry added 3. dev-team/skills/shared-patterns/standards-coverage-table.md — §58 4. dev-team/agents/multi-tenant-reviewer.md — Focus Areas (3 new rows) and Severity table (CRITICAL/HIGH/MEDIUM additions) Bonus: dev-team/skills/dev-systemplane-migration/SKILL.md — cross-ref to the new §27 for MT services. Tenant cache + event listener patterns (existing §[Tenant Discovery and Cache Invalidation]) are explicitly NOT in scope — already documented and enforced. The new section sits on top of that infrastructure.
The plugin-br-bank-transfer dropped scripts/check-systemplane-seed.sh and make check-systemplane-seed in 5fb8131b (PR #171) — migration cadence is the canonical convention for schema/seed changes, and the meta-script enforcing "every registration has a seed row" was deemed over-engineering. Aligns the Ring standard with that decision: - multi-tenant.md §27: removes the drift-guard requirement from the cold-tenant resolution subsection and from the NON-COMPLIANT signs list. Drops the matching Anti-Rationalization row. - multi-tenant-reviewer.md: rewords the Focus Areas row to require only the migration (or v1.5.0 Manager binding). Drops the HIGH severity bullet referring to the missing drift guard. - standards-coverage-table.md: §58 description no longer mentions the drift guard. Seed migration (000NNN_systemplane_defaults_seed.up.sql) and v1.5.0 Manager binding remain the two MANDATORY paths for cold-tenant resolution in MT.
Standards that hardcode lib versions (v1.4.0, v1.5.0) go stale on every
minor bump. The architectural patterns (Padrão A, compliance reads,
migration seed, Manager binding) are version-agnostic; only the exact
API surface depends on the version the service is pinned to.
Rewrite §27 and related files (multi-tenant-reviewer.md,
standards-coverage-table.md, dev-systemplane-migration/SKILL.md) to:
- Strip explicit v1.4.0 / v1.5.0 references.
- Add a "Version discovery (mandatory for agents)" paragraph at the top
of §27 directing readers to consult the service's go.mod and the
lib's latest release / CHANGELOG.
- Reword version-pinned guidance ("Manager binding once on v1.5.0+") to
version-agnostic phrasing ("Manager binding once available in the
pinned lib version").
No semantic change to the patterns themselves. Architectural content
intact.
4563b22 to
03abf23
Compare
There was a problem hiding this comment.
♻️ Duplicate comments (3)
dev-team/docs/standards/golang/multi-tenant.md (1)
2691-2691:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winBroken internal anchor — use
#event-driven-tenant-discovery.The link
#tenant-discovery-and-cache-invalidationdoes not match any heading in this file. The correct anchor is#event-driven-tenant-discovery(line 871).🔗 Proposed fix
-This section captures the historical asymmetry inside earlier `lib-systemplane` releases (cache + push hot-reload disabled in MT) and the consumer-side pattern that makes ST and MT behave identically from the caller's perspective regardless of which lib version a service runs. The asymmetry is closed by the `Manager` API; consult the latest [`lib-systemplane` release](https://github.com/LerianStudio/lib-systemplane/releases/latest) and its CHANGELOG to confirm which version your service is on and whether `Client.BindManager` is available. +**Tenant cache and event listener are NOT in scope here.** Use the existing event-driven discovery pattern documented in §[Event-Driven Tenant Discovery](`#event-driven-tenant-discovery`) (`TenantCache` + `TenantEventListener` + `EventDispatcher` over `tenant-events:*` Pub/Sub) without modification. The systemplane patterns below sit on top of that infrastructure.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dev-team/docs/standards/golang/multi-tenant.md` at line 2691, The internal anchor in the sentence referencing Tenant Discovery uses the wrong fragment `#tenant-discovery-and-cache-invalidation`; update it to the correct anchor `#event-driven-tenant-discovery` so the link resolves; locate the text containing the incorrect fragment string and replace `#tenant-discovery-and-cache-invalidation` with `#event-driven-tenant-discovery` (the relevant context mentions TenantCache, TenantEventListener, and EventDispatcher).dev-team/docs/standards/golang/index.md (1)
62-62:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate section count for multi-tenant.md after adding §27.
The Module Index shows "2 sections" for multi-tenant.md, but with the new §27 entry the total should be "3 sections."
📝 Proposed fix
-| 11 | [multi-tenant.md](multi-tenant.md) | 2 | ~2240 | Multi-Tenant Patterns (Tenant Manager, JWT), Auth-Before-Tenant Ordering | +| 11 | [multi-tenant.md](multi-tenant.md) | 3 | ~2500 | Multi-Tenant Patterns (Tenant Manager, JWT), Auth-Before-Tenant Ordering, Systemplane MT Compliance |🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dev-team/docs/standards/golang/index.md` at line 62, Update the Module Index row for multi-tenant.md to reflect the new section count: change the "2" sections value to "3" in the table entry that currently reads "| 11 | [multi-tenant.md](multi-tenant.md) | 2 | ~2240 | Multi-Tenant Patterns (Tenant Manager, JWT), Auth-Before-Tenant Ordering |" so the second numeric column shows 3 instead of 2; ensure only that cell is modified (keep filename, size and description identical).dev-team/agents/multi-tenant-reviewer.md (1)
58-58:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDetection command may miss
AssignXviolations outside 5-line window.Using
grep -A 5with a fixed context can produce false negatives whenAssignString/AssignBool/AssignInt/AssignInt64appear beyond line 5 of the registration block. Use block-aware multiline matching or a wider pattern.🔍 Improved detection approach
# Extract entire registration blocks containing RuntimeClass: systemplaneKeyRuntimeClassReadLive, # then search each block for AssignX violations: grep -Pzo '(?s)\{[^}]*RuntimeClass:\s*systemplaneKeyRuntimeClassReadLive[^}]*\}' service_systemplane.go | \ grep -E 'Assign(String|Bool|Int|Int64):' && echo "VIOLATION: Padrão B detected" || echo "OK: Padrão A only"Alternatively, use
ast-grepfor structural matching of the entire registration initializer.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@dev-team/agents/multi-tenant-reviewer.md` at line 58, The current detection uses a fixed-context grep "-A 5" which can miss AssignString/AssignBool/AssignInt/AssignInt64 occurrences outside that window; update the check in service_systemplane.go's registration validation that targets blocks containing "RuntimeClass: systemplaneKeyRuntimeClassReadLive" to instead extract the entire registration/initializer block and then search that whole block for any AssignString/AssignBool/AssignInt/AssignInt64 tokens (or replace the grep-based check with an AST/structural match like ast-grep); ensure the new matcher operates per-block so any AssignX anywhere in the RuntimeClass: systemplaneKeyRuntimeClassReadLive registration is flagged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Duplicate comments:
In `@dev-team/agents/multi-tenant-reviewer.md`:
- Line 58: The current detection uses a fixed-context grep "-A 5" which can miss
AssignString/AssignBool/AssignInt/AssignInt64 occurrences outside that window;
update the check in service_systemplane.go's registration validation that
targets blocks containing "RuntimeClass: systemplaneKeyRuntimeClassReadLive" to
instead extract the entire registration/initializer block and then search that
whole block for any AssignString/AssignBool/AssignInt/AssignInt64 tokens (or
replace the grep-based check with an AST/structural match like ast-grep); ensure
the new matcher operates per-block so any AssignX anywhere in the RuntimeClass:
systemplaneKeyRuntimeClassReadLive registration is flagged.
In `@dev-team/docs/standards/golang/index.md`:
- Line 62: Update the Module Index row for multi-tenant.md to reflect the new
section count: change the "2" sections value to "3" in the table entry that
currently reads "| 11 | [multi-tenant.md](multi-tenant.md) | 2 | ~2240 |
Multi-Tenant Patterns (Tenant Manager, JWT), Auth-Before-Tenant Ordering |" so
the second numeric column shows 3 instead of 2; ensure only that cell is
modified (keep filename, size and description identical).
In `@dev-team/docs/standards/golang/multi-tenant.md`:
- Line 2691: The internal anchor in the sentence referencing Tenant Discovery
uses the wrong fragment `#tenant-discovery-and-cache-invalidation`; update it to
the correct anchor `#event-driven-tenant-discovery` so the link resolves; locate
the text containing the incorrect fragment string and replace
`#tenant-discovery-and-cache-invalidation` with `#event-driven-tenant-discovery`
(the relevant context mentions TenantCache, TenantEventListener, and
EventDispatcher).
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: dca5b5f0-c6cb-40e0-b7e3-f4ac55896c7c
📒 Files selected for processing (5)
dev-team/agents/multi-tenant-reviewer.mddev-team/docs/standards/golang/index.mddev-team/docs/standards/golang/multi-tenant.mddev-team/skills/dev-systemplane-migration/SKILL.mddev-team/skills/shared-patterns/standards-coverage-table.md
§27 (added in PR LerianStudio#406) showed `client.BindManager(manager)` as a separate call. The real public API has no exported BindManager — NewManager(client, pgMgr, opts...) binds the Manager to the Client internally. A service copying the §27 snippet verbatim would not compile. Correct the wiring snippet and prose in multi-tenant.md §27, the multi-tenant-reviewer agent, and the coverage table. Also fix the lifecycle-handler event field/constants to event.EventType + tmevent.EventTenant* (verified against lib-commons), matching what the plugin-br-bank-transfer Phase 6 implementation actually used. Architectural patterns unchanged; this is an API-accuracy fix so other services follow a snippet that compiles.
Summary
New MANDATORY standards section capturing the consumer-side pattern for using lib-systemplane in multi-tenant deployments. Derived from the D9 initiative in `plugin-br-bank-transfer` (PR #171 + lib-systemplane PRs #12/#13).
Closes the gap where Ring's multi-tenant standard covered tenant cache, event listener, message-queue consumers, M2M credentials, and service auth — but did not cover the per-tenant systemplane integration pattern that is critical once `MULTI_TENANT_ENABLED=true` is paired with `lib-systemplane` for runtime configuration.
What's in §27
FOUR-FILE UPDATE RULE cycle
Bonus: `dev-team/skills/dev-systemplane-migration/SKILL.md` cross-refs the new §27 for MT services.
NOT touched (already adequate)
The existing Tenant Discovery and Cache Invalidation section (`tenantcache` + `TenantEventListener` + `EventDispatcher` over `tenant-events:*` Pub/Sub) is unchanged. The new §27 sits on top of that infrastructure and explicitly references it.
Test plan
Origin