Skip to content

feat(dcb): gate materialized view activation on authoritative eligibility - #1118

Merged
tomohisa merged 3 commits into
mainfrom
claude/sek-g27-mv-active-eligibility
Aug 10, 2026
Merged

feat(dcb): gate materialized view activation on authoritative eligibility#1118
tomohisa merged 3 commits into
mainfrom
claude/sek-g27-mv-active-eligibility

Conversation

@tomohisa

Copy link
Copy Markdown
Contributor

Closes #1113. Adds authoritative target capture, typed fail-closed eligibility, four-provider expected active/generation CAS, no initialization auto-activation, initial activation through the same boundary, compatibility surfaces, race/zero-mutation tests, and EN/JA docs. Verification: full dcb solution net9/net10, MaterializedView unit/integration/Postgres suites, git diff --check.

@tomohisa tomohisa added intent-target Intent automation target — implementation worker may claim this issue. intent-pr-reviewing PR is currently under review by intent automation. labels Aug 10, 2026
@tomohisa

Copy link
Copy Markdown
Contributor Author

Request update — SEK-G27 semantic review at full head c3f370aea61adf4c041f514701f06dd3e8ae8a3e.

  1. The provider CAS does not fence the candidate snapshot through the complete transaction. The packet requires eligible compare-and-switch to be atomic in all four providers, concurrent or stale attempts to leave the former pointer unchanged, and provider failure to preserve the former pointer. In Postgres TryActivateInTransactionAsync, the first statement validates the candidate snapshot while changing sekiban_mv_active (lines 454-530), then markActiveSql updates the registry rows (lines 532-544), but its affected-row count is ignored and the method always returns success. MySQL lines 491-582, SQL Server lines 485-592, and SQLite lines 519-640 have the same two-step/result-discard shape. On providers where a registry row can change after the first statement read and before the second update, the second update can affect zero rows while the active pointer still commits. Lock/fence the candidate rows for the whole operation or otherwise make a candidate mismatch roll back every pointer mutation, including the overload that receives a caller transaction. Assert that exactly CandidateCount rows transition before returning success.

  2. The required four-provider race, failure, and zero-mutation proof is absent. The only new real store CAS test is SqliteRegistry_UsesExpectedGenerationAndLeavesPointerUnchangedForInvalidOrStaleRequests, and it performs two sequential calls against SQLite. The common multi-provider diff only asserts that initialization leaves the active pointer null. There is no concurrent same-generation test with exactly one winner, candidate-change interleaving test, all-four-provider invalid/bypass zero-mutation suite, or provider-failure plus deterministic-retry proof. Also, BehindCandidate_IsRejectedEvenWhenASeparateBestEffortStatusWouldSayCaughtUp assigns an unrelated local boolean; no production G24 observation reaches the activation path, so this is not the packet-required killing test. Add substantive common-provider tests covering Unknown, unsafe/faulted, wrong identity, stale target/expected pointer, actual concurrent winners, rollback/pointer preservation, initialization mutation, and a wired favorable G24 observation that still cannot authorize activation without authoritative truth.

Sonar triage: the 35.1% duplication is confined to the waived four-provider same-shape store/executor class (provider-neutral activation/contracts/base and Orleans grain report 0 duplicated new lines). No open vulnerability exists and security/reliability/maintainability ratings are A. The S8970 findings are non-semantic null-forgiving cleanup at constructor/transaction/host invariants. S3776 on MvActivationEligibility.Evaluate is a real maintainability signal for this authorization boundary (complexity 30); the straight fail-closed guards are readable, but the current branch coverage is far too sparse. Extracting per-entry checks and testing every typed rejection is the natural repair and should accompany finding 2.

Confirmed: initialization itself no longer calls SetActiveAsync; authoritative target capture and the typed evaluator exist; expected active version plus generation are carried; G24 status is absent from production authorization; EN/JA docs are present; no cloud/coordinator/rollback/heartbeat scope widening was found; and the PR body correctly closes #1113. Design alignment checked against SEK-G27 packet.yaml, implementation.md, review-context.md, github-body.md, issue #1113 In/Out of Scope and AC, intents/sekiban/intent-tree/00-map.md, and intents/sekiban/design/materialized-view-service-scope.md. Focused net9/net10 tests passed 6/6 each, but tests are necessary and not sufficient.

