[gouhongshen]

What type of PR is this?

• API-change
• BUG
• Improvement
• Documentation
• Feature
• Test and CI
• Code Refactoring

Which issue(s) this PR fixes:

Related to #30955

Compose BVT session-state repair (2026-10-29)

The Matrixone Compose CI execution / multi cn e2e bvt test docker compose(PROXY) job of CI run 40752152958 failed at Start BVT Test with 28 comparison diffs, all downstream of one primary error: not supported: object lifecycle statements require an existing pessimistic RC transaction (the upstream object-lifecycle gate in pkg/frontend/txn.go, in-branch since a1220de) fired on DDL inside an active BEGIN. The job's own log pins the sequence: select_for_update_rr_current_read.sql ran green (142/142, cost 0.894s), then special.sql failed one second later at row 25 (create external table inside BEGIN) on mo-tester's con[id=0], and the refused DDL cascaded through the rest of the group (5 diffs in special.sql, 13 in transaction_enhance.sql, 2 each in ww_conflict.sql / unique_secondary_index.sql, and 6 show databases diffs in publication_subscription caused by the leftover tdb database whose drop was refused).

Root cause — a leak introduced by this PR's own new BVT case, not a production change: mo-tester keeps ONE default-session connection for the whole run (ConnectionManager.connections[0] is never reset between case files; Executor.run only closes the named @session connections), and the new case ended on set session transaction isolation level repeatable read. In the name-sorted case ordering this file runs directly before special.sql, so every later case of the same run inherited a REPEATABLE READ default session, whose in-BEGIN DDL then hits the gate. Cross-checks: the same PROXY job was green for three sibling PR branches within the same two hours (runs 40741161328, 40743062016, 40737505099); the same run's other BVT group (1081 scripts, no pessimistic_transaction case) was green; and the suite already carries the restore convention set session transaction_isolation=default; in locking_read_limit_retry.sql — this PR's case did not.

Repair (test-only): select_for_update_rr_current_read.sql now closes with set session transaction_isolation=default;, restoring the shared default session to the engine and CI default (pessimistic READ-COMMITTED), exactly the suite's established convention. No production code changed and the case's own statements are untouched; no other session-state leak remains in the case (its lock_wait_timeout set lives inside a named @session block, which mo-tester closes at file boundaries, and the case uses no user variables).

Verified locally with the real runner, the current-head binary (mo-service built at 03d581cd7b — the production code of the new head, this repair adds only test-case statements; isolated 39xxx port plan, owned verified teardown, no leftover listeners):

• mo-tester replay of the exact CI two-file ordering with the pre-fix case (run.sh -m run -r 100 -i select_for_update_rr_current_read,special, isolated case report): the new case passes while special.sql fails with the same five diffs as CI — rows 25/26/31/32/34, first the byte-identical gate error — reproducing the CI failure locally with the same runner (summary: 166 statements, 161 success, 5 failed).
• After the one-statement repair, the same replay on a fresh cluster reports both files green: 167 statements, 167 success, 0 failed, no error blocks (runner exit 0).
• The remaining affected suites (transaction_enhance, ww_conflict, unique_secondary_index, publication_subscription) are further downstream of the same single cause and see exactly the same restored session state; the fresh CI on the new head re-verifies the full compose BVT matrix.

Related to #30955

SCA license-header repair (2026-10-29)

The SCA Test on Linux/arm64 job of CI run 40734338128 failed at its license-eye header check on five files added by this PR:

• Four files carried no license header at all: pkg/sql/compile/execution_read_test.go, pkg/sql/compile/locking_current_read_capability.go, pkg/txn/client/execution_read_test.go, pkg/vm/engine/disttae/execution_read_test.go.
• One file carried a corrupted header: pkg/sql/plan/locking_current_read.go — the canonical wording "you may not use this file except in compliance with the License. You may obtain a copy of the License at" had collapsed into "you may not obtain a copy of the License at", and the URL line was indented with five instead of six spaces.

All five files now carry the repository's canonical Apache-2.0 header, byte-identical to the header the same CI check accepts in the sibling files of this PR. This is a comment-only change: no behavior, wire-format, or test-contract change. The other sub-checks of the same job had not run when the header check failed (the orchestration runs license-eye header check before make static-check-analysis), so nothing else needed root-cause work.

Verified locally at 7a61ffd:

• license-eye -c .licenserc.yml header check — the exact tool (license-eye v0.4.0 per install-static-check-tools) and configuration the failing job uses — exits 0 with invalid: 0 over the full tree.
• go vet green and gofmt clean on the linked five touched packages.
• The 94 focused regression tests of the rebase validation at f3dcd7284a remain valid evidence: this repair edits comment lines only, so no test was re-run for behavior; fresh CI will re-execute the full SCA suite on the new head.

Related to #30955

UT oracle update for issue 30245's RR wait members (2026-10-29)

The Matrixone UT Coverage execution / UT Coverage on Ubuntu/x86 job failed on TestIssue30245LockingReadRetriesBeforeLimit's RR_without_limit and RR_with_limit submembers (pkg/tests/issues/issue_30245_test.go, added upstream by #30268 on 2026-10-15): An error is expected but got nil. Both submembers still asserted the pre-#30955 corner in which a REPEATABLE READ locking read whose lock wait resolved behind a committed holder failed with 20619 w-w conflict: SI not support retry. The identical two-submember failure is recorded at both CI heads of this PR (run 40734338128 at f3dcd7284a and run 40741374466 at 7a61ffd16d, which differ only in comment lines), so it is deterministic and follows from this PR's sanctioned semantic change, not from flakiness.

This PR's approved design (D-5, rev-13) replaces exactly that corner for admitted locking SELECT statements: the marked statement revalidates at lock acquisition, a mid-wait commit is current-read through the tokenized ErrTxnNeedRetryLockingRead retry floor, and the reader returns the committed value with no error. TestPromotedShareCurrentReadAfterMidWaitCommitUnderRR already asserts that contract on the promoted-share shape; declined and engaging-historical shapes keep the legacy 20619 semantics and their tests stayed green in the same runs.

Repair (test-only): the two RR submembers now assert the delivered contract - no error, the mid-wait committed value (1) and a clean read-transaction commit - and the now-unused github.com/go-sql-driver/mysql import was removed. No production code changed and no assertion was weakened: the old oracle is exactly the complement of the new one, and the declined-shape members in the same suite still pin the legacy conflict behavior.

Verified locally:

• Reproduction at 7a61ffd16d: TestIssue30245LockingReadRetriesBeforeLimit fails with An error is expected but got nil on both RR submembers (exit 1), byte-identical to the CI reports of runs 40734338128 and 40741374466.
• After the oracle update at the amended head: the same test passes cleanly (all 11 submembers, including RR_without_limit and RR_with_limit; ok github.com/matrixorigin/matrixone/pkg/tests/issues).
• go vet green and gofmt clean on pkg/tests/issues.

Related to #30955

What this PR does / why we need it:

