Skip to content

Let merge-permission users write through dolt_conflicts_<t>#11032

Merged
tbantle22 merged 1 commit into
taylor/conflicts-delete-permission-gatefrom
taylor/merge-perm-conflicts-impl
May 12, 2026
Merged

Let merge-permission users write through dolt_conflicts_<t>#11032
tbantle22 merged 1 commit into
taylor/conflicts-delete-permission-gatefrom
taylor/merge-perm-conflicts-impl

Conversation

@tbantle22
Copy link
Copy Markdown
Contributor

Summary

Stacks on top of #11030. Adds a branch-control carve-out so a user with only Permissions_Merge on a branch can resolve a merge that produced data conflicts by writing through dolt_conflicts_<t> and via DOLT_CONFLICTS_RESOLVE. Lets a PR reviewer on the SQL workbench finish a conflicting merge without being granted full write access on the target branch.

Admission rule applied at every entry point — Write, OR (Merge AND MergeActive AND data conflicts exist):

  • ProllyConflictsTable.Updater (dtables/conflicts_tables_prolly.go) — new gate via checkConflictsTableWriteAccess.
  • ProllyConflictsTable.Deleter (same file) — replaces the bare Write gate from Gate DELETE on dolt_conflicts_<t> with branch_control Write permission #11030 with the same helper.
  • DoDoltConflictsResolve (dprocedures/dolt_conflicts_resolve.go) — new dsess.CheckAccessOrMergeActive helper; "data conflicts exist" scoped to the working root.

The Updater path's source-table srcUpdater is constructed eagerly inside newProllyConflictOurTableUpdater, so a merge-permission caller admitted at the outer gate would otherwise be rejected by the inner WritableDoltTable.Updater Write check. To bridge that delegation chain:

  • New dsess.ConflictsBypassMarker keyed on (db, branch, table). prollyConflictOurTableUpdater sets it on a derived context just for the source-table writer factory call.
  • WritableDoltTable.Updater honors the marker as a Write-check bypass only when it matches the target table. A marker for t1 cannot leak into a write on t2.

Test plan

  • Extended "Merge permission allows merge with conflicts" (branch_control_test.go) with the full positive workflow: merge-only user runs UPDATE dolt_conflicts_test (write goes through to source table), CALL DOLT_CONFLICTS_RESOLVE, CALL DOLT_COMMIT. Source-table direct writes remain rejected.
  • New "Merge permission allows DELETE from dolt_conflicts_<t> to resolve conflicts" — exercises the DELETE path end-to-end.
  • New "Merge permission does not allow conflicts writes outside an active merge" — without an active merge, even a merge-permission user is rejected by DOLT_CONFLICTS_RESOLVE and by direct source-table writes.
  • go test ./libraries/doltcore/sqle/enginetest/ -run "TestBranchControl" passes.
  • go vet ./libraries/doltcore/sqle/... clean.

Known limitation

When a merge-only user runs UPDATE/DELETE on dolt_conflicts_<t> with zero matching rows (e.g., empty conflicts table outside an active merge), the engine never invokes the writer's per-row methods, so the static error from the factory's permission check never surfaces. This is the same property that affects every WritableDoltTable write check — consistent with existing Dolt behavior. The negative test asserts via DOLT_CONFLICTS_RESOLVE and a direct source-table write instead, both of which run the gate regardless of matching-row count.

🤖 Generated with Claude Code

Adds a carve-out so a user with only branch_control.Permissions_Merge
on a branch can resolve a merge that produced data conflicts by writing
through the dolt_conflicts_<t> system tables. Lets PR reviewers on the
SQL workbench finish a conflicting merge without being granted full
write access on the target branch.

Admission rule applied at every entry point:
  Write, OR (Merge AND MergeActive AND data conflicts exist).

The "data conflicts exist" check is scoped to the specific table for
the conflicts-table writers and to the working root for the
DOLT_CONFLICTS_RESOLVE procedure.

Implementation:
- New checkConflictsTableWriteAccess helper in dtables/. Gates both
  ProllyConflictsTable.Updater and .Deleter (relaxing the existing
  Write gate added in #11030).
- New ConflictsBypassMarker on context.Context, keyed by (db, branch,
  table). prollyConflictOurTableUpdater sets it before constructing
  the eagerly-built source-table srcUpdater; WritableDoltTable.Updater
  honors it as a Write-check bypass only when the marker matches the
  target table. Scoped narrowly so a marker for t1 cannot bypass
  writes on t2.
- New CheckAccessOrMergeActive helper in dsess for procedure-level
  gating; DoDoltConflictsResolve now uses it.

Tests in branch_control_test.go: extended the existing "Merge
permission allows merge with conflicts" case with the full
UPDATE -> DOLT_CONFLICTS_RESOLVE -> DOLT_COMMIT positive workflow;
added a DELETE-path positive case; added a negative case for a
merge-only user outside an active merge.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@tbantle22 tbantle22 requested a review from reltuk May 12, 2026 18:49
Copy link
Copy Markdown
Contributor

@reltuk reltuk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@tbantle22 tbantle22 merged commit 6217ad2 into taylor/conflicts-delete-permission-gate May 12, 2026
17 checks passed
@tbantle22 tbantle22 deleted the taylor/merge-perm-conflicts-impl branch May 12, 2026 20:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants