kv: bound pre-Phase-D intent creation and classify resolution evidence

[bootjp]

Summary

Bound creation of a new pre-Phase-D intent to the maximum transaction lock TTL plus one hour of grace after Phase-D activation. The check uses the applied Phase-D floor and a fresh allocation from the durable TSO group. It bypasses cached batch slots, which could otherwise keep the window apparently open long after the TSO passed its end. An allocator without a floor or fresh-admission capability fails closed for new pre-D PREPAREs.

ClassifyPrePhaseDCommit also records the decision for a local primary commit record. It remains unwired until the forwarding path can distinguish an unreadable remote primary from a definitively absent local record. Existing intent resolution remains admissible after the creation window closes.

Behavior and risk

A newly forwarded PREPARE with a pre-Phase-D start is rejected with ErrPrePhaseDWindowClosed after the window. The exceptional path now makes one fresh TSO allocation to measure durable progress. Ordinary post-Phase-D timestamps use the existing validation path.

Test evidence

• go test ./kv -run 'Test(PrePhaseD|ForwardedStartTimestamp)' -count=1 — pass.
• The same focused tests with -race — pass.
• golangci-lint run ./kv/... --timeout=5m with workspace caches — 0 issues.
• Regression: TestPrePhaseDAdmissionBypassesStaleBatchSlot retains an unused old batch slot, advances the underlying TSO beyond the window, and verifies admission closes.
• After the first full CI run exposed a test fixture missing the new capability, the two affected adapter tests and the TSO migration ceiling test passed together under -race; golangci-lint run ./kv/... ./adapter/... reported 0 issues.

Self-review

1. Data loss: Intent creation is rejected after the bound; existing intent resolution is unchanged.
2. Concurrency: The admission check reads current durable TSO progress instead of a locally retained slot.
3. Performance: The additional TSO allocation runs only on the pre-Phase-D exceptional path.
4. Data consistency: The floor and progress are both read through the TSO allocator; the local wall clock does not decide admission.
5. Test coverage: Boundary, missing-capability, wrapped-allocator, and stale-batch cases are covered.

Summary by CodeRabbit

• 機能改善
  • Phase D の新規 PREPARE 受付期間を過ぎると、新たなトランザクションを拒否するようになりました。
  • 受付期間終了後も、既存トランザクションの COMMIT／ABORT は処理できます。COMMIT は確認可能な記録と一致する場合に受け付けます。
  • PREPARE と COMMIT／ABORT が混在する要求は拒否されます。

[bootjp]

@codex review

[bootjp]

@claude review

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7d29c7a9-0278-4e62-b4e3-782e64f910b3

---

📥 Commits

Reviewing files that changed from the base of the PR and between 9ff4d29 and decf142.

---

📒 Files selected for processing (10)

• adapter/internal.go
• adapter/internal_test.go
• docs/design/2026_09_02_partial_prephase_d_resolution_evidence.md
• kv/tso.go
• kv/tso_prephase_d.go
• kv/tso_prephase_d_test.go
• kv/tso_raft.go
• kv/tso_raft_test.go
• kv/tso_runtime.go
• kv/tso_test.go

---

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

---

---

📝 Walkthrough

Walkthrough

Phase D前の転送トランザクションに受付期限を設けました。COMMITとABORTの開始時刻検証をPREPAREから分けました。primary記録の証拠を分類する関数と、関連するテストおよび設計文書を追加しました。

Changes

Phase D前の転送トランザクション

Layer / File(s)	Summary
PREPARE受付期限の判定 kv/tso.go, kv/tso_prephase_d.go, kv/tso_raft.go, kv/tso_runtime.go, kv/tso_prephase_d_test.go, kv/tso_test.go, kv/tso_raft_test.go, docs/design/2026_09_02_partial_prephase_d_resolution_evidence.md	Phase D floorと新しいTSO割り当て時刻を使い、Phase D前の開始時刻を検証します。受付期限は最大トランザクションロックTTLに1時間を加えた期間です。floorまたは新しい時刻を取得できない場合の処理と、アロケータ間でfloorを引き継ぐ処理を追加しました。テストでは期限の境界、ラッパー経由の判定、既存のバッチ枠を使った期限回避の防止を確認します。設計文書では、この受付制限を実装済みと記載しています。
COMMIT・ABORTの転送検証 adapter/internal.go, adapter/internal_test.go, kv/tso_prephase_d.go, kv/tso_prephase_d_test.go, docs/design/2026_09_02_partial_prephase_d_resolution_evidence.md	COMMITまたはABORTだけの要求では、開始時刻の検証でErrTSOTimestampPrePhaseDを許可します。PREPAREを含む要求は通常の転送開始時刻検証を行います。primary記録の証拠を使うCOMMIT分類関数を追加し、対応するテストを追加しました。設計文書では、この記録確認がcommit検証経路に未接続であることを記載しています。

Priority: ➖ Normal

Estimated code review effort: 3 (Moderate) | ~25 minutes

Change: Bug fix

Sequence Diagram(s)

sequenceDiagram
  participant Validator as ValidateForwardedTxnStartTimestamp
  participant Admission as prePhaseDStartWithinWindow
  participant Allocator as BatchAllocator
  participant TSO
  Validator->>Admission: Phase D前の開始時刻を検証
  Admission->>Allocator: PhaseDFloorを取得
  Allocator->>TSO: Phase D floorを参照
  TSO-->>Allocator: Phase D floor
  Allocator-->>Admission: Phase D floor
  Admission->>Allocator: FreshAdmissionTimestampを要求
  Allocator->>TSO: 新しいTSO時刻を割り当て
  TSO-->>Allocator: 割り当て時刻
  Allocator-->>Admission: 割り当て時刻
  Admission-->>Validator: 受付可否

Loading

---

Merge Risk: ⚪ Minimal · up to decf1

The change adds a bounded admission window for new pre-Phase-D PREPAREs and still allows COMMIT and ABORT to resolve existing intents. No concrete merge-blocking issue was found in the supplied context.

Pre-merge checks | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 9 files. (1 skipped: …	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		タイトルは、pre-Phase-D intentの作成期間の制限と解決証拠の分類という主要な変更を正確に示しています。変更内容に関連し、簡潔で具体的です。
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

---

Full details: Docstring Coverage

Explanation

Docstring coverage is 62.50% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 24 functions across 9 files. (1 skipped: 1 unsupported.)

---

• Fix all pre-merge checks with AI

✨ Finishing Touches 💡 1

📝 Generate docstrings 💡

• Commit to this branch
• Create a new PR

---

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

---

_{Comment @coderabbitai help to get the list of available commands.}

[chatgpt-codex-connector]

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review	Status	Commit	Review trigger
📝 Code Review	✅ Completed 2026-10-12T15:44:23.042428Z	decf142	New commits

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

[claude]

Claude finished @bootjp's task in 3m 34s —— View job

---

Review

• Gather context (PR description, diff, design doc)
• Read changed files (kv/tso.go, kv/tso_prephase_d.go, kv/tso_prephase_d_test.go, design doc)
• Trace the production allocator wiring for TSOPhaseDFloorSource
• Post review feedback

Summary

The design (§4.2/§4.3 in docs/design/2026_09_02_partial_prephase_d_resolution_evidence.md) and the pure functions in kv/tso_prephase_d.go are sound and well-tested in isolation. But the one thing the PR title promises — "bound pre-Phase-D intent creation" in production — doesn't actually happen, because the interface the bound depends on never reaches the concrete allocator that's wired up at runtime.

Finding 1 (blocking): the admission window is a no-op in every real deployment

prePhaseDStartWithinWindow (kv/tso.go:427-434) only narrows the carve-out if alloc.(TSOPhaseDFloorSource) succeeds; otherwise it returns true (window open) by design. ValidateForwardedTxnStartTimestamp is called from adapter/internal.go:984 with i.tsAllocator, which is populated in main.go:3658-3662 via kv.ConfiguredTimestampAllocatorThrough(coordinate) — a helper whose own doc comment says it deliberately returns the allocator without resolving runtime-mode decorators. In the only path where Phase D is ever actually active (configureDedicatedCoordinatorTSO, main.go:2461-2524), that value is *DynamicTimestampAllocator (controller.Allocator()).

Walking the chain that *DynamicTimestampAllocator wraps:

• DynamicTimestampAllocator (kv/tso_runtime.go:84-192) forwards PhaseDActive/PhaseDRequired/ValidateDurableTimestamp/Invalidate through currentTimestampAllocator() — but has no PhaseDFloor() method.
• Its durableAllocator in Phase D is a *BatchAllocator (kv/tso_runtime.go:251-256), which likewise forwards PhaseDActive/PhaseDRequired/ValidateDurableTimestamp to b.tso (kv/tso.go:862-878) but has no PhaseDFloor() method.
• b.tso is a *LeaderRoutedTSOAllocator (kv/tso_raft.go:502), which forwards PhaseDActive/PhaseDRequired/ValidateDurableTimestamp (kv/tso_raft.go:751-824) but again has no PhaseDFloor() method — even though the *RaftTSOAllocator underneath it (kv/tso_raft.go:468-473) is the one type that actually implements PhaseDFloor().

So the type assertion at kv/tso.go:428 fails at every layer of the real wrapper chain, prePhaseDStartWithinWindow always returns true, and ValidateForwardedTxnStartTimestamp never returns ErrPrePhaseDWindowClosed in production — regardless of how much time has passed since Phase D activated. This directly contradicts the PR body's "§4.2 is wired" claim and the design doc's "What has shipped" section, and it's exactly the attack surface this PR set out to close (an attacker can still PREPARE at an arbitrary pre-Phase-D start indefinitely).

This gap is invisible in the test suite because TestForwardedStartTimestampClosesAfterTheAdmissionWindow (kv/tso_prephase_d_test.go:180-200) drives ValidateForwardedTxnStartTimestamp with stubPrePhaseDAllocator, a fake that implements TSOPhaseDFloorSource directly — it never exercises the real DynamicTimestampAllocator → BatchAllocator → LeaderRoutedTSOAllocator → RaftTSOAllocator chain that production actually wires up.

Fix: add PhaseDFloor() uint64 forwarding methods to DynamicTimestampAllocator, BatchAllocator, and LeaderRoutedTSOAllocator, mirroring the existing PhaseDActive/PhaseDRequired forwarding pattern each already has, plus a test that goes through the real wrapper chain (or at minimum a compile-time assertion that the production-wired type satisfies TSOPhaseDFloorSource) so this can't silently regress again.

Fix this →

Finding 2 (minor): likely data race in the new tests on the package-level clock var

prePhaseDNowMillis (kv/tso_prephase_d.go:155) is a plain package-level var, reassigned directly (no atomic/mutex) by two different top-level tests — TestForwardedStartTimestampClosesAfterTheAdmissionWindow and TestForwardedStartTimestampAfterPhaseDIsUnaffectedByTheWindow (kv/tso_prephase_d_test.go:180-217) — both marked t.Parallel(). Go's test scheduler can run both concurrently in the same package, giving two goroutines unsynchronized read/write access to the same global. This is a classic -race-flaggable pattern; whether it fires depends on scheduling, so it may be passing today by luck rather than by design. Since the whole point of t.Cleanup(func() { prePhaseDNowMillis = restore }) is correctness under -race, at minimum these two tests should drop t.Parallel() (or the var should be swapped under a mutex/t.Setenv-style serialization) so the suite doesn't become a source of intermittent CI flakes.

Everything else

• PrePhaseDStartAdmissible, PhaseDActivationMillis, and ClassifyPrePhaseDCommit are pure, well-documented, and their boundary tests (kv/tso_prephase_d_test.go) look correctly targeted (off-by-one at the window edge, physical-vs-logical half, record-disagrees-refuses, abort-is-always-a-window-decision).
• Leaving §4.1 (ClassifyPrePhaseDCommit) unwired pending the primaryTxnRecordedStatus readability signal is a reasonable, clearly-documented scope cut, not a defect.
• The wall-clock-read justification in the header comment is convincing as written (admission-only, never an ordering decision) — no objection there.
• Design-doc-first workflow was followed correctly (doc renamed to _partial_, decision recorded in §4a before the implementation commit).

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 3e27a89ff3

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 7fe4e617ac

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[Copilot]

🟡 Changes recommended

Production allocator wrappers bypass the new bound, while resolution logic, wall-clock reliance, and test races introduce additional correctness concerns.

4 open findings

• PhaseDFloor not forwarded through production allocator wrappers · New
• Wall-clock skew can extend admission past the deadline · New
• Creation window incorrectly blocks legacy intent resolution · New
• Parallel test races on shared prePhaseDNowMillis · New

What changed in this PR

Adds bounded admission for pre-Phase-D transaction intents and introduces durable-evidence classification for future commit resolution wiring.

Changes:

• Adds Phase-D floor sourcing and an intent-creation window.
• Adds commit-evidence classification and targeted tests.
• Updates the design status and implementation record.

File	Description
kv/​tso.go	Enforces the new admission window.
kv/​tso_prephase_d.go	Defines window and evidence-classification logic.
kv/​tso_prephase_d_test.go	Tests admission and classification behavior.
docs/​design/​2026_09_02_partial_prephase_d_resolution_evidence.md	Records the partial implementation decision.

🧠 Review effort: Balanced

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e80cc0c83b

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".