package review

import (
	"context"
	"errors"
	"strings"
	"testing"
	"time"

	"github.com/stretchr/testify/require"

	"github.com/tidbcloud/AgentTeam/internal/core/identifiers"
	"github.com/tidbcloud/AgentTeam/internal/coredb"
	"github.com/tidbcloud/AgentTeam/internal/store/agent"
	"github.com/tidbcloud/AgentTeam/internal/store/storeerr"
)

// TestReviewContextPersistenceContracts uses one serialized rollback
// transaction for a family of serial scenarios. Each scenario uses a distinct
// workspace and target tuple, so sharing the outer transaction avoids repeated
// setup without weakening behavior coverage.
func TestReviewContextPersistenceContracts(t *testing.T) {

	ctx := context.Background()
	db := startReviewRollbackDB(t, ctx)
	tests := []struct {
		name string
		run  func(*testing.T, context.Context, coredb.DB)
	}{
		{name: "CreateReviewContextDedupsByTargetHash", run: testCreateReviewContextDedupsByTargetHash},
		{name: "UpdateReviewContextRewritesTargetHash", run: testUpdateReviewContextRewritesTargetHash},
		{name: "UpdateReviewContextRejectsIdentityConflict", run: testUpdateReviewContextRejectsIdentityConflict},
		{name: "UpdateReviewContextAllowsNoOpIdentityUpdate", run: testUpdateReviewContextAllowsNoOpIdentityUpdate},
		{name: "CreateReviewContextInvokesValidateReviewParams", run: testCreateReviewContextInvokesValidateReviewParams},
		{name: "UpdateReviewContextInvokesValidateReviewParams", run: testUpdateReviewContextInvokesValidateReviewParams},
		{name: "UpdateReviewContextPreservesExistingDisplayNameWhenOmitted", run: testUpdateReviewContextPreservesExistingDisplayNameWhenOmitted},
		{name: "UpdateReviewContextPreservesExistingDisplayNameWhenWhitespaceOnly", run: testUpdateReviewContextPreservesExistingDisplayNameWhenWhitespaceOnly},
		{name: "UpdateReviewContextUniqueViolationMapsToTypedConflict", run: testUpdateReviewContextUniqueViolationMapsToTypedConflict},
	}
	for _, test := range tests {
		t.Run(test.name, func(t *testing.T) {
			test.run(t, ctx, db)
		})
	}
}

// TestCreateReviewContextDedupsByTargetHash pins the load-bearing identity
// contract: target_hash is the dedup key, so re-creating with the SAME
// (machine/root/base/head) tuple returns the existing row unchanged rather
// than creating a duplicate, while distinct roots produce distinct rows.
//
// Reviewer3 required this as a real-DB regression: pure hash-function tests
// cannot prove the SQL ON DUPLICATE KEY UPDATE path actually fires against
// the UNIQUE(workspace_id, target_hash) index. This test runs against a
// migrated TiDB instance so the round-trip is exercised end-to-end.
func testCreateReviewContextDedupsByTargetHash(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_target_hash_dedup"
		user      = "usr_target_hash_dedup"
		machineID = "machine_target_hash"
		baseRef   = "main"
		headRef   = "feature"
	)

	// Three rows that differ ONLY by RootPath — the canonical path is part of
	// the identity tuple, so each MUST land in its own row.
	rootA, rootB, rootC := "/repo/target_hash/a", "/repo/target_hash/b", "/repo/target_hash/c"
	rcA, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootA, BaseRef: baseRef, HeadRef: headRef,
	})
	require.NoError(t, err)
	rcB, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootB, BaseRef: baseRef, HeadRef: headRef,
	})
	require.NoError(t, err)
	rcC, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootC, BaseRef: baseRef, HeadRef: headRef,
	})
	require.NoError(t, err)
	require.NotEqual(t, rcA.ID, rcB.ID)
	require.NotEqual(t, rcA.ID, rcC.ID)
	require.NotEqual(t, rcB.ID, rcC.ID)
	require.NotEqual(t, rcA.RootPath, rcB.RootPath)

	// Re-create A's exact tuple — the existing row's id wins; no new row.
	rcA2, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootA, BaseRef: baseRef, HeadRef: headRef,
		DisplayName: "refreshed display name",
	})
	require.NoError(t, err)
	require.Equal(t, rcA.ID, rcA2.ID, "re-creating same target tuple MUST return the existing row id")
	require.Equal(t, "refreshed display name", rcA2.DisplayName,
		"ON DUPLICATE KEY UPDATE MUST refresh attached metadata on the existing row")

	// Three distinct rows in the DB after the upsert.
	var count int
	require.NoError(t, db.QueryRow(ctx, `SELECT COUNT(*) FROM review_contexts WHERE workspace_id = $1`, ws).Scan(&count))
	require.Equal(t, 3, count, "exactly three rows for three distinct targets")
}

// TestUpdateReviewContextRewritesTargetHash is the second half of the
// correctness blocker reviewer3 + codex6 raised: UpdateReviewContextForUser
// MUST recompute target_hash from the new params and write it atomically in
// the same UPDATE — otherwise the old identity silently lingers (lookup by
// the new identity misses) AND a later upsert of the new tuple creates a
// duplicate row that splits one review target across two ids. The test
// proves the recompute + atomic write by updating one row to an unowned
// identity and asserting both the old-hash-miss and new-hash-hit behavior.
func testUpdateReviewContextRewritesTargetHash(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_target_hash_update"
		user      = "usr_target_hash_update"
		machineID = "machine_update"
		baseRef   = "main"
		headRef   = "feature"
		rootOld   = "/repo/update/old"
		rootNew   = "/repo/update/new"
	)

	rc, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootOld, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_target_hash_update",
	})
	require.NoError(t, err)
	oldHash := reviewContextTargetHash(machineID, rootOld, baseRef, headRef)
	newHash := reviewContextTargetHash(machineID, rootNew, baseRef, headRef)
	require.NotEqual(t, oldHash, newHash)

	// Update to an UNOWNED new identity. The UPDATE must write the new
	// target_hash atomically. SourceAgentID is carried through so the
	// access check (ensureReviewContextAccess) routes through the permissive
	// RequireAgentOperation stub.
	updated, err := store.UpdateReviewContextForUser(ctx, user, ws, rc.ID, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootNew, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_target_hash_update",
	})
	require.NoError(t, err)
	require.Equal(t, rootNew, updated.RootPath)

	// The old target_hash MUST miss now — no row should be reachable from the
	// previous identity. If the UPDATE had not written target_hash, the old
	// hash would still resolve and the new hash would not.
	var idByOld string
	err = db.QueryRow(ctx, `SELECT id FROM review_contexts WHERE workspace_id = $1 AND target_hash = $2`, ws, oldHash).Scan(&idByOld)
	require.Error(t, err, "old target_hash MUST be unowned after update writes new hash")

	var idByNew string
	require.NoError(t, db.QueryRow(ctx, `SELECT id FROM review_contexts WHERE workspace_id = $1 AND target_hash = $2`, ws, newHash).Scan(&idByNew))
	require.Equal(t, rc.ID, idByNew, "new target_hash MUST resolve to the same row id")
}

// TestUpdateReviewContextRejectsIdentityConflict pins the typed-conflict
// contract: when the update would move a row onto an identity owned by a
// DIFFERENT id within the same workspace, the Store MUST refuse the mutation
// with storeerr.ErrConflict (HTTP 409). Both the original row and the
// already-owning row MUST remain unchanged — silently overwriting would
// lose the binding of the original row; upserting would split the identity
// across two ids. Surfacing the conflict via DB duplicate-key error would
// leak through as HTTP 500 with no stable code; the explicit pre-check
// here returns a typed error so the HTTP layer maps to 409.
func testUpdateReviewContextRejectsIdentityConflict(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_target_hash_conflict"
		user      = "usr_target_hash_conflict"
		machineID = "machine_conflict"
		baseRef   = "main"
		headRef   = "feature"
		rootA     = "/repo/conflict/a"
		rootB     = "/repo/conflict/b"
	)

	rcA, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootA, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_conflict_a",
	})
	require.NoError(t, err)
	rcB, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootB, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_conflict_b",
	})
	require.NoError(t, err)

	// Try to move A onto B's identity. Must fail with typed conflict.
	_, err = store.UpdateReviewContextForUser(ctx, user, ws, rcA.ID, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootB, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_conflict_a",
	})
	require.Error(t, err)
	require.True(t, errors.Is(err, storeerr.ErrConflict),
		"identity collision MUST surface as storeerr.ErrConflict (HTTP 409); got %v", err)

	// Both rows MUST be unchanged: A still on rootA, B still on rootB.
	reloadedA, err := store.GetReviewContext(ctx, ws, rcA.ID)
	require.NoError(t, err)
	require.Equal(t, rootA, reloadedA.RootPath, "row A MUST be unchanged after conflict rejection")
	reloadedB, err := store.GetReviewContext(ctx, ws, rcB.ID)
	require.NoError(t, err)
	require.Equal(t, rootB, reloadedB.RootPath, "row B MUST be unchanged after conflict rejection")

	// Two rows in the DB, no splits.
	var count int
	require.NoError(t, db.QueryRow(ctx, `SELECT COUNT(*) FROM review_contexts WHERE workspace_id = $1`, ws).Scan(&count))
	require.Equal(t, 2, count)
}

// TestUpdateReviewContextAllowsNoOpIdentityUpdate verifies that updating a
// row to the identity it already owns is NOT flagged as a conflict — the
// pre-check skips when newHash == existingHash. Without this, a caller
// refreshing display_name/pr_url on an existing target without changing
// machine/root/refs would hit a spurious 409 against itself.
func testUpdateReviewContextAllowsNoOpIdentityUpdate(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_target_hash_noop"
		user      = "usr_target_hash_noop"
		machineID = "machine_noop"
		baseRef   = "main"
		headRef   = "feature"
		root      = "/repo/noop"
	)

	rc, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: root, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_noop",
	})
	require.NoError(t, err)

	updated, err := store.UpdateReviewContextForUser(ctx, user, ws, rc.ID, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: root, BaseRef: baseRef, HeadRef: headRef,
		DisplayName:   "noop refreshed",
		SourceAgentID: "agt_noop",
	})
	require.NoError(t, err, "no-op identity update MUST NOT be flagged as conflict")
	require.Equal(t, "noop refreshed", updated.DisplayName)
	require.Equal(t, rc.ID, updated.ID)
}

// TestCreateReviewContextInvokesValidateReviewParams pins the codex6
// directive that the create path actually invoke ValidateReviewParams — not
// just exist as an unused helper. A non-absolute RootPath MUST be rejected
// with storeerr.BadRequest (HTTP 400), not a generic 500. The test focuses
// on the agent-caller path because that's the production write surface
// today; the user-caller path uses the same createReviewContext helper
// underneath.
func testCreateReviewContextInvokesValidateReviewParams(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_validate"
		user      = "usr_validate"
		machineID = "machine_validate"
	)

	// Relative path: ValidateReviewParams rejects via storeerr.BadRequest.
	_, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: "relative/path",
	})
	require.Error(t, err)
	require.True(t, errors.Is(err, storeerr.ErrBadRequest),
		"non-absolute RootPath MUST surface as storeerr.ErrBadRequest (HTTP 400); got %v", err)

	// Trailing-whitespace absolute path: MUST pass ValidateReviewParams.
	// The trailing space is part of the canonical root and survives
	// byte-exact through the insert + reload round-trip.
	rc, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: "/repo/validate ",
	})
	require.NoError(t, err)
	require.True(t, strings.HasSuffix(rc.RootPath, " "),
		"trailing-whitespace root MUST survive byte-exact through create+reload; got %q", rc.RootPath)
}

