Skip to content

docs(adr): define durable fencing contract - #557

Merged
delete-cloud merged 1 commit into
mainfrom
feat/durable-fencing-adr-contracts
Jun 5, 2026
Merged

delete-cloud merged 1 commit into
mainfrom
feat/durable-fencing-adr-contracts

Conversation

@delete-cloud

@delete-cloud delete-cloud commented Jun 5, 2026 •

Copy link
Copy Markdown
Owner

Summary

  • supersede ADR-0067 for the next durable fencing architecture
  • add ADR-0068 defining single local SQLite DB, DB-managed owner epoch, same-transaction owner + target checks, worker/maintenance authority, and follow-up PR sequence
  • add ADR contract tests locking the agreed PR1 design boundary

Tests

  • uv run pytest tests/coding_agent/test_durable_fencing_adr_contract.py -q
  • uv run ruff check tests/coding_agent/test_durable_fencing_adr_contract.py
  • uv run ruff format --check tests/coding_agent/test_durable_fencing_adr_contract.py
  • git diff --check

Local review

  • subagent review found 2 P2 contract-test gaps; both fixed
  • subagent rereview: no P1/P2 blocker

Summary by CodeRabbit

  • Documentation

    • Added new Architecture Decision Record (ADR-0068) documenting the local SQLite transactional durable fencing design approach.
    • Marked prior ADR-0067 as superseded by the new architecture.
  • Tests

    • Added comprehensive contract tests to validate the durable fencing architecture specification and design constraints.

@coderabbitai

coderabbitai Bot commented Jun 5, 2026 •

Copy link
Copy Markdown
Contributor

Review Change Stack

Warning

Review limit reached

@delete-cloud, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 44 minutes and 2 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more in the billing tab.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d5aa462a-23f6-4989-9c48-657583ae7508

📥 Commits

Reviewing files that changed from the base of the PR and between ddfeb0b and f51f868.

📒 Files selected for processing (3)
  • coding-agent/docs/adr/0067-local-sqlite-durable-tape-runtime.md
  • coding-agent/docs/adr/0068-local-sqlite-transactional-durable-fencing.md
  • coding-agent/tests/coding_agent/test_durable_fencing_adr_contract.py
📝 Walkthrough

Walkthrough

This PR transitions the durable fencing architecture from ADR-0067 to ADR-0068, replacing a multi-database local bundle approach with a single SQLite database (local.sqlite3) enforcing transactional atomicity for owner epoch checks, target ownership validation, and protected mutations. It introduces a comprehensive contract test suite to validate that ADR-0068 fully documents the required fencing semantics, authority models, and write-path operations.

Changes

Transactional Durable Fencing Architecture and Validation

Layer / File(s) Summary
ADR-0067 supersession and ADR-0068 target architecture
coding-agent/docs/adr/0067-local-sqlite-durable-tape-runtime.md, coding-agent/docs/adr/0068-local-sqlite-transactional-durable-fencing.md
ADR-0067 is marked superseded; ADR-0068 defines single-database transactional fencing with owner epoch checks, target ownership constraints, and protected mutations within atomic transactions, plus worker/maintenance authority, error semantics, and a four-PR implementation plan.
Durable fencing contract test suite
coding-agent/tests/coding_agent/test_durable_fencing_adr_contract.py
Six tests with path constants and utility helpers validate that ADR-0068 documents supersession semantics, single-DB epoch/transactional fencing requirements, ATTACH rejection, session ownership constraints, worker authority and recovery semantics, and complete write-path operation inventory.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

  • delete-cloud/Coding-Agent-Loop#556: Main PR that adds ADR-0068 and a comprehensive test suite validating that ADR-0068 supersedes ADR-0067's earlier multi-database local durability approach.

Poem

🐰 A SQLite home, single and strong,
Where transactions keep fencing along,
Epoch and owner dance in one beat,
Before commit seals the feat—
📝 Our durable contract, tested and sung!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title 'docs(adr): define durable fencing contract' clearly and concisely summarizes the main change—adding ADR-0068 to define the durable fencing architecture contract.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/durable-fencing-adr-contracts

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🧹 Nitpick comments (1)
coding-agent/docs/adr/0068-local-sqlite-transactional-durable-fencing.md (1)

49-53: 💤 Low value

Minor redundancy in phrasing.

Lines 51 and 53 both state that preflight checks "do not provide durable fencing." If the repetition is intentional for emphasis, consider using formatting (e.g., bold) instead of repeating the sentence.

📝 Suggested consolidation
 Cross-database `_assert_owner` checks and owner preflight checks remain useful
 admission or user-facing validation tools, but they are best-effort preflight.
-They do not provide durable fencing and must not be documented as preventing
-stale writes after owner loss.
-Best-effort preflight does not provide durable fencing.
+**They do not provide durable fencing** and must not be documented as preventing
+stale writes after owner loss.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@coding-agent/docs/adr/0068-local-sqlite-transactional-durable-fencing.md`
around lines 49 - 53, The two adjacent sentences repeating that owner preflight
checks "do not provide durable fencing" are redundant; in the ADR section
discussing Cross-database `_assert_owner` checks and owner preflight checks,
remove the duplicate sentence (keep a single clear statement) or alternatively
keep one sentence and bold it for emphasis so the doc reads concisely without
verbatim repetition while still conveying that preflight checks are best-effort
and do not provide durable fencing.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@coding-agent/docs/adr/0068-local-sqlite-transactional-durable-fencing.md`:
- Around line 49-53: The two adjacent sentences repeating that owner preflight
checks "do not provide durable fencing" are redundant; in the ADR section
discussing Cross-database `_assert_owner` checks and owner preflight checks,
remove the duplicate sentence (keep a single clear statement) or alternatively
keep one sentence and bold it for emphasis so the doc reads concisely without
verbatim repetition while still conveying that preflight checks are best-effort
and do not provide durable fencing.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: fb46e4d3-c61c-49cc-8964-4182771c5b5c

📥 Commits

Reviewing files that changed from the base of the PR and between bf67321 and ddfeb0b.

📒 Files selected for processing (3)
  • coding-agent/docs/adr/0067-local-sqlite-durable-tape-runtime.md
  • coding-agent/docs/adr/0068-local-sqlite-transactional-durable-fencing.md
  • coding-agent/tests/coding_agent/test_durable_fencing_adr_contract.py

@delete-cloud
delete-cloud force-pushed the feat/durable-fencing-adr-contracts branch from ddfeb0b to 85aaafb Compare June 5, 2026 06:47
@delete-cloud
delete-cloud force-pushed the feat/durable-fencing-adr-contracts branch from 85aaafb to f51f868 Compare June 5, 2026 06:58
@delete-cloud
delete-cloud merged commit 4b00a4c into main Jun 5, 2026
1 check passed
@delete-cloud
delete-cloud deleted the feat/durable-fencing-adr-contracts branch June 5, 2026 06:58
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.

1 participant