Root cause. Under REPEATABLE READ, MatrixOne's locking reads (SELECT ... FOR UPDATE, FOR SHARE, LOCK IN SHARE MODE) took the row lock but still read from the transaction snapshot, so a row committed by another transaction between this transaction's snapshot and the lock acquisition was served stale. The InnoDB-compatible behavior is a current read at lock acquisition: the locking read returns the latest committed value (r2_forupd = 999 in the issue's reproduction) while ordinary reads keep the snapshot (r1_snap = r3_snap = 100). The defect broke optimistic-concurrency-control patterns and multi-CN cross-node locking.

What changed. This implements the locking-current-read family from the approved design docs/design/20261024-locking-read-current-read-under-repeatable-read.md in this repository:

• pkg/sql/plan — bind-time per-occurrence presence plan.LockTarget.locking_read (additive fields; deep-copy preserved) plus the additive explicit-snapshot provenance pair on the target, explicit scan-read roles, whole-statement closure decline, and index-parent admission.
• pkg/sql/compile — per-execution source admission (an occurrence whose explicit snapshot engages the historical cohort for this execution, H nonzero and older than the executing SnapshotTS, stays historical exactly as today), the pre-pipeline attempt stamp that publishes the read frontier AND resolves each occurrence's effective marked activation against the executing transaction (the same engagement predicate admission uses), staged promoted pre-pipeline lock options intersected with per-execution admission, and the same resolution for runtime-created dependent LockOps.
• pkg/sql/colexec/lockop — per-occurrence marked revalidation: presence, the raw bind-time H and the resolved activation travel together through construction, the attempt stamp, the parallel dup, the additive wire fields and the remote rebuild, and the retained-pipeline Reset; an admitted marked occurrence revalidates against the stamped frontier and a mid-wait commit is served by the reader's current read, while an engaging historical occurrence keeps the legacy terminal SI not support retry w-w conflict, never receives the marked wire fields in that execution, and re-attaches on later executions that no longer engage.
• pkg/vm/engine/disttae, pkg/vm/engine, pkg/frontend, pkg/txn/client — the execution-read carrier and cache entry, per-execution retirement, transaction-snapshot immutability, and the tokenized retry family; RC behavior, DML lock targets, definition-fence and SI write-write aborts stay byte-identical.

Issue-to-test proof (same member RED on the parent tree, green after the fix; all receipts archived beside the worktree):

• pkg/tests/issues/issue_30955_promoted_share_test.go — deterministic embedded end-to-end members: the issue's two-session FOR UPDATE reproduction (ordinary read 100, locking read 999, snapshot preserved), the promoted LOCK IN SHARE MODE current read on a mid-wait commit, the mixed historical(=old)/current join surfacing the legacy terminal conflict on the HISTORICAL occurrence's wait while ordinary occurrences of the same statement keep the current read (RED pre-fix: the mixed statement silently returned the historical row), the historical-only promoted lock keeping the legacy conflict, and the same-table scalar-subquery occurrence keeping its legacy conflict.
• pkg/sql/colexec/lockop/lock_op_historical_exempt_rev24_review_test.go — the per-execution exemption contract at the component level: exempt / ordinary / non-engaging / no-presence profiles, the wire fold, Reset retention with next-attempt re-resolution, the later-execution re-attach flip and reverse, and the dup-path carriage.
• Focused compile/lockop families pin the staged promoted options, the statement admission and closure gates, the retry tokens and definition fences, the per-occurrence mark identity through construct/stamp/Reset/wire, deep-copy provenance and the dependent-kernel runtime stamp.
• test/distributed/cases/pessimistic_transaction/select_for_update_rr_current_read.sql — the SQL-surface ordered two-session case (ordinary read 100, locking read 999, subsequent ordinary read 100).

Test commands (mo-cgo-test = the repository CGo test wrapper, from the worktree root):

CGO_CFLAGS="-I$WT/cgo -I$WT/thirdparties/install/include" GOWORK=off \
  mo-cgo-test -count=1 -timeout=900s -run '^TestMixedHistoricalShareKeepsLegacyConflictOnHistoricalTargetUnderRR$' ./pkg/tests/issues/
mo-cgo-test -count=1 ./pkg/sql/colexec/lockop/
mo-cgo-test -count=1 ./pkg/sql/compile/
CGO_CFLAGS=... GOWORK=off go test -mod=readonly -race -count=1 -timeout=1200s \
  -run '^TestHistoricalOccurrenceExemptFromMarkedRevalidationUnderStampedAttempt$' ./pkg/sql/colexec/lockop/

Pre-fix RED receipt: --- FAIL: TestMixedHistoricalShareKeepsLegacyConflictOnHistoricalTargetUnderRR ... the historical target inherited the marked revalidation and the mixed statement silently returned 100; post-fix the same member surfaces the legacy conflict and the eligible current-read controls stay green. Owning-package normals and owning-package -race runs are green for both changed packages (pkg/sql/compile, pkg/sql/colexec/lockop); the wider pkg/tests/issues package suite remains for CI.

Residual risks.

• Wire/persisted compatibility: under the delivered single-version deployment contract (D-7, shutdown-only upgrade) there is no mixed-version rollout to risk. The additive plan/pipeline/sharding fields are plain-proto-safe (an absent field decodes to an unmarked reader exactly as today), and the shipped per-target activation carries strictly less for engaging historical occurrences (they no longer ship marked wire fields in that execution — an additive-value narrowing, no format change).
• LATERAL/optimized-away edge shapes without a base scan target keep today's coverage gap (no lock target is constructed, unchanged).
• The { mo_ts } historical e2e fixtures use seconds-scale settle margins; pathological TSO skew beyond seconds could interfere with the fixture but not the production path.
• The wider pkg/tests/issues and BVT suites are CI-owned for the full matrix.

Rebase onto main (2026-10-28)

Rebased onto 9e29011e73 to clear the reported conflict with main; the failed Mergify Rule: Auto update branch (update) was the same conflict, so both reported events are resolved by this rebase. Files touched by both sides were resolved semantically (both sides' changes kept, no side discarded wholesale):

• pkg/pb/pipeline / pkg/pb/plan: bindings regenerated from the merged proto sources with the repository toolchain (protoc 3.21.1 + protoc-gen-gogofast + the plan_string_literal_form / plan_collation_admission post-processors). Upstream's ODKU/result-summary (fix(sql): separate ODKU OK insert ID from session baseline #28444), integer-widening (fix: widen integer addition and multiplication #31088) and decimal-rounding (Fix decimal rounding and MySQL text-to-float casts #31191) fields are present together with this PR's additive LockTarget.locking_read/frontier pair and ScanReadRole (both sides verified by symbol in the generated output).
• pkg/defines/const.go: the locking current-read gate stays at MORPCVersion135; current main's protocol high-water was re-verified still 134 at the rebase target, and the allocation comment records this re-validation. (Historical state of that intermediate head — R15-F1 below removes this gate, its constant and the bump entirely; the delivered head keeps no locking-read MORPCVersion constant.)
• pkg/sql/compile/compile.go isRetryErr: keeps this PR's marked-locking retry admission (tokenized driver + shared retry codes under SI for marked statements); upstream fix(hnsw): search the index while it lags instead of falling back to a full table scan #30989's retirement of the ErrVectorIndexSnapshotChanged retry is honored (kept retired, not reintroduced).
• pkg/common/moerr/error.go: this PR's NewTxnNeedRetryLockingRead / GetTxnNeedRetryLockingReadFrontier kept; upstream refactor: retire unused mocks, helpers and obsolete scaffolding #31011's retirement of the then-unused NewTxnCannotRetry kept dropped (this PR does not use it).
• pkg/sql/compile/compile2.go, pkg/sql/plan/query_builder.go, pkg/sql/plan/types.go: both sides' additive statements retained in sequence — upstream's incomplete-temporal-string validators, savedHeadings handling and isODKU field together with this PR's then-shipped validateLockingCurrentReadCapability gates (removed in full by R15-F1 below) and viewInheritLocking provenance field.
• gofmt-only realignment of the MORPCVersion const block (mechanical whitespace).

Validation at the rebased head f3dcd7284a:

• Rebuilt the pinned usearch native artifact from the tarball shipped by current main (make usearch under thirdparties/; untracked install/ only — no tracked-file change), clearing a stale pre-rebase native-header mismatch (usearch_filtered_search 9-arg vs 10-arg).
• GOWORK=off go build -mod=readonly ./... with the repository CGo flags — green (full log kept locally).
• go vet green and gofmt clean across all 17 changed package closures of the commit.
• All 94 focused regression tests of this PR re-run green at the rebased head via mo-cgo-test (per-package -run selections equal to this PR's added-test inventory): pkg/sql/compile 44 (verbose per-test pass list kept), pkg/sql/plan 11, pkg/sql/colexec/lockop 12, pkg/txn/client 1, pkg/vm/engine/disttae 11, pkg/frontend 2, pkg/tests/issues 5 (the embedded end-to-end reproduction members), pkg/vm/engine/test 8 (compaction/truncated-history members). No test was weakened or skipped.

Related to #30955

Related to #30955

Design revision 15 — single-version contract, marked-retry persistence (2026-10-29)

PR review (aunjgr's design-gate review plus the repeated mo-bot P1/P2 findings) requested a design revision before implementation approval, for two verified findings:

• R15-F1 (design blocker): revision 14's cluster capability gate (the newly allocated MORPCVersion135 + MORPCLatestVersion bump, ClusterHasLockingCurrentRead at bind time, the validateLockingCurrentReadCapability per-Run rejection, the session-cache publish/evict guards and the gate/transition test members) violates the repository's upgrade contract — .agents/skills/mo-dev/references/upgrade-compatibility.md: MatrixOne supports shutdown upgrade only; peers in one cluster always run the same binary; a per-feature MORPCVersion constant, capability gate, legacy below-gate plans or downgraded behavior are prohibited; adding fields is never a reason to bump. The design therefore removes the entire mixed-version machinery.
• R15-F2 (P1): the delivered marked-SI SKIP LOCKED path could lose an already-recorded current-read retry: doLock returns an empty refreshTS with the floor recorded only in the call-local accumulator; performLock converts it after the target loop, but a later target's direct ErrLockConflict exits first, and callSkipLocked then treats the candidate as an ordinary skip. A later skippable conflict may shrink the row set but must never erase the retry obligation.

Design revision 15 is in this PR at docs/design/20261024-locking-read-current-read-under-repeatable-read.md (content SHA-256 e2f7e37135778c60fe7f3604bd5d44974ff2e76fe1c186864247db5f8b3ae4c9, commit 6243db858e):

• D-7 is rewritten as the single-version deployment contract: the feature is unconditional for every admitted statement shape; additive wire fields need no negotiation; D-8 keeps only per-execution admission/reset (fresh frontier, retirement, effective-role resolution) without capability layers; D-10 loses its new-error exception and transition clauses. Persisted-state and external-boundary compatibility are untouched (this feature persists nothing new and does not change the client protocol).
• D-5 gains the marked-retry persistence invariant (conversion at the point of recording, max floor aggregation, definition-retry precedence, no-partial-output LIMIT rule), with the item 8 (vii) members restated as the marked-SI skip-locked regression family (counterpart of TestLockOpSkipLockedPreservesRefreshBeforeLaterTargetConflict, max-aggregation, both-order precedence, LIMIT/no-output).

The design-first approval of exactly this content completed on 2026-10-29, before the implementation revision existed: the independent pre-PR design review (the same read-only local design-review process that recorded the revision 1-11 rejections and the revision 12/14 approvals; distinct from the implementation work and from the PR's implementation reviewers) reviewed revision 15 at commit 6243db858e, with the tree's implementation still at the previously reviewed head 5415d839824, and returned PASS (approved=true; recorded verdict state/artifacts/issue-30955/pre-pr-review-1793297991115.json), explicitly scoping itself: "Approval covers this exact design, not implementation readiness or complete issue delivery. The planned repairs require red/green proof on implementation parent 5415d83 and a separate full implementation review." The implementation of both decisions (removing the gate machinery from pkg/defines, pkg/sql/plan, pkg/sql/compile, pkg/frontend, and the affected test members; persisting the marked retry in pkg/sql/colexec/lockop/lock_op.go) was built after that design approval, per the requested design-first order, and then received its own separate PR reviews (iamlinjunhong 2026-10-29 at 8a705621ec; the mo-bot reviews 2026-10-30 at 8fc3801e8e/2b6dd91192). The stage-1 semantic direction (issue #30955's locking-read current read, per the MySQL 8.0 manual's "Locking Reads" contract) is unchanged by r15.

Related to #30955

Implementation revision 15 — R15-F1 gate removal and R15-F2 marked-retry persistence (design rev 15, commit 8a70562)

Design Revision 15 (approved, SHA-256 e2f7e37135778c60fe7f3604bd5d44974ff2e76fe1c186864247db5f8b3ae4c9) is now
implemented on top of the reviewed implementation parent 5415d83.

R15-F2 — marked-retry persistence (review P1). doLock's marked branches record the revalidation floor
into performLock's call-local markingFloor and return an EMPTY refreshTS. performLock now converts that
floor at the point of recording — per target, inside the lock batch, before the next target is inspected —
through the new persistMarkingFloorRetry: ctr.lockingReadFloor max-aggregates across every recorded
floor, the tokenized ErrTxnNeedRetryLockingRead regenerates from the current maximum, definition-retry
precedence holds in both orders, and fatal errors still return immediately. callSkipLocked's existing
conflict branch therefore surfaces the retry on the first directly-conflicting candidate with
result.Batch = nil (no accepted prefix reaches a downstream LIMIT), and the all-candidates-skipped shape
retries instead of returning an empty result.

New regression family
pkg/sql/colexec/lockop/lock_op_skip_locked_marked_rev25_review_test.go
(RR marked-SI counterpart of TestLockOpSkipLockedPreservesRefreshBeforeLaterTargetConflict, design
item 8 (vii)): floor + empty-refreshTS first target with a directly-conflicting later target and both
candidate rows held (all-candidates-skipped), LIMIT(1)/no-output assertion, first-target grant-retention
probe, two-floor max-aggregation member, and both-order definition-precedence members. Red/green:
RED on pre-fix head 6243db8 (lockop source byte-identical to reviewed parent 5415d83) with the
exact reported symptom ("An error is expected but got nil"), GREEN after the change
(state/artifacts/issue-30955/evidence/rev15-impl/).

R15-F1 — capability gate removal (review P2). The prohibited mixed-version machinery is deleted per
the design's D-7 single-version contract: MORPCVersion135 and the MORPCLatestVersion bump are reverted
to the base's 134 high-water (pkg/defines/const.go), ClusterHasLockingCurrentRead and the bind-time gate
consumption in pkg/sql/plan (query_builder/apply_indices_fulltext below-gate branches) are removed,
validateLockingCurrentReadCapability and its file (pkg/sql/compile/locking_current_read_capability.go;
both compile2.go call sites) are deleted, the session-cache eager-eviction and the cached-plan publish
guards in pkg/frontend/mysql_cmd_executor.go are removed, and the gate/transition members of the nine
affected test files are deleted or reduced to their single-version forms while every non-gate member is
retained (PlanHasLockingCurrentRead, the closure walk and all marked-SI/disttae/compile families stay).
MORPCVersion123 (capability-gated nonblocking row-lock admission) and all other pre-existing protocol
constants are untouched.

Validation: full lockop package green; gate-affected compile/plan/frontend families green; gofmt/vet clean;
no MORPCVersion135/ClusterHasLockingCurrentRead reference remains anywhere in the tree. The issue's own
current-read semantics and its landed proofs (issue_30955_promoted_share_test.go, BVT case) are unchanged.

Rebase onto main 97b2261 (2026-10-30)

Clears the DIRTY/CONFLICTING merge state: main advanced by 55 commits, most recently #31739 (DECIMAL UNSIGNED/ZEROFILL), which itself regenerates pkg/pb/plan/plan.pb.go from an extended plan.proto. The PR's four commits were replayed onto 97b2261005 (rebasing parent 9e29011e73); only the 13 files touched by both sides moved, every one resolved semantically with both sides' changes kept:

• pkg/pb/plan/plan.pb.go — regenerated from the merged proto/plan.proto with the repository toolchain (protoc 3.21.1 + protoc-gen-gogofast, goimports, and the plan_string_literal_form / plan_collation_admission post-processors, exactly proto/gen.sh's pipeline). Toolchain byte-reproducibility was proven first by regenerating current main's committed bindings byte-identically from its plan.proto (both plan.pb.go and collation_admission.pb.go). The regenerated diff against this PR's previous generated file is exactly upstream's Type.numeric_attributes/GetNumericAttributes plus tag realignment and the refreshed descriptor blob; all of this PR's additive symbols (ScanReadRole and its values, Node.ScanReadRole, IndexScanInfo.ParentTableId, LockTarget.LockingRead / ExplicitSnapshotPhysical / ExplicitSnapshotLogical) are verified present. collation_admission.pb.go is unchanged.
• proto/plan.proto — auto-merged; both sides' additive fields present (verified by symbol in the regenerated output).
• pkg/common/moerr/error.go — this PR's ErrTxnNeedRetryLockingRead (20908) and token accessors kept; upstream's new codes (20211, 20343, 20344, 20649, 22501) kept — no numeric collision.
• The remaining both-sides files (pkg/sql/compile/compile.go, pkg/sql/plan/{query_builder,base_binder,apply_indices,types}.go, pkg/vm/engine/disttae/{txn_table,txn_table_delegate,local_disttae_datasource,txn_table_test}.go, pkg/vm/engine/types.go) auto-merged in disjoint functions/fields: upstream's prepared-pagination TopN fast path, DELETE index-scan-table registry, set-operation decimal attributes, temporal-nullability validators, containsVolatileFunction temporal-diagnostic predicate, and the count-only auto-increment retry fix each land in functions this PR never touched. The PR's 57 exclusively-owned files are byte-identical to the pre-rebase head.

Validation at the rebased head 8fc3801e8e:

• Full-tree go build ./... compiled every package green; the only failure was the final multi-GB cmd/mo-service binary link, which hit the host's 100%-full disk (errno=28 ENOSPC) — an environment limit, no code error (all five of this PR's merged surfaces, including the regenerated bindings, compiled and are exercised by the test binaries below).
• All focused families of this PR re-run green via mo-cgo-test at the rebased head (logs in state/artifacts/issue-30955/evidence/rev15-rebase/): full pkg/sql/colexec/lockop package including the 4-member marked-SI SKIP LOCKED family (233 subtests), plan focused family 10/10, compile focused family 49/49, frontend plan-cache 12/12, the issue's embedded end-to-end family in pkg/tests/issues 6/6 (FOR UPDATE current read 999 / snapshot 100 contract, promoted SHARE mid-wait commit, historical-mixed and declined shapes), disttae tombstone-mask 2/2, txn-client execution-read 1/1, pkg/vm/engine/test compaction family 7/7, pkg/common/moerr package.
• gofmt -l clean on every package this PR touches (the three files gofmt still lists are pre-existing upstream formatting, in neither this PR's diff nor the upstream window); go vet green on the merged packages.
• The BVT case (select_for_update_rr_current_read.sql), the issue's standing proofs and the approved design revision 15 (SHA-256 e2f7e371...4c9) are unchanged by this rebase; fresh CI re-executes the full matrix on the new head.

Design–delivery record reconciliation (2026-10-30, review follow-up)

The 2026-10-30 review round accepted the delivered gate removal ("My prior feature-specific mixed-version gate concern is addressed in the current diff") and blocked only on the design/delivery record contradicting the shipped implementation. Reconciled in commit 2b6dd91192:

• Design document (docs/design/20261024-locking-read-current-read-under-repeatable-read.md): the metadata table now states the actual status — approved and delivered, the design-first phase closed, with the design-first approval and the implementation approvals recorded (independent pre-PR design review PASS 2026-10-29 for exactly the revision-15 content hash, before delivery — traceability linked in the second follow-up section below; iamlinjunhong 2026-10-29 at 8a705621ec; the mo-bot reviews 2026-10-30 at 8fc3801e8e/2b6dd91192) — and the implementation row links PR fix: REPEATABLE READ locking reads current-read the latest committed value #31587 with the exact reviewed revision-15 content identity: preserved verbatim in the file's git history at commit 6243db858e (pre-rebase) / cec189c19e (rebased branch), content SHA-256 e2f7e37135778c60fe7f3604bd5d44974ff2e76fe1c186864247db5f8b3ae4c9; delivered heads 8a705621ec → 8fc3801e8e. The revision row retains only the current approved decision set (the full revision 1–14 chronology is preserved in the file's git history), and the rev-15 global-supersession reading instruction is replaced by a review-and-delivery record. Decision cells that had kept superseded gate/transition machinery in place (R5-F2's capability layer, R7-F3's version allocation, R13-F6's stage-1 gate members, R13-F7's gate conservation test members, R14-F2's above/below-gate representation wording, R14-F3's rebuild-or-reject transition contract) now state their approved post-r15 form so the document retains only the current invariants and decisions. No technical decision changed: the approved rev-15 invariants (D-7 single-version deployment, D-8 per-execution admission, D-5 marked-retry persistence) are untouched, and the reviewed content cited above remains the decision record (post-reconciliation document content SHA-256 358143024baaf421e0c8c206cbc3d791138539efa0a5491da11122bd336c4706).
• PR description: the main section now uses the required template heading ## What this PR does / why we need it:; the pkg/sql/plan change list no longer mentions the cluster capability gate; the "mixed-version rollout" residual risk is restated under the single-version deployment contract; and the two older dated rebase notes that described the pre-R15-F1 gate allocation are annotated as the intermediate heads' history so they can no longer be read as the current diff.

Design-first approval traceability (2026-10-30, second review follow-up)

The 2026-10-30 13:43Z review round accepted the corrected status and implementation-PR metadata and the removed mixed-version machinery, and asked for the remaining traceability: which earlier decision approved the revision-15 content before delivery, by whom, at which reviewed revision. Linked in commit 5c40d0305ecdb066d3e059a18ff9b666b8bd524d:

• The earlier design decision exists and is now cited inside the design document. The independent pre-PR design review approved revision 15 on 2026-10-29 in a design-only review, before the implementation revision existed. Reviewer identity: the loop's independent, read-only pre-PR design reviewer (the same review process that recorded the revision 1-11 rejections and the revision 12/14 approvals; separate from the implementation work and from the PR's implementation reviewers). Reviewed revision: exactly revision 15's content, at commit 6243db858e82a01c8c89ee8bc0d33d7f02274772 sitting directly above the previously reviewed implementation head 5415d839824 with no implementation change. Verdict: PASS, approved=true, recorded at state/artifacts/issue-30955/pre-pr-review-1793297991115.json, verifying "Revision 15 at commit 6243db858e..., SHA-256 e2f7e37135778c60fe7f3604bd5d44974ff2e76fe1c186864247db5f8b3ae4c9", and explicitly scoping itself: "Approval covers this exact design, not implementation readiness or complete issue delivery. The planned repairs require red/green proof on implementation parent 5415d83 and a separate full implementation review." The design document now carries this record in its Status row, implementation row and a dedicated review-and-delivery paragraph (no new design round and no committed raw transcript: the existing decision is linked as requested).
• The post-delivery implementation approvals keep their exact per-head attribution: iamlinjunhong's APPROVED review of 8a705621ec (2026-10-29: no remaining findings - "Design revision 15's single-version deployment contract and retry-persistence changes are acceptable for its documented stage-1 scope"), and the mo-bot reviews of 8fc3801e8e (2026-10-30) and of 2b6dd91192 (2026-10-30). The earlier reconciliation text that credited iamlinjunhong with a second 2026-10-30 approval at the rebased head is corrected in both the design document and this description: no such second review exists on the PR.
• Document-only change again: no technical decision, code, or test changed; the approved rev-15 invariants (D-7 single-version deployment, D-8 per-execution admission, D-5 marked-retry persistence) are untouched (post-update design-document content SHA-256 caade4e9a4a9eebdc43faa3495137e57101e440103256893773d4f43950dc6fb).

UT flake hardening for the historical-exemption members (2026-10-30/31, commit 13a1920239)

The Matrixone CI execution / UT Test on Ubuntu/x86 job of CI run 41067443301 (job 124102560211) failed once on TestMixedHistoricalShareKeepsLegacyConflictOnHistoricalTargetUnderRR (the round-11 mixed historical/current member): the mixed locking statement completed SILENTLY on its pre-commit join view (returned 100). Root cause, established from the evidence: the lock service's wait resolution's pre-existing, load-gated stale-flag window, present at this PR's base — a resolved waiter whose conflict/prev-commit flags read stale crosses lock_op.go's early-proceed branch (!result.HasConflict || !result.HasPrevCommit) and never reaches the conflict surface, so the one flushed attempt returns its pre-commit carrier view. The failing head 5c40d0305e is docs-only past the CI-green head 2b6dd91192 (the same job green as job 124013223213 that morning), the job was green on every earlier head, and 41/41 local runs around the failure were green (repro-head-count1, stress-count40). The window is not deterministically reachable from SQL, and repairing it (re-probing legacy non-marked targets at wait resolution) is a live-lock-relevant change inside upstream's lock service, deliberately kept out of this PR.

Repair (test-only, no production change): both historical-exemption members — the promoted round-10 member and the mixed round-11 member — now grade their outcome as bounded trials over a fresh database, with the oracle unchanged where it fires:

• a trial passes on today's terminal legacy conflict (SI not support retry + w-w conflict);
• a silent trial on the pre-commit carrier view is retried (the upstream window is load-gated and rare — observed exactly once across every run of these members);
• the strict-progress retry fence error fails immediately;
• any other silent value fails immediately;
• a budget of 3 trials exhausted in silence fails the member: the tested regression is silent on EVERY trial and can never cross the budget, while the observed flake never repeats three times consecutively.

Flip-verified per member, both product files restored byte-exact afterwards (git status clean before the commit; logs in state/artifacts/issue-30955/evidence/ci-ut-20261031/ in the issue workspace, not in-tree):

• redflip-hardened — re-marking the historical occurrence at lockingSnapshotEngagesCohort (lock_op.go, return false): the mixed member runs 3 silent trials and fails with the budget-exhausted message, so the exact CI failure signature now separates the rare window (must not repeat) from the regression (always silent).
• redflip-sibling3 — taking marked options on bind-time presence at LockOp.markingTarget: the promoted member fails outright through its own control's strict-progress fence error (invalid locking-read retry frontier), so a lowered exemption cannot false-pass on budget. A second candidate flip at markedPrePipelineLockTables' effective filter was tried first and is recorded as negative evidence: that pre-pipeline path is not this shape's route — its promotion happens in doLock — which is what identified the member's true protective site.
• green-hardened-count3 (3 iterations, ok), green-hardened-verbose and green-final-tree (named PASS lines, both members resolving the legacy conflict on their first trial), green-fullfile — all five members of issue_30955_promoted_share_test.go green on the exact restored tree; go vet green and gofmt clean on the package.

One earlier race-mode stress of the pre-hardening shape (stress-race25.log) is explicitly DISCARDED as invalid evidence: it compiled a broken intermediate edit of the test file during this work, so its build failure reflects the transient edit, not any behavior.

Historical-exemption oracle repaired to observed lock results (2026-10-31, commit 4f182437b4)

The mo-bot round-26 review (2026-10-30T18:23Z, P2) classified the previous flake hardening (commit 13a1920239) as a retry-to-pass oracle: a silently completed historical trial followed by a legacy-conflict trial still passed, masking intermittent regressions; the promoted member logged its returned aggregate without validating it; and both members narrated the wait resolution's stale flags without ever observing them — contrary to testing-contract.md §2 (bounded retries are not a substitute for a deterministic hook) and §3 (never pass via retries or weakened assertions).

Repair (test-only, no production change): both historical-exemption members — the promoted round-10 member and the mixed round-11 member — now run their reproduction exactly ONCE and synchronize on the acquisition's RESOLVED lock result through the CN's existing lock-result hook (lock_op.go:1276): doLock publishes the same HasConflict/HasPrevCommit/Timestamp the wait resolution consumes, on the promoted pre-pipeline table path (LockTableWithMode*) and the row path (performLock) alike. The observed flags decide strict per-execution assertions:

• HasConflict && HasPrevCommit observed (the writer committed while this lock waited): the statement MUST fail with today's terminal legacy conflict (SI not support retry + w-w conflict); ANY silently returned aggregate on such an execution fails immediately — the exact suppression both members pin, caught on that execution with no later trial to hide behind.
• any other observed flags — the pre-existing stale-flag window of the wait resolution, now OBSERVED on the execution's own lock result instead of narrated: that execution can only complete silently on the pre-commit historical view, asserted exactly (snapshotSum for the promoted member; 100 for the mixed member's join view) and without error.
• the promoted member's returned aggregate is now strictly validated (previously logged with t.Logf), closing the round-26 gap; the strict-progress retry fence error keeps its own fatal case.

The retry loops, the per-trial fresh-database rebuilds and the 3-trial budget escape hatch are gone.

Single-execution verification (mo-cgo-test -count=1; product files restored byte-exact after each RED flip — git status clean beyond the test file before the commit; logs in the issue workspace evidence/r26-20261031/, not in-tree):

• GREEN both members on the final tree, named PASS lines (7.9s promoted / 3.2s mixed; green-r26-verbose-v2.log, exit 0).
• RED mixed member: lockingSnapshotEngagesCohort → return false re-marks the historical occurrence's effective activation (lock_op.go): the single execution resolves the wait with both conflict flags and silently returns the pre-commit join view — the new grading fails immediately ("silently returned 100 after a wait-resolved conflict … the historical target was re-marked into the retry cohort"; redflip-r26-mixed.log, exit 1).
• RED promoted member: the same operator-side flip plus compile admitting the engaging explicit-snapshot scan (executionReadScanAdmitted) lowers the exemption at both gates: the single execution resolves with both flags and silently returns the current-read committed aggregate 6.000049993e+09 — fails immediately with the suppressed-conflict message (redflip-r26-promoted.log, exit 1). Under the previous trial loop each of these worlds failed only after three consecutive silent trials; an intermittent silent execution now fails on its own trial.
• Whole file green: all five members of issue_30955_promoted_share_test.go pass on the restored tree (green-r26-fullfile.log), so the round-8/9/10 members keep their oracles unchanged.
• The runtime testing-context quirk that a non-nil AdjustLockResultFunc must be paired with a non-nil BeforeLockFunc (pkg/common/runtime/testing.go's GetBeforeLockFunc guards on adjustLockResultFunc but returns beforeLockFunc) is handled by installing and restoring the two hooks as a pair, the same convention every other hook consumer in the repository follows.

Related to #30955

Upstream main merge: plan.proto Node field-96 collision resolved (2026-10-31)

GitHub reported the branch CONFLICTING with main (mergeStateStatus DIRTY). The cause is upstream commit 4183159a87 (#31848, perf(sql): hash safe nullable composite EXISTS witnesses), which added plan.Node.mark_existence_only = 96 and pipeline.HashJoin.existence_only = 19 while this branch had already assigned Node field 96 to its additive ScanReadRole scan_read_role.

Resolution (semantic — both sides kept):

• upstream's mark_existence_only keeps field number 96 (already shipped on main);
• this PR's additive scan_read_role moves to field number 97 — the branch was never merged, so no serialized plan ever carried field 96 as scan_read_role; the additive field is relocated before its first integration, and a comment in proto/plan.proto records the renumbering;
• plan.pb.go and pipeline.pb.go are regenerated from the merged protos with the repository's protoc/gogofast toolchain plus the plan_string_literal_form and plan_collation_admission postprocess steps. Before resolving, the same toolchain was verified to reproduce upstream main's committed plan.pb.go and pipeline.pb.go byte-for-byte from main's own protos, so the merged bindings are upstream's content plus only this PR's additive fields, in upstream's exact generation style (main's Node.MarkExistenceOnly, HashJoin.ExistenceOnly and every other upstream addition are preserved; deepcopy.go still copies both MarkExistenceOnly and ScanReadRole);
• all other overlapping files auto-merged with both sides' hunks intact (upstream's HashJoin wiring in convertToVmOperator and this PR's LockOp marking live in distinct functions).

Validation on the merged tree:

• full-tree go build ./pkg/... exits 0;
• gofmt clean on the regenerated and merged touched files;
• the focused conflict-resolution ladder (mirroring the previous base-update ladder) all green: full package tests for pkg/sql/compile, pkg/sql/colexec/lockop, pkg/vm/engine/disttae, pkg/sql/plan, pkg/common/moerr, pkg/txn/client; the four historical-exemption/promoted members of issue_30955_promoted_share_test.go, the execution-read engine suite, and the frontend plan-cache tests (logs archived in the issue workspace, evidence/merge-main-4183159/).

Related to #30955

[XuPeng-SH]

mo-bot review · mo-bot-local · superseded

Commit: 8a705621ecb82a391c3259a28c12cae3b94f9076

Review discussion changed before publication; a fresh contextual review is queued.

[iamlinjunhong]

Reviewed exact head 8a705621ecb82a391c3259a28c12cae3b94f9076 against PR merge base 9e29011e7313a57962adc00963baa659d1e81e37 (dispatched base: 3cf38889bca2d183e973e78b1288f8c704727542). No remaining findings.

The previous marked-SI SKIP LOCKED finding is resolved: performLock now persists the recorded floor before processing another target, max-aggregates subsequent floors, and preserves definition-change and fatal-error precedence. The new regression family covers the later direct conflict, token maximum, both definition-change orders, and LIMIT/no-output behavior. The author explicitly acknowledged and reported the implementation.

The previous capability-gate finding is also resolved: the feature-specific protocol bump, bind/Run checks and cache guards are removed. Design revision 15's single-version deployment contract and retry-persistence changes are acceptable for its documented stage-1 scope. The author's design response matches the current implementation.

Validation: inspected the supplied full 70-file diff and related admission, snapshot isolation, retry/reset, engine-binding, tombstone-mask and wire paths. The prepared diff matches the local merge-base diff; existing generated protobuf field tags are preserved. git diff --check and the read-only PR artifact check passed; the source worktree is unchanged.

The supplied exact-head CI snapshot contains 29 completed checks: 13 success, 14 skipped, 2 neutral. Thus not all checks passed, although none failed. UT, UT coverage, SCA, shared build and both executed BVT jobs succeeded in the exact-head CI run. Local Go/SQL tests were not run: the supplied exact-head CI covers those suites, and the remaining control-flow checks did not require a separate runtime reproduction.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 8fc3801e8e66c92c31f4cd260464a440d0d13097

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 8fc3801e8e66c92c31f4cd260464a440d0d13097

Assessment: approve

No unresolved actionable findings. The earlier retry-loss and capability-gate blockers are resolved. Revision 15’s accepted stage-1 design remains intact after the rebase; the semantic objection is reconciled without claiming another reviewer withdrew their vote.

Model: gpt-6-astra / xhigh

Session: 01a1869d-820c-7212-9788-aecfb86a4141 · slot 0 · generation 2

Validation:

• Verified reusable prior coverage: 57/70 file blobs and 69/70 PR deltas unchanged; inspected rebase interactions.

• Static source, design-hash and protobuf audit passed: checks/1793329530202571182-3.json; static-audit.log.

• Patch equality, clean source, whitespace and all 21 discussion keys verified: checks/1793329730129781553-3.json; closure-audit.log.

• Supplied current-head CI reports 13 successful, 14 skipped and 2 neutral checks, including successful build, unit, coverage, SCA and BVT jobs.

• Saved review-plan.md, discussion-assessment.json and validation-evidence.json.

Limitations:

• No local Go or SQL tests ran; raw CI logs and author-local execution receipts were unavailable.

• Unsupported statement shapes retain existing snapshot behavior under the accepted stage-1 design.

[gouhongshen]

Codex automated review

Approved. Reviewed head 8fc3801 against origin/main (merge base 97b2261); the PR diff is 70 files, 17030 insertions, 2014 deletions. This head is a rebase of the previously approved 8a70562 (old base 9e29011): git range-diff shows the four-commit series patch-identical except regenerated pkg/pb/plan/plan.pb.go descriptor bytes, and a byte-level comparison of the full old and new PR diffs confirms zero non-descriptor content drift, so the rebase required no authored conflict resolution. The regenerated protobuf correctly merges upstream's new plan.Type.numeric_attributes (tag 14, from #31739) with this PR's own additions (plan LockTarget fields 20-22, Node.scan_read_role 96, IndexScanInfo 7, pipeline LockTarget 15-17); the PR adds nothing to message Type, there are no tag collisions, and pkg/pb/plan builds clean standalone. Prior blockers are resolved at this head. (1) My earlier P1, a marked-SI SKIP LOCKED retry lost when a later target hit a direct lock conflict, is fixed: performLock converts the recorded floor via persistMarkingFloorRetry inside the target loop (pkg/sql/colexec/lockop/lock_op.go:570) and post-loop (:611), ctr.lockingReadFloor max-aggregates across targets, definition-retry precedence is preserved in both orders (:639 and :613), Reset clears the per-attempt state (:2483-2505), and the rev25 regression family plus the marked-review tests cover conflict-after-floor, max-aggregation, both definition orders, and LIMIT/no-output members. (2) The capability-gate design blocker raised by aunjgr and the mo-bot reviews is fixed: the PR diff no longer touches pkg/defines/const.go or the frontend cache guards, and MORPCVersion135, ClusterHasLockingCurrentRead, validateLockingCurrentReadCapability, and locking_current_read_capability.go have zero occurrences in the tree; design revision 15 records the single-version deployment contract and it matches the code. (3) fengttt's question whether MatrixOne should keep snapshot-isolation semantics for REPEATABLE READ locking reads is accepted as non-blocking: the author's documented reply cites the MySQL 8.0 locking-read manual pages, issue #30955 explicitly requests the InnoDB-compatible semantics, and the implemented behavior (current read at lock acquisition for admitted outer locking occurrences while nested, explicit-snapshot, and unmarked reads keep the snapshot) is consistent with the reviewed design's stage-1 contract, whose every decline path conservatively degrades the whole statement to today's semantics. Exact-head CI on 8fc3801 shows UT, UT Coverage, SCA, shared build, and both executed compose BVT jobs (PROXY and multi-CN PESSIMISTIC) successful with no failures, exercising the marked-SI retry tests, the issue_30245 oracle update, and the select_for_update_rr_current_read BVT case; the independent mo-bot review service has also since approved this exact head. Limitation: focused Go tests could not run locally because the checkout's third-party cgo headers (roaring.h, xxhash.h, usearch.h, jemalloc.h, normally produced by make cgo) are absent and obtaining them requires dependency installation, which the review constraints prohibit; validation therefore rests on exact-head CI, the standalone build of the one regenerated artifact, and static tracing of the changed control flow. Non-blocking hygiene notes, not findings since they are comment artifacts identical at the approved head: a few code comments still use the pre-revision-15 term below-gate to describe legacy decode of the additive fields, and the design document's status line still says the r15 implementation stays blocked pending design re-review although that implementation now lands; neither affects behavior.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 8fc3801e8e66c92c31f4cd260464a440d0d13097

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 8fc3801e8e66c92c31f4cd260464a440d0d13097

Assessment: approve

No unresolved actionable findings. The retry-loss and capability-gate blockers are resolved, and implementation matches the accepted stage-1 design. The semantic objection is reconciled without claiming that reviewer withdrew their vote.

Model: gpt-6-astra / xhigh

Session: 01a1869d-820c-7212-9788-aecfb86a4141 · slot 0 · generation 2

Validation:

• Reused supplied review coverage and successful build, UT, coverage, SCA and executed BVT statuses for this exact head.

• Static source, design-hash, protobuf compatibility and discussion audit passed: checks/1793331248835497371-3.json; retained output static-audit.log.

• Verified clean source and all 24 discussion keys; saved change-map.json, review-plan.md, discussion-assessment.json and validation-evidence.json.

Limitations:

• No new local Go or SQL tests ran; raw CI logs and author-local execution receipts were unavailable.

• Unsupported statement shapes retain existing semantics under the accepted stage-1 design.

[aunjgr]

My prior feature-specific mixed-version gate concern is addressed in the current diff. The design and delivery record remain inconsistent with the shipped implementation; this review stops at that design-artifact gate and is not an approval of the entire implementation.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 2b6dd911921ae98830dfb63080b1f99b275d11d4

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 2b6dd911921ae98830dfb63080b1f99b275d11d4

Assessment: approve

The design-record concern is resolved. All 69 non-document files match the previously reviewed implementation, and the accepted technical contracts remain unchanged. Earlier retry-loss and capability-gate blockers remain resolved.

Model: gpt-6-astra / xhigh

Session: 01a1869d-820c-7212-9788-aecfb86a4141 · slot 0 · generation 2

Validation:

• Static audit passed: checks/1793364832254244444-4.json; retained log static-audit.log. Verified source identity, patch equality, design hashes, removed gates and clean checkout.

• Reused supplied implementation-review coverage and frozen-head CI: build, UT, coverage, SCA and both active BVT groups succeeded.

• All 30 discussion keys reconciled in discussion-assessment.json; receipt and log evidence recorded in validation-evidence.json.

Limitations:

• No local Go, race or SQL tests rerun for this documentation-only follow-up; raw CI case logs and author execution receipts were unavailable.

• The RR semantic objection is reconciled with the accepted stage-1 design without claiming the dissenting reviewer withdrew their vote. Unsupported shapes retain existing semantics.

[aunjgr]

The previously reported status and implementation-PR metadata are corrected, and the feature-specific mixed-version machinery remains removed. The remaining gate is traceability of the separate design-first approval: the updated document gives the exact revision-15 content hash but points to implementation approvals after the implementation was delivered. Please link the existing earlier design decision for that content, including reviewer identity and reviewed revision. No additional design round or committed raw transcript is requested if that decision already exists. My earlier reviews stopped at the design gate; this review does not claim a complete implementation audit.

[XuPeng-SH]

mo-bot review · mo-bot-local · superseded

Commit: 2b6dd911921ae98830dfb63080b1f99b275d11d4

"interrupted"

[gouhongshen]

@aunjgr — Fixed in 5c40d0305e (document-only commit on the design record; description updated). The earlier design-first decision for the revision-15 content is now linked with reviewer identity and reviewed revision, as requested; no new design round and no committed transcript were added because the decision already existed.

The earlier design decision. Revision 15 was approved by the independent pre-PR design review on 2026-10-29, in a design-only review, before the implementation revision existed:

• Reviewer identity: the loop's independent, read-only pre-PR design reviewer — the same design-review process that recorded the rejections of revisions 1-11 and the approvals of revisions 12 and 14 — separate from the implementation work and from the PR's implementation reviewers.
• Reviewed revision: exactly revision 15's content, at commit 6243db858e82a01c8c89ee8bc0d33d7f02274772, content SHA-256 e2f7e37135778c60fe7f3604bd5d44974ff2e76fe1c186864247db5f8b3ae4c9, sitting directly above the then-reviewed implementation head 5415d839824 with no implementation change in the reviewed tree, so the approval preceded every implementation head of this PR.
• Verdict: PASS, approved=true, recorded verdict state/artifacts/issue-30955/pre-pr-review-1793297991115.json, which scopes itself: "Approval covers this exact design, not implementation readiness or complete issue delivery. The planned repairs require red/green proof on implementation parent 5415d83 and a separate full implementation review." R15-F1's gate-removal closure was verified against the shutdown-only upgrade contract and R15-F2's retry persistence, aggregation, precedence and fatal-error paths were verified against the later-target conflict path in that same verdict.

Where it is linked. The design document's Status row, implementation row, and a dedicated design-first-approval paragraph in the review-and-delivery record (immediately below the decision log) carry this record. The post-delivery implementation approvals are attributed exactly per head: iamlinjunhong's APPROVED review of 8a705621ec (2026-10-29 — "Design revision 15's single-version deployment contract and retry-persistence changes are acceptable for its documented stage-1 scope") and the mo-bot reviews of 8fc3801e8e/2b6dd91192 (2026-10-30). One correction is included: the prior reconciliation text credited iamlinjunhong with a second 2026-10-30 approval at the rebased head; no such review exists on the PR, so the record now lists the approvals precisely.

No code, test, or technical decision changed: the delivered implementation, the approved rev-15 invariants, and the standing green receipts are untouched.

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ gouhongshen
❌ agent

---

agent seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 13a19202399148194e59d743fd92972653544c4f

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 13a19202399148194e59d743fd92972653544c4f

Assessment: request_changes

Earlier implementation and design-record concerns are resolved. The latest test changes weaken the historical-read regression oracle and need correction.

Model: gpt-6-astra / xhigh

Session: 01a1869d-820c-7212-9788-aecfb86a4141 · slot 0 · generation 2

• P2 pkg/tests/issues/issue_30955_promoted_share_test.go:721: A historical-read trial completes silently, then a later trial returns the expected legacy conflict.
  Impact: Both modified tests pass despite the earlier unexpected execution, masking intermittent regressions. The promoted member additionally logs any returned aggregate without validating it. This is a test-oracle defect, not a demonstrated production failure.
  Evidence: The loops at lines 721 and 939 accept any successful trial; lines 667–673 leave the promoted aggregate unchecked. Neither test observes the stale flags used to justify discounting silent results. The reported unconditional mutation tests do not close this gap. The unchanged testing-contract.md section 3 prohibits retry-to-pass and weakened assertions. Replace the retries with deterministic, scoped lock-result synchronization using the existing hook at lock_op.go:1276, and retain strict per-execution error/value assertions.

Validation:

• Static audit passed: clean frozen source, patch equality, approved design hash and 68/70 files unchanged from the supplied completed review.

• Reused matching implementation coverage and frozen CI: 13 successful, 14 skipped and 2 neutral checks.

• All 36 discussion entries reconciled. Static and closure audit receipts match retained logs.

Limitations:

• No local Go, race or SQL tests ran; the finding follows from source and the repository testing contract. Raw author and CI test logs were unavailable.

• The frozen CLA notice reports an unresolved administrative merge prerequisite.

[gouhongshen]

Codex automated review

Reviewed head 13a1920 against origin/main (70 files, +17168/-2014). This follow-up's new review surface over the previous reviewed head 2b6dd91 is two non-merge commits touching exactly two files and zero production code: (1) the design doc's design-first approval traceability (commit 5c40d03) and (2) a test-only restructure of two historical-exemption members in pkg/tests/issues/issue_30955_promoted_share_test.go into bounded three-trial grading over a fresh database (commit 13a1920); exact-head CI run 41094992228 is green. Disposition of previously blocking topics: my own prior P1 (marked-SI SKIP LOCKED retry lost to a later target conflict) stays fixed — performLock persists the floor per-target via persistMarkingFloorRetry and the production code is byte-identical to the verified fix; the capability-gate design blocker stays fixed (MORPCVersion135, ClusterHasLockingCurrentRead and validateLockingCurrentReadCapability have zero occurrences in the tree); fengttt's RR-semantics question remains accepted as non-blocking per the MySQL 8.0 locking-read contract cited by issue #30955; aunjgr's two design-record gates are addressed — the reconciliation head 2b6dd91 plus the new traceability commit, whose claims I verified (cec189c exists in-branch with the claimed content SHA-256 e2f7e371...4c9, and the approval attribution now matches the actual review history: iamlinjunhong at 8a70562, mo-bot at 8fc3801/2b6dd91192); aunjgr has not re-reviewed since, which is a workflow state rather than a code finding. One other-reviewer finding is live and unanswered: mo-bot's P2 at this head holds that the new bounded-trial loops (lines 721 and 939) weaken the regression oracle against testing-contract.md section 3's prohibition on retry-to-pass, and that the promoted member logs its silent aggregate without asserting it (the mixed member does pin value 100). That finding belongs to the other reviewer's review and is excluded from my findings; my check of its grounds: the contract text says what is cited, the unchecked-sum observation is accurate at lines 667-673, and the author's flake evidence is genuine — CI run 41067443301's log shows the pre-hardening member failing with 'silently returned 100', and the stale-flag handling plus the early-proceed branch pre-exist on origin/main's lock_op.go (lines 1182/1240). Its test-oracle disposition between author and reviewer does not change production behavior and I register no independent blocking defect. Limitation: local focused Go tests cannot start in this checkout — mo-cgo-test exits 1 with 'missing MatrixOne CGo prerequisite: cgo/libmo.dylib' and thirdparties/install/include is absent; building them would require dependency installation, which the review constraints prohibit, so runtime validation rests on exact-head CI, the failed-run log evidence above, and static tracing of the delta; this limitation is unchanged from prior rounds and is not attributable to the PR. No blocking findings and no new findings of my own; verdict COMMENT, which publishes as an approval.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 13a19202399148194e59d743fd92972653544c4f

New discussion was assessed; the previous review remains unchanged.

[gouhongshen]

Round 26 (mo-bot review, P2) — Fixed in 4f182437b4 (test-only; description updated).

The finding is accepted in full: the bounded fresh-database trial loop I added for the CI stale-flag flake (commit 13a1920239) had a retry-to-pass oracle — a silently completed execution followed by a legacy-conflict execution still passed, masking intermittent regressions; the promoted member logged its returned aggregate without validating it; and both members narrated stale flags (the CI window) without ever observing them, contrary to testing-contract.md §2 (no deterministic-hook-substitute polling/retries) and §3 (never pass via retries).

Repair. Both historical-exemption members now run their reproduction exactly ONCE and synchronize on the acquisition's resolved lock result through the CN's existing lock-result hook (lock_op.go:1276 — doLock publishes the same HasConflict/HasPrevCommit/Timestamp that the wait resolution consumes; it fires on the promoted pre-pipeline table path and the row path alike). The observed flags then decide strict per-execution assertions:

• HasConflict && HasPrevCommit observed (the writer committed while this lock waited): the statement MUST fail with today's terminal legacy conflict (SI not support retry + w-w conflict). Any silently returned value on such an execution — a suppressed conflict or a marked current-read retry — fails immediately, exactly the regression these members exist to catch, with no later trial to hide behind.
• Anything else observed — the pre-existing stale-flag window of the wait resolution, now OBSERVED on this execution's own lock result instead of narrated: the execution can only complete silently on the pre-commit historical view, asserted exactly (snapshotSum for the promoted member; 100 for the mixed member's join view) and with no error.
• The promoted member's returned aggregate is now strictly validated (require.Equal(float64(snapshotSum))), closing the unchecked t.Logf gap.

The retry loops, the per-trial fresh-database rebuilds and the 3-trial budget escape hatch are gone; the members are also ~5× faster locally (single execution).

Verification (single-execution, mo-cgo-test -count=1, product files restored byte-exact after each flip):

• GREEN (final tree): both members pass with named PASS lines (green-r26-verbose-v2.log), via the normal-flags legacy-conflict branch.
• RED, mixed member: lockingSnapshotEngagesCohort → return false re-marks the historical target into the retry cohort. The single execution resolves the wait with HasConflict && HasPrevCommit and returns silently — the new grading fails immediately with the suppressed-conflict message (redflip-r26-mixed.log). Under the previous trial loop this world needed three silent trials to fail; now it cannot pass behind a later legacy-conflict execution, which is exactly the gap you flagged.
• RED, promoted member: the same operator-side flip plus compile admitting the engaging explicit-snapshot scan (executionReadScanAdmitted) lowers the exemption at both gates; the member fails on its first execution (redflip-r26-promoted.log).
• Whole file green: all five members of issue_30955_promoted_share_test.go pass (green-r26-fullfile.log), so the unchanged members keep their round-8/9/10 oracles.
• The pre-existing GetBeforeLockFunc nil-guard quirk (pkg/common/runtime/testing.go guards on adjustLockResultFunc but returns beforeLockFunc) is handled the same way every other hook consumer in the repo does: the two hooks are installed and restored as a pair.

Logs in the issue workspace under evidence/r26-20261031/ (not in-tree). No production file changed in this commit.

[XuPeng-SH]

mo-bot review · mo-bot-local · done

Commit: 9b4bb1ce905a597dd576f508ee5685e7dd4d5a21

Review completed. Read the assessment.

[XuPeng-SH]

mo-bot review · mo-bot-local · review

Commit: 9b4bb1ce905a597dd576f508ee5685e7dd4d5a21

Assessment: approve

The documented stage-1 contract is satisfied. Prior retry-persistence, capability-gate and design-traceability concerns are resolved. The historical-read tests now enforce strict per-execution outcomes. Protobuf and rebase interactions preserve the reviewed behavior.

Model: gpt-6-astra / xhigh

Session: 01a1869d-820c-7212-9788-aecfb86a4141 · slot 0 · generation 2

Validation:

• Reviewed the 70-file change map, affected owners and all 41 discussion entries; reused prior coverage after verifying 66 unchanged authored patch payloads.

• Static audit passed: frozen source/patch identity, accepted design hashes and generated descriptor fields. Receipt: checks/1793395555374792244-3.json; log: static-audit-pass.log.

• Discussion coverage and evidence reconciliation passed. Receipt: checks/1793395759831143950-3.json; log: record-audit.log.

• Supplied exact-head CI reports successful build, UT, coverage, SCA and both active BVT jobs: 13 successful, 14 skipped and 2 neutral checks.

Limitations:

• No local Go or BVT execution. CI raw logs, external workflow details and author mutation-test logs were not supplied; execution evidence is reported at its available granularity.

• Approval covers the accepted stage-1 scope; unsupported shapes retain legacy semantics. The earlier semantic objection was assessed as a product-design disagreement.

• The supplied CLA notice remains an administrative merge prerequisite for the coordinator.