// TestUpdateReviewContextInvokesValidateReviewParams pins the parallel
// contract for the update path: ValidateReviewParams MUST run before the
// UPDATE, so a malformed new RootPath surfaces as 400 rather than getting
// written to the row.
func testUpdateReviewContextInvokesValidateReviewParams(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_validate_update"
		user      = "usr_validate_update"
		machineID = "machine_validate_update"
		root      = "/repo/validate_update"
	)

	rc, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: root,
		SourceAgentID: "agt_validate_update",
	})
	require.NoError(t, err)

	_, err = store.UpdateReviewContextForUser(ctx, user, ws, rc.ID, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: "relative/update",
		SourceAgentID: "agt_validate_update",
	})
	require.Error(t, err)
	require.True(t, errors.Is(err, storeerr.ErrBadRequest),
		"update with non-absolute RootPath MUST surface as storeerr.ErrBadRequest; got %v", err)

	// Row MUST be unchanged.
	reloaded, err := store.GetReviewContext(ctx, ws, rc.ID)
	require.NoError(t, err)
	require.Equal(t, root, reloaded.RootPath, "row MUST be unchanged after validation rejection")
}

// TestUpdateReviewContextPreservesExistingDisplayNameWhenOmitted pins the
// omit=preserve contract for DisplayName that reviewer3 specified: an update
// that omits DisplayName MUST keep the existing row's DisplayName unchanged.
// A earlier revision of the code called ValidateReviewParams BEFORE the
// preserve check; validate fills empty DisplayName with
// defaultReviewDisplayName(rootPath), so the "if DisplayName == ” preserve"
// branch was dead code and the default ALWAYS won, silently overwriting the
// existing label on every ref-only update. The fix is ordering: preserve
// BEFORE validate. This test catches a regression that reverts the order.
func testUpdateReviewContextPreservesExistingDisplayNameWhenOmitted(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_dn_preserve"
		user      = "usr_dn_preserve"
		machineID = "machine_dn_preserve"
		baseRef   = "main"
		headRef   = "feature"
		root      = "/repo/dn_preserve"
	)

	// Create with a custom DisplayName that is DISTINCT from the default
	// (defaultReviewDisplayName would yield "dn_preserve" — the basename).
	// Using "Custom Label" makes any default-overwrite regression visible.
	rc, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: root, BaseRef: baseRef, HeadRef: headRef,
		DisplayName:   "Custom Label",
		SourceAgentID: "agt_dn_preserve",
	})
	require.NoError(t, err)
	require.Equal(t, "Custom Label", rc.DisplayName, "create MUST persist the supplied DisplayName")

	// Update with a DIFFERENT root (so defaultReviewDisplayName would yield a
	// different basename) but OMIT DisplayName. The existing "Custom Label"
	// MUST survive — validate's default must NOT fire when the caller omitted
	// the field.
	updated, err := store.UpdateReviewContextForUser(ctx, user, ws, rc.ID, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID,
		RootPath: "/repo/dn_preserve_new", BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_dn_preserve",
	})
	require.NoError(t, err)
	require.Equal(t, "Custom Label", updated.DisplayName,
		"omit DisplayName in update MUST preserve the existing label; the validate-time default MUST NOT fire")

	// Sanity: re-load and confirm.
	reloaded, err := store.GetReviewContext(ctx, ws, rc.ID)
	require.NoError(t, err)
	require.Equal(t, "Custom Label", reloaded.DisplayName,
		"DisplayName MUST be persisted as the preserved value")
}

// TestUpdateReviewContextPreservesExistingDisplayNameWhenWhitespaceOnly pins
// the whitespace-only preserve contract reviewer3 caught on the post-amend
// gate. The previous contract (and the documented "omit = preserve"
// semantics) treats a DisplayName that trims to empty IDENTICALLY to an
// omitted DisplayName: preserve the existing label.
//
// Why this matters: a caller that sends `DisplayName: "   "` over the wire
// (e.g. a buggy client that clears the field by sending whitespace) MUST NOT
// cause validate's defaultReviewDisplayName(rootPath) to overwrite the
// existing label. Without TrimSpace in the preserve check, validate would
// see the whitespace as non-empty, skip the default, and persist raw
// whitespace bytes — diverging from the documented contract and from the
// previous behavior.
//
// Mutation contract: revert the preserve check from
// `strings.TrimSpace(params.DisplayName) == ""` back to
// `params.DisplayName == ""` and this test fails — the whitespace-only
// param slips past preserve, validate skips the default (because the raw
// whitespace is non-empty), and the row's DisplayName becomes "   ".
func testUpdateReviewContextPreservesExistingDisplayNameWhenWhitespaceOnly(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_dn_ws_preserve"
		user      = "usr_dn_ws_preserve"
		machineID = "machine_dn_ws_preserve"
		baseRef   = "main"
		headRef   = "feature"
		root      = "/repo/dn_ws_preserve"
	)

	rc, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: root, BaseRef: baseRef, HeadRef: headRef,
		DisplayName:   "Custom Label",
		SourceAgentID: "agt_dn_ws_preserve",
	})
	require.NoError(t, err)
	require.Equal(t, "Custom Label", rc.DisplayName)

	// Whitespace-only DisplayName MUST be treated as omit.
	updated, err := store.UpdateReviewContextForUser(ctx, user, ws, rc.ID, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID,
		RootPath: "/repo/dn_ws_preserve_new", BaseRef: baseRef, HeadRef: headRef,
		DisplayName:   "   ",
		SourceAgentID: "agt_dn_ws_preserve",
	})
	require.NoError(t, err)
	require.Equal(t, "Custom Label", updated.DisplayName,
		"whitespace-only DisplayName MUST preserve the existing label — trim before the omit check")

	reloaded, err := store.GetReviewContext(ctx, ws, rc.ID)
	require.NoError(t, err)
	require.Equal(t, "Custom Label", reloaded.DisplayName,
		"DisplayName MUST be persisted as the preserved value, not the whitespace-only input")
}

// TestUpdateReviewContextUniqueViolationMapsToTypedConflict pins the TOCTOU
// loser path codex6 + reviewer3 specified: the SELECT-owner-then-UPDATE
// sequence in UpdateReviewContextForUser has a race window between the
// pre-check and the UPDATE. A concurrent caller that wins the identity in
// that window causes the loser's UPDATE to hit the UNIQUE(workspace_id,
// target_hash) index and surface a 1062 duplicate-key error. Without the
// IsUniqueViolation mapping in the UPDATE error branch, the loser's error
// wraps as a generic "update review context" failure and the HTTP layer
// maps it to 500. WITH the mapping, the loser surfaces the SAME typed
// storeerr.ErrConflict the pre-check uses, so the HTTP layer answers 409
// regardless of which path catches the collision.
//
// Determinism split (codex6 direction):
//
//   - THIS test pins the ERROR-MAPPING branch. The per-Store test-only hook
//     onUpdatePreCheckPassed fires after the pre-check passes and inserts a
//     rival owner row on the SAME connection (rollback fixture is
//     single-connection, so the row is visible to the UPDATE that follows).
//     The UPDATE then deterministically hits the UNIQUE index and surfaces
//     a real 1062 — this is NOT a synthetic error returned by the hook.
//     Without the IsUniqueViolation → ErrConflict mapping in the UPDATE
//     branch, the test fails at the errors.Is(ErrConflict) assertion.
//
//   - Atomic state under the SAME SQL-constraint path is pinned HERE by the
//     "original row MUST be unchanged" assertion: a real 1062 on the UPDATE
//     rolls back the entire single-statement UPDATE, so rc.ID still owns
//     rootOld after the conflict.
//
//   - Atomic state under the PRE-CHECK path is pinned by
//     TestUpdateReviewContextRejectsIdentityConflict (no hook; the rival
//     row is staged before the call, the pre-check catches it). That test
//     asserts BOTH rows unchanged and row-count stays at 2.
//
// Why no deterministic concurrent-scheduling test: the rollback fixture
// wraps a single database/sql.Tx (one connection), so two goroutines
// issuing statements on it concurrently corrupt the connection
// ("commands out of sync"). A separate pool-backed fixture CAN surface
// real SQL-level races, but TiDB testkit snapshot-isolation quirks across
// connections make the winner/loser pattern non-deterministic and the
// assertions flaky. The hook split above covers both the error mapping
// and the atomic-state contracts deterministically, so a probabilistic
// race test adds coverage we already have at the cost of CI stability.
func testUpdateReviewContextUniqueViolationMapsToTypedConflict(t *testing.T, ctx context.Context, db coredb.DB) {
	store := newReviewStoreWithPermissiveAccess(db)

	const (
		ws        = "ws_tocou_hook"
		user      = "usr_tocou_hook"
		machineID = "machine_tocou_hook"
		baseRef   = "main"
		headRef   = "feature"
		rootOld   = "/repo/tocou/old"
		rootNew   = "/repo/tocou/new"
	)

	rc, err := store.CreateReviewContextForUser(ctx, user, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootOld, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_tocou_hook",
	})
	require.NoError(t, err)

	newHash := reviewContextTargetHash(machineID, rootNew, baseRef, headRef)

	// Set the test-only hook. The hook fires AFTER the pre-check passes
	// (newHash is unowned at pre-check time) but BEFORE the UPDATE. The hook
	// synthesizes a rival owner row with a fresh id that owns newHash. The
	// rollback fixture is single-connection, so the row is immediately
	// visible to the UPDATE that follows, which then deterministically hits
	// the UNIQUE(workspace_id, target_hash) index and surfaces 1062.
	//
	// Raw INSERT (instead of store.CreateReviewContextForUser) avoids nesting
	// production code paths and keeps the hook's contract obvious: it stages
	// exactly one rival row, no more.
	store.onUpdatePreCheckPassed = func(ctx context.Context, workspaceID, targetHash, skipID string) {
		rivalID, err := identifiers.NewEntityID("rcx")
		if err != nil {
			t.Fatalf("generate rival id: %v", err)
		}
		now := time.Now().UTC()
		_, err = db.Exec(ctx, `
INSERT INTO review_contexts (id, workspace_id, target_hash, thread_id, source_agent_id, machine_id, root_path,
  base_ref, head_ref, display_name, pr_url, created_by_member_id, updated_by_member_id, created_at, updated_at)
VALUES ($1, $2, $3, NULL, NULL, $4, $5, $6, $7, 'rival_owner', '', NULL, NULL, $8, $9)
`,
			rivalID, workspaceID, targetHash, machineID, rootNew, baseRef, headRef, now, now,
		)
		if err != nil {
			t.Fatalf("insert rival owner via hook: %v", err)
		}
	}

	// Trigger the update. Pre-check passes → hook inserts rival → UPDATE
	// hits 1062 → IsUniqueViolation branch MUST surface typed Conflict.
	_, err = store.UpdateReviewContextForUser(ctx, user, ws, rc.ID, agent.UpsertReviewContextParams{
		WorkspaceID: ws, MachineID: machineID, RootPath: rootNew, BaseRef: baseRef, HeadRef: headRef,
		SourceAgentID: "agt_tocou_hook",
	})
	require.Error(t, err)
	require.True(t, errors.Is(err, storeerr.ErrConflict),
		"UPDATE-branch 1062 MUST map to storeerr.ErrConflict (HTTP 409); got %v", err)

	// Original row MUST be unchanged: a 1062 on the UPDATE rolls back the
	// entire single-statement UPDATE, so rc.ID still owns rootOld.
	reloaded, err := store.GetReviewContext(ctx, ws, rc.ID)
	require.NoError(t, err)
	require.Equal(t, rootOld, reloaded.RootPath,
		"original row MUST be unchanged after Conflict; UPDATE atomicity guarantees 1062 rolls back")

	// The rival owner row is the sole owner of newHash — exactly one row in
	// the DB owns the new target_hash, no splits.
	var ownerCount int
	require.NoError(t, db.QueryRow(ctx,
		`SELECT COUNT(*) FROM review_contexts WHERE workspace_id = $1 AND target_hash = $2`,
		ws, newHash).Scan(&ownerCount))
	require.Equal(t, 1, ownerCount,
		"exactly one row (the rival owner) MUST own the new target_hash")
}

func updatedRoot(rc *agent.ReviewContext) string {
	if rc == nil {
		return "<nil>"
	}
	return rc.RootPath
}