@tomohisa tomohisa added intent-pr-request-update Review requested updates to this PR; implementer must address. intent-pr-update-in-progress Implementer is updating this PR per review feedback. and removed intent-pr-reviewing PR is currently under review by intent automation. labels Aug 10, 2026
@tomohisa tomohisa added intent-pr-rereview-ready PR has been updated and is ready for re-review. intent-pr-reviewing PR is currently under review by intent automation. and removed intent-pr-request-update Review requested updates to this PR; implementer must address. intent-pr-update-in-progress Implementer is updating this PR per review feedback. intent-pr-rereview-ready PR has been updated and is ready for re-review. labels Aug 10, 2026
@tomohisa

Copy link
Copy Markdown
Contributor Author

SEK-G27 rereview at exact head a7ea9ac987c79b78d179e4462a796c6c8845c2e8: one implementation finding remains.

Packet AC: A killing test proves G24 best-effort status is not authoritative evidence. FavorableWiredG24Observation_CannotAuthorizeBehindCandidate now creates a real production ProjectionStatusReader observation with IsCaughtUp == true, but that observation is discarded before authorization: the test then calls MvActivationEligibility.Evaluate directly with a behind registry row. No executor or Orleans activation entry point receives or can observe the G24 result. Consequently, a mutation that makes MvExecutorBase or MaterializedViewGrain authorize cutover from favorable G24 status would not fail this test; only the independent static evaluator remains tested. This is improved setup, but it is still not the required production-path killing proof.

Required unblock: compose a real activation path (provider executor or actual grain/catch-up completion) in the same production-style container as the favorable G24 status, invoke that path with Unknown/behind authoritative truth, and assert the typed rejection plus zero active-pointer mutation. An architecture/dependency assertion may supplement this, but the test must fail if production activation starts treating G24 observation as authorization.

The original provider-CAS blocker is resolved: all four stores lock/fence candidate rows, compare the exact snapshot, require marked == CandidateCount, and roll local or caller-owned operations back through provider-native savepoints. The common suite is substantive and locally passed 24/24 with zero skips on both net9.0 and net10.0, covering same-generation one-winner concurrency, candidate interleaving, invalid/bypass zero mutation, provider failure/retry, caller savepoint rollback, and trigger-forced mark-count mismatch rollback. MvActivationEligibility.Evaluate is decomposed into per-entry/active-pointer checks; every evaluator rejection is covered, S3776 is gone, and only S8970 minor cleanup remains. Sonar reports vulnerabilities 0 and A security/reliability/maintainability; 29.5% duplication remains confined to the standing-waiver four-provider store/executor shape.

All other packet AC and prior findings remain satisfied by semantic inspection; no scheduling/routing/rollback/G24-publication/cloud scope widening was found, EN/JA docs remain aligned, git diff --check passes, and Closes #1113 is correct. design_alignment_checked=true: SEK-G27 packet.yaml, implementation.md, review-context.md, github-body.md, issue #1113 In/Out of Scope and AC, prior review comment 5245149411, intents/sekiban/intent-tree/00-map.md, and intents/sekiban/design/materialized-view-service-scope.md. Tests are necessary but not sufficient.

@tomohisa tomohisa added intent-pr-request-update Review requested updates to this PR; implementer must address. intent-pr-update-in-progress Implementer is updating this PR per review feedback. and removed intent-pr-reviewing PR is currently under review by intent automation. labels Aug 10, 2026
@tomohisa tomohisa added intent-pr-rereview-ready PR has been updated and is ready for re-review. and removed intent-pr-request-update Review requested updates to this PR; implementer must address. intent-pr-update-in-progress Implementer is updating this PR per review feedback. labels Aug 10, 2026
@sonarqubecloud

Copy link
Copy Markdown

Quality Gate Failed Quality Gate failed

Failed conditions
28.6% Duplication on New Code (required ≤ 3%)

See analysis details on SonarQube Cloud

@tomohisa
tomohisa marked this pull request as ready for review August 10, 2026 21:04
@tomohisa
tomohisa merged commit 5b41fb8 into main Aug 10, 2026
3 of 4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

intent-pr-rereview-ready PR has been updated and is ready for re-review. intent-target Intent automation target — implementation worker may claim this issue.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

SEK-G27 dcb materialized view: gate active-version cutover on authoritative eligibility

1